fcntl spinlock in Linux?

[J. Bruce Fields]

On Mon, Feb 18, 2013 at 04:08:36PM +1030, Rusty Russell wrote:
> "J. Bruce Fields" <bfields at fieldses.org> writes:
> 
> > On Thu, Feb 14, 2013 at 08:12:59PM +1030, Rusty Russell wrote:
> >> Volker Lendecke <Volker.Lendecke at SerNet.DE> writes:
> >> > Also, I am pretty certain that the fact that this happens in
> >> > an otherwise overloaded system adds to that effect, during
> >> > my tests here the systems were otherwise idle.
> >> 
> >> Oops, I dropped the list from my CC's somehow.
> >> 
> >> OK, I'll assume that improving this benchmark with 10000 or 20000 mean
> >> we have a real improvement.
> >> 
> >> I haven't tested, but one thing I noticed looking at the code: we're
> >> often deleted from the block list when we're woken, so we don't need
> >> to re-grab the lock in that case:
> >
> > Hm.  But  __locks_delete_block does more than make &waiter->fl_block
> > empty.  Isn't there a possibility that I could call locks_delete_block
> > while someone else was partway through __locks_delete_block?  So
> > something like:
> >
> > 				__locks_delete_block:
> > 				list_del_init(&waiter->fl_block);
> >
> > 	locks_delete_block sees
> > 	fl_block empty, returns,
> > 	caller frees the block
> > 				list_del_init(&waiter->fl_link);
> > 				waiter->fl_next = NULL;
> >
> > and then someone's writing into memory that belonged to the now-freed
> > block?
> 
> Sorry, it was more a though experiement (and benchmarking question) than
> patch candidate.

Fair enough, definitely looks worth a try.

> In practice, we should probably do:
> 
> void locks_delete_block(struct file_lock *waiter)
> {
> 	/* We could already be deleted by the time this runs. */
> 	if (!waiter->fl_next)
> 		return;
> 	lock_flocks();
> 	__locks_delete_block(waiter);
> 	unlock_flocks();
> }
> 
> and in __locks_delete_block:
> 
> {
> 	list_del_init(&waiter->fl_block);
> 	list_del_init(&waiter->fl_link);
>         smp_wmb();
> 	waiter->fl_next = NULL;
> }
> 
> (I'm assuming waiter->fl_next is never NULL, but that might be
> v. wrong).

That looks right.

--b.