[v2,12/13] ceph: call update_dentry_lease even when r_locked dir is not set
diff mbox

Message ID 20170201114914.20808-13-jlayton@redhat.com
State New
Headers show

Commit Message

Jeff Layton Feb. 1, 2017, 11:49 a.m. UTC
We don't really require that the parent be locked in order to update the
lease on a dentry. Lease info is protected by the d_lock. In the event
that the parent is not locked in ceph_fill_trace, and we have both
parent and target info, go ahead and update the dentry lease.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/ceph/inode.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Yan, Zheng Feb. 2, 2017, 8:34 a.m. UTC | #1
> On 1 Feb 2017, at 19:49, Jeff Layton <jlayton@redhat.com> wrote:
> 
> We don't really require that the parent be locked in order to update the
> lease on a dentry. Lease info is protected by the d_lock. In the event
> that the parent is not locked in ceph_fill_trace, and we have both
> parent and target info, go ahead and update the dentry lease.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> fs/ceph/inode.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
> 
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 15e042a8d71f..87863f962d50 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -1345,6 +1345,16 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> 			goto done;
> 		}
> 		req->r_dentry = dn;  /* may have spliced */
> +	} else if (rinfo->head->is_dentry && rinfo->head->is_target) {
> +		if ((le32_to_cpu(rinfo->diri.in->cap.caps) & CEPH_CAP_FILE_SHARED) ||
> +		    le32_to_cpu(rinfo->dlease->duration_ms)) {
> +			vino.ino = le64_to_cpu(rinfo->diri.in->ino);
> +			vino.snap = le64_to_cpu(rinfo->diri.in->snapid);
> +			update_dentry_lease(req->r_dentry, rinfo->dlease,
> +				session, req->r_request_started, &vino);
> +		} else {
> +			dout("%s: no dentry lease or dir cap\n", __func__);
> +		}
> 	}

I think checking rinfo->head->is_target is not needed here, because null dentry can also have lease.
Besides, I think we need to check if rinfo->head->is_target matches d_really_is_negative(dn) and if
the target inode matches d_inode(dn).

> 	dout("fill_trace done err=%d\n", err);
> -- 
> 2.9.3
> 

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff Layton Feb. 2, 2017, 11:27 a.m. UTC | #2
On Thu, 2017-02-02 at 16:34 +0800, Yan, Zheng wrote:
> > On 1 Feb 2017, at 19:49, Jeff Layton <jlayton@redhat.com> wrote:
> > 
> > We don't really require that the parent be locked in order to update the
> > lease on a dentry. Lease info is protected by the d_lock. In the event
> > that the parent is not locked in ceph_fill_trace, and we have both
> > parent and target info, go ahead and update the dentry lease.
> > 
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/ceph/inode.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> > 
> > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > index 15e042a8d71f..87863f962d50 100644
> > --- a/fs/ceph/inode.c
> > +++ b/fs/ceph/inode.c
> > @@ -1345,6 +1345,16 @@ int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
> > 			goto done;
> > 		}
> > 		req->r_dentry = dn;  /* may have spliced */
> > +	} else if (rinfo->head->is_dentry && rinfo->head->is_target) {
> > +		if ((le32_to_cpu(rinfo->diri.in->cap.caps) & CEPH_CAP_FILE_SHARED) ||
> > +		    le32_to_cpu(rinfo->dlease->duration_ms)) {
> > +			vino.ino = le64_to_cpu(rinfo->diri.in->ino);
> > +			vino.snap = le64_to_cpu(rinfo->diri.in->snapid);
> > +			update_dentry_lease(req->r_dentry, rinfo->dlease,
> > +				session, req->r_request_started, &vino);
> > +		} else {
> > +			dout("%s: no dentry lease or dir cap\n", __func__);
> > +		}
> > 	}
> 
> I think checking rinfo->head->is_target is not needed here, because null dentry can also have lease.
> Besides, I think we need to check if rinfo->head->is_target matches d_really_is_negative(dn) and if
> the target inode matches d_inode(dn).
> 

Yeah, I think you're right here. I'll respin this one.

Patch
diff mbox

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index 15e042a8d71f..87863f962d50 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -1345,6 +1345,16 @@  int ceph_fill_trace(struct super_block *sb, struct ceph_mds_request *req)
 			goto done;
 		}
 		req->r_dentry = dn;  /* may have spliced */
+	} else if (rinfo->head->is_dentry && rinfo->head->is_target) {
+		if ((le32_to_cpu(rinfo->diri.in->cap.caps) & CEPH_CAP_FILE_SHARED) ||
+		    le32_to_cpu(rinfo->dlease->duration_ms)) {
+			vino.ino = le64_to_cpu(rinfo->diri.in->ino);
+			vino.snap = le64_to_cpu(rinfo->diri.in->snapid);
+			update_dentry_lease(req->r_dentry, rinfo->dlease,
+				session, req->r_request_started, &vino);
+		} else {
+			dout("%s: no dentry lease or dir cap\n", __func__);
+		}
 	}
 done:
 	dout("fill_trace done err=%d\n", err);