xfs
[Top] [All Lists]

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

To: Matthew Wilcox <matthew@xxxxxx>
Subject: Re: [PATCH 2/6] Replace inode flush semaphore with a completion
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 27 Jun 2008 14:13:51 +1000
Cc: xfs@xxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
In-reply-to: <20080627023011.GC7703@xxxxxxxxxxxxxxxx>
Mail-followup-to: Matthew Wilcox <matthew@xxxxxx>, xfs@xxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
References: <1214455277-6387-1-git-send-email-david@xxxxxxxxxxxxx> <1214455277-6387-3-git-send-email-david@xxxxxxxxxxxxx> <20080627023011.GC7703@xxxxxxxxxxxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.17+20080114 (2008-01-14)
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@xxxxxxxxxxxxx


<Prev in Thread] Current Thread [Next in Thread>