Messages in this thread Patch in this message | | | Date | Fri, 27 Jun 2008 14:13:51 +1000 | From | Dave Chinner <> | Subject | Re: [PATCH 2/6] Replace inode flush semaphore with a completion |
| |
On Thu, Jun 26, 2008 at 08:30:11PM -0600, Matthew Wilcox wrote: > On Thu, Jun 26, 2008 at 02:41:13PM +1000, Dave Chinner wrote: > > Use the new completion flush code to implement the inode > > flush lock. Removes one of the final users of semaphores > > in the XFS code base. > > Let's demonstrate converting this one to completions ...
Great. I need all the help I can get. ;)
> > --- a/fs/xfs/xfs_iget.c > > +++ b/fs/xfs/xfs_iget.c > > @@ -216,7 +216,7 @@ finish_inode: > > mrlock_init(&ip->i_iolock, MRLOCK_BARRIER, "xfsio", ip->i_ino); > > init_waitqueue_head(&ip->i_ipin_wait); > > atomic_set(&ip->i_pincount, 0); > > - initnsema(&ip->i_flock, 1, "xfsfino"); > > + init_completion_flush(&ip->i_flush); > > + init_completion(&ip->i_flush); > + complete(&ip->i_flush);
Ok, that would work. Need commenting, though.
> > int > > xfs_iflock_nowait(xfs_inode_t *ip) > > { > > - return (cpsema(&(ip->i_flock))); > > + return completion_flush_start_nowait(&ip->i_flush); > > This is where you need a new function ... > > + return nowait_for_completion(&ip->i_flush); > > Yes, we probably need a better name for the down_trylock() equivalent.
I suggested try_wait_for_completion() in a different email - it's a difficult on to say "a wait_for_completion() call that doesn't wait"....
I think I'll stick with the try_.... version to match other "try" operations.
> > int > > @@ -3070,7 +3069,7 @@ xfs_iflush( > > XFS_STATS_INC(xs_iflush_count); > > > > ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED)); > > - ASSERT(issemalocked(&(ip->i_flock))); > > + ASSERT(completion_flush_inprogress(&ip->i_flush)); > > is_complete()?
That's a little to general for my liking - I note that there are already an existing is_complete() function in a driver. completion_done() perhaps?
Cheers,
Dave. -- Dave Chinner david@fromorbit.com
| |