Re: is the renewal in dentry_lease_is_valid necessary?

[Jeff Layton <jlayton@redhat.com>]

On Wed, 2016-06-29 at 14:53 +0800, Yan, Zheng wrote:
> > 
> > On Jun 28, 2016, at 22:00, Jeff Layton <jlayton@redhat.com> wrote:
> > 
> > On Tue, 2016-06-28 at 21:03 +0800, Yan, Zheng wrote:
> > > 
> > > On Tue, Jun 28, 2016 at 3:07 AM, Jeff Layton <jlayton@redhat.com>
> > > wrote:
> > > > 
> > > > 
> > > > I'm eyeballing how to make ceph dentry revalidation occur in
> > > > rcu-walk
> > > > mode in the kernel cephfs client. It looks fairly
> > > > straightforward, but
> > > > for one thing:
> > > > 
> > > > dentry_lease_is_valid queues an asynchronous lease renewal if
> > > > we're
> > > > halfway to the next lease renewal time:
> > > > 
> > > >         if (session) {
> > > >                 ceph_mdsc_lease_send_msg(session, dir, dentry,
> > > >                                          CEPH_MDS_LEASE_RENEW,
> > > > seq);
> > > >                 ceph_put_mds_session(session);
> > > >         }
> > > > 
> > > > That can sleep, so we have to drop out of rcu walk mode in that
> > > > case.
> > > > 
> > > > Is this lease renewal strictly necessary? I can make the code
> > > > drop out
> > > > of rcuwalk mode and try again in that case, but if this is just
> > > > an
> > > > opportunistic thing, then it may be better for performance to
> > > > take a
> > > > pass on renewing the lease early if we're still in RCU walk
> > > > mode.
> > > > 
> > > dentry_lease_is_valid is the only place that renew lease. I think
> > > it's
> > > better to drop out of rcuwalk and try again
> > > 
> > > Regards
> > > Yan, Zheng
> > > 
> > That seems really odd to me.
> > 
> > That call is only ever done in the d_revalidate codepath, so you'll
> > only get a lease renewal when you have repeated path-based access
> > to an
> > existing dentry.
> > 
> > Maybe this means that I don't really understand ceph leases. More
> > questions...
> > 
> > What exactly does the lease cover here?
> Lease covers dentry. MDS revoke release before changing dentry’s
> linkage. If client access a dentry frequently, it should renew the
> dentry lease when necessary. If dentry lease expires, client need to
> send a lookup request if it want to access the dentry.
> 

Ok, got it, thanks. In that case, what might be better is to queue the
lease renewal to a workqueue and stay in RCU lookup mode. It's not like
we're planning to wait around on the reply so dropping out of RCU mode
is probably not what we want. I'll take a look at whether it makes
sense to embed a work_struct in the ceph_dentry_info.

-- 
Jeff Layton <jlayton@redhat.com>

--
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