| Message ID | 20170201114914.20808-13-jlayton@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> 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
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.
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);
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(+)