BUG: cannot acquire lock twice with NFSv4

BUG: cannot acquire lock twice with NFSv4

[Arnaud Giersch]

Hi,

I tried to experiment with NFSv4 to mount my home directory, but
bogofilter started to fail.
I was able to produce the following minimal test case, by writing a
program that:

  1. opens a first file, and acquires read lock on it ;
  2. opens a second file, and acquires read lock on it ;
  3. releases locks, and closes files.

Both opened files are of course on the NFS mount.  On the first run, all
seems to be fine.  On the second (and subsequent) runs, the lock is
refused at step 2 with errno=37 (ENOLCK, No locks available).

Steps 1 and 2 look like the following (I can provide the full source
code if needed):

    int fd = open(filename, O_RDONLY);
    struct flock lock = {
        .l_type = F_RDLCK,
        .l_whence = SEEK_SET,
        .l_start = 0,
        .l_len = 0,
    }
    fcntl(fd, F_SETLK, &lock);

Umounting and re-mounting the directory make the first run work again as
expected.  There is no problem with NFSv3, only with v4.

I first experimented the problem with the Debian kernels 2.6.26-2-amd64
for the server, and 2.6.33-2-amd64 for the client.
I also tried the latest kernel (2.6.34-rc3), either as client or as server
with no luck.

Regards,

        Arnaud Giersch

Re: BUG: cannot acquire lock twice with NFSv4

[Arnaud Giersch]

Vendredi 09 avril 2010, vers 15:05:14 (+0200), j'ai =C3=A9crit=C2=A0:

>   1. opens a first file, and acquires read lock on it ;
>   2. opens a second file, and acquires read lock on it ;
>   3. releases locks, and closes files.
>
> Both opened files are of course on the NFS mount.  On the first run, =
all
> seems to be fine.  On the second (and subsequent) runs, the lock is
> refused at step 2 with errno=3D37 (ENOLCK, No locks available).

I noticed that things appear to work as expected if commit
8e469ebd6dc32cbaf620e134d79f740bf0ebab79 (NFSv4: Don't allow posix
locking against servers that don't support it) is reverted.

=46rom what I can see, the client is granted a read delegation for the
second file.  On the second run, the client fetches some cached state
for this file.

On this second run, state->flags (as in _nfs4_do_open, or in
nfs4_proc_lock) only contains NFS_DELEGATED_STATE, while on the first
run, it also contained NFS_O_RDONLY_STATE and NFS_STATE_POSIX_LOCKS.

Then, I do not fully understand the code, and I am not able to suggest
any satisfying patch to correct the problem.

Regards,

        Arnaud Giersch

Re: BUG: cannot acquire lock twice with NFSv4

[Trond Myklebust]

On Sat, 2010-04-10 at 10:46 +0200, Arnaud Giersch wrote:=20
> Vendredi 09 avril 2010, vers 15:05:14 (+0200), j'ai =C3=A9crit :
>=20
> >   1. opens a first file, and acquires read lock on it ;
> >   2. opens a second file, and acquires read lock on it ;
> >   3. releases locks, and closes files.
> >
> > Both opened files are of course on the NFS mount.  On the first run=
, all
> > seems to be fine.  On the second (and subsequent) runs, the lock is
> > refused at step 2 with errno=3D37 (ENOLCK, No locks available).
>=20
> I noticed that things appear to work as expected if commit
> 8e469ebd6dc32cbaf620e134d79f740bf0ebab79 (NFSv4: Don't allow posix
> locking against servers that don't support it) is reverted.
>=20
> From what I can see, the client is granted a read delegation for the
> second file.  On the second run, the client fetches some cached state
> for this file.
>=20
> On this second run, state->flags (as in _nfs4_do_open, or in
> nfs4_proc_lock) only contains NFS_DELEGATED_STATE, while on the first
> run, it also contained NFS_O_RDONLY_STATE and NFS_STATE_POSIX_LOCKS.

Ah. Nice catch! That does indeed need to be fixed... I'll draft up a
patch tomorrow, and send you a copy.

Cheers
  Trond

Re: BUG: cannot acquire lock twice with NFSv4

[Trond Myklebust]

On Sun, 2010-04-11 at 11:24 -0400, Trond Myklebust wrote: 
> On Sat, 2010-04-10 at 10:46 +0200, Arnaud Giersch wrote: 
> > Vendredi 09 avril 2010, vers 15:05:14 (+0200), j'ai écrit :
> > 
> > >   1. opens a first file, and acquires read lock on it ;
> > >   2. opens a second file, and acquires read lock on it ;
> > >   3. releases locks, and closes files.
> > >
> > > Both opened files are of course on the NFS mount.  On the first run, all
> > > seems to be fine.  On the second (and subsequent) runs, the lock is
> > > refused at step 2 with errno=37 (ENOLCK, No locks available).
> > 
> > I noticed that things appear to work as expected if commit
> > 8e469ebd6dc32cbaf620e134d79f740bf0ebab79 (NFSv4: Don't allow posix
> > locking against servers that don't support it) is reverted.
> > 
> > From what I can see, the client is granted a read delegation for the
> > second file.  On the second run, the client fetches some cached state
> > for this file.
> > 
> > On this second run, state->flags (as in _nfs4_do_open, or in
> > nfs4_proc_lock) only contains NFS_DELEGATED_STATE, while on the first
> > run, it also contained NFS_O_RDONLY_STATE and NFS_STATE_POSIX_LOCKS.
> 
> Ah. Nice catch! That does indeed need to be fixed... I'll draft up a
> patch tomorrow, and send you a copy.
> 
> Cheers
>   Trond

How about the following?

Cheers
  Trond
----------------------------------------------------------------------------------------------- 
NFSv4: fix delegated locking

From: Trond Myklebust <Trond.Myklebust@netapp.com>

Arnaud Giersch reports that NFSv4 locking is broken when we hold a
delegation since commit 8e469ebd6dc32cbaf620e134d79f740bf0ebab79 (NFSv4:
Don't allow posix locking against servers that don't support it).

According to Arnaud, the lock succeeds the first time he opens the file
(since we cannot do a delegated open) but then fails after we start using
delegated opens.

The following patch fixes it by ensuring that locking behaviour is
governed by a per-filesystem capability flag that is initially set, but
gets cleared if the server ever returns an OPEN without the
NFS4_OPEN_RESULT_LOCKTYPE_POSIX flag being set.

Reported-by: Arnaud Giersch <arnaud.giersch@iut-bm.univ-fcomte.fr>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---

 fs/nfs/client.c           |    3 ++-
 fs/nfs/nfs4proc.c         |    4 +++-
 include/linux/nfs_fs_sb.h |    1 +
 3 files changed, 6 insertions(+), 2 deletions(-)


diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 2a3d352..a8766c4 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1294,7 +1294,8 @@ static int nfs4_init_server(struct nfs_server *server,
 
 	/* Initialise the client representation from the mount data */
 	server->flags = data->flags;
-	server->caps |= NFS_CAP_ATOMIC_OPEN|NFS_CAP_CHANGE_ATTR;
+	server->caps |= NFS_CAP_ATOMIC_OPEN|NFS_CAP_CHANGE_ATTR|
+		NFS_CAP_POSIX_LOCK;
 	server->options = data->options;
 
 	/* Get a client record */
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index fe0cd9e..6380670 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1523,6 +1523,8 @@ static int _nfs4_proc_open(struct nfs4_opendata *data)
 		nfs_post_op_update_inode(dir, o_res->dir_attr);
 	} else
 		nfs_refresh_inode(dir, o_res->dir_attr);
+	if ((o_res->rflags & NFS4_OPEN_RESULT_LOCKTYPE_POSIX) == 0)
+		server->caps &= ~NFS_CAP_POSIX_LOCK;
 	if(o_res->rflags & NFS4_OPEN_RESULT_CONFIRM) {
 		status = _nfs4_proc_open_confirm(data);
 		if (status != 0)
@@ -1664,7 +1666,7 @@ static int _nfs4_do_open(struct inode *dir, struct path *path, fmode_t fmode, in
 	status = PTR_ERR(state);
 	if (IS_ERR(state))
 		goto err_opendata_put;
-	if ((opendata->o_res.rflags & NFS4_OPEN_RESULT_LOCKTYPE_POSIX) != 0)
+	if (server->caps & NFS_CAP_POSIX_LOCK)
 		set_bit(NFS_STATE_POSIX_LOCKS, &state->flags);
 	nfs4_opendata_put(opendata);
 	nfs4_put_state_owner(sp);
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 717a5e5..e82957a 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -176,6 +176,7 @@ struct nfs_server {
 #define NFS_CAP_ATIME		(1U << 11)
 #define NFS_CAP_CTIME		(1U << 12)
 #define NFS_CAP_MTIME		(1U << 13)
+#define NFS_CAP_POSIX_LOCK	(1U << 14)
 

 /* maximum number of slots to use */

Re: BUG: cannot acquire lock twice with NFSv4

[Arnaud Giersch]

Dimanche 11 avril 2010, vers 22:50:30 (+0200), Trond Myklebust a écrit :

> How about the following?
> ----------------------------------------------------------------------------------------------- 
> NFSv4: fix delegated locking
[...]

It looks fine to me, and the locking now appears to work as expected.

Thank you!

        Arnaud