Re: [PATCH v4 3/4] NFSD: handle GETATTR conflict with write delegation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 5/21/23 7:56 PM, dai.ngo@xxxxxxxxxx wrote:

On 5/21/23 4:08 PM, Jeff Layton wrote:
On Sat, 2023-05-20 at 14:36 -0700, Dai Ngo wrote:
If the GETATTR request on a file that has write delegation in effect
and the request attributes include the change info and size attribute
then the write delegation is recalled and NFS4ERR_DELAY is returned
for the GETATTR.

Signed-off-by: Dai Ngo <dai.ngo@xxxxxxxxxx>
---
  fs/nfsd/nfs4xdr.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 45 insertions(+)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 76db2fe29624..e069b970f136 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2920,6 +2920,46 @@ nfsd4_encode_bitmap(struct xdr_stream *xdr, u32 bmval0, u32 bmval1, u32 bmval2)
      return nfserr_resource;
  }
  +static struct file_lock *
+nfs4_wrdeleg_filelock(struct svc_rqst *rqstp, struct inode *inode)
+{
+    struct file_lock_context *ctx;
+    struct file_lock *fl;
+
+    ctx = locks_inode_context(inode);
+    if (!ctx)
+        return NULL;
+    spin_lock(&ctx->flc_lock);
+    if (!list_empty(&ctx->flc_lease)) {
+        fl = list_first_entry(&ctx->flc_lease,
+                    struct file_lock, fl_list);
+        if (fl->fl_type == F_WRLCK) {
+            spin_unlock(&ctx->flc_lock);
+            return fl;
+        }
+    }
+    spin_unlock(&ctx->flc_lock);
+    return NULL;
+}
+
+static __be32
+nfs4_handle_wrdeleg_conflict(struct svc_rqst *rqstp, struct inode *inode)
+{
+    __be32 status;
+    struct file_lock *fl;
+    struct nfs4_delegation *dp;
+
+    fl = nfs4_wrdeleg_filelock(rqstp, inode);
+    if (!fl)
+        return 0;
+    dp = fl->fl_owner;
+    if (dp->dl_recall.cb_clp == *(rqstp->rq_lease_breaker))
+        return 0;
+    refcount_inc(&dp->dl_stid.sc_count);
Another question: Why are you taking a reference here at all?

This is same as in nfsd_break_one_deleg and revoke_delegation.
I think it is to prevent the delegation to be freed while delegation
is being recalled.

  AFAICT,
you don't even look at the delegation again after that point, so it's
not clear to me who's responsible for putting that reference.

In v2, the sc_count is decrement by nfs4_put_stid. I forgot to do that
in V4. I'll add it back in v5.

Actually the refcount is decremented after the CB_RECALL is done
by nfs4_put_stid in nfsd4_cb_recall_release. So we don't have to
decrement it here. This is to prevent the delegation to be free
while the recall is being sent.

-Dai


Thanks,
-Dai


+    status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ));
+    return status;
+}
+
  /*
   * Note: @fhp can be NULL; in this case, we might have to compose the filehandle
   * ourselves.
@@ -2966,6 +3006,11 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
          if (status)
              goto out;
      }
+    if (bmval0 & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) {
+        status = nfs4_handle_wrdeleg_conflict(rqstp, d_inode(dentry));
+        if (status)
+            goto out;
+    }
        err = vfs_getattr(&path, &stat,
                STATX_BASIC_STATS | STATX_BTIME | STATX_CHANGE_COOKIE,



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux