nfsd4: fix open downgrade, again
J. Bruce Fields [Mon, 19 Sep 2011 19:07:41 +0000 (15:07 -0400)]
commit 3d02fa29dec920c597dd7b7db608a4bc71f088ce upstream.

Yet another open-management regression:

- nfs4_file_downgrade() doesn't remove the BOTH access bit on
  downgrade, so the server's idea of the stateid's access gets
  out of sync with the client's.  If we want to keep an O_RDWR
  open in this case, we should do that in the file_put_access
  logic rather than here.
- We forgot to convert v4 access to an open mode here.

This logic has proven too hard to get right.  In the future we may
consider:
- reexamining the lock/openowner relationship (locks probably
  don't really need to take their own references here).
- adding open upgrade/downgrade support to the vfs.
- removing the atomic operations.  They're redundant as long as
  this is all under some other lock.

Also, maybe some kind of additional static checking would help catch
O_/NFS4_SHARE_ACCESS confusion.

Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

fs/nfsd/nfs4state.c

index 9890d95..cd2a43d 100644 (file)
@@ -192,8 +192,15 @@ static void nfs4_file_put_fd(struct nfs4_file *fp, int oflag)
 static void __nfs4_file_put_access(struct nfs4_file *fp, int oflag)
 {
        if (atomic_dec_and_test(&fp->fi_access[oflag])) {
-               nfs4_file_put_fd(fp, O_RDWR);
                nfs4_file_put_fd(fp, oflag);
+               /*
+                * It's also safe to get rid of the RDWR open *if*
+                * we no longer have need of the other kind of access
+                * or if we already have the other kind of open:
+                */
+               if (fp->fi_fds[1-oflag]
+                       || atomic_read(&fp->fi_access[1 - oflag]) == 0)
+                       nfs4_file_put_fd(fp, O_RDWR);
        }
 }
 
@@ -3530,8 +3537,9 @@ static inline void nfs4_file_downgrade(struct nfs4_stateid *stp, unsigned int to
        int i;
 
        for (i = 1; i < 4; i++) {
-               if (test_bit(i, &stp->st_access_bmap) && !(i & to_access)) {
-                       nfs4_file_put_access(stp->st_file, i);
+               if (test_bit(i, &stp->st_access_bmap)
+                                       && ((i & to_access) != i)) {
+                       nfs4_file_put_access(stp->st_file, nfs4_access_to_omode(i));
                        __clear_bit(i, &stp->st_access_bmap);
                }
        }