Re: [PATCH 2/6] Replace inode flush semaphore with a completion

Dave Chinner
2008-06-27T04:14:10Z

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