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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 27 Jun 2008 14:13:51 +1000
From:	Dave Chinner <david@...morbit.com>
To:	Matthew Wilcox <matthew@....cx>
Cc:	xfs@....sgi.com, linux-kernel@...r.kernel.org
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@...morbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/