Re: nfsd changes for 2.6.37

[Arnd Bergmann <arnd@arndb.de>]

On Wednesday 27 October 2010 00:11:56 J. Bruce Fields wrote:
> > BUG: sleeping function called from invalid context at mm/slab.c:3101
> > in_atomic(): 1, irqs_disabled(): 0, pid: 4345, name: lease_tests
> > 1 lock held by lease_tests/4345:
> >  #0:  (file_lock_lock){+.+.+.}, at: [<ffffffff81128be5>] lock_flocks+0x15/0x20
> > Pid: 4345, comm: lease_tests Not tainted 2.6.36-05858-gbd5e20b #1028
> > Call Trace:
> >  [<ffffffff8103141d>] __might_sleep+0x10d/0x140
> >  [<ffffffff810e3ad3>] kmem_cache_alloc+0x1f3/0x230
> >  [<ffffffff8112a4d2>] generic_setlease+0x112/0x2c0
> >  [<ffffffff8112a6b5>] __vfs_setlease+0x35/0x40
> >  [<ffffffff8112acfe>] fcntl_setlease+0xce/0x180
> >  [<ffffffff810f7c2e>] sys_fcntl+0x2fe/0x630
> >  [<ffffffff81961999>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> >  [<ffffffff81002658>] system_call_fastpath+0x16/0x1b
> > 
> > I'm testing a patch.
> 

Thanks for the report!

> @@ -1524,8 +1528,6 @@ int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
>         if (error)
>                 return error;
>  
> -       lock_flocks();
> -
>         error = __vfs_setlease(filp, arg, &flp);
>         if (error || arg == F_UNLCK)
>                 goto out_unlock;
> @@ -1541,7 +1543,6 @@ int fcntl_setlease(unsigned int fd, struct file *filp, long arg)
>  
>         error = __f_setown(filp, task_pid(current), PIDTYPE_PID, 0);
>  out_unlock:
> -       unlock_flocks();
>         return error;
>  }

If you don't hold lock_flocks throughout fcntl_setlease, the flp variable
points to a flock that may get modified by another thread and you call
time_out_leases() without holding lock_flocks, which it requires.

The two alternatives I can see are to either use GFP_ATOMIC or to
take the lock inside of generic_setlease and drop it outside.
Neither of the two sounds particularly appealing.

	Arnd