| Message ID | 20220328022535.847164-1-xiubli@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: remove unused CEPH_MDS_LEASE_RELEASE related code | expand |
On Mon, 2022-03-28 at 10:25 +0800, xiubli@redhat.com wrote: > From: Xiubo Li <xiubli@redhat.com> > > The ceph_mdsc_lease_release() has been removed by commit(8aa152c77890) > and the CEPH_MDS_LEASE_RELEASE will never be used. > Like it says in Documentation/process/5.Posting.rst: "...and please provide both the commit ID and the title when citing commits" You might want to reword this with something like: "ceph_mdsc_lease_release was removed by commit 8aa152c77890 (ceph: remove ceph_mdsc_lease_release). ceph_mdsc_lease_send_msg will never call this function with CEPH_MDS_LEASE_RELEASE." > Signed-off-by: Xiubo Li <xiubli@redhat.com> > --- > fs/ceph/mds_client.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > index 459c6f23915f..a89ee866ebbb 100644 > --- a/fs/ceph/mds_client.c > +++ b/fs/ceph/mds_client.c > @@ -4424,12 +4424,6 @@ void ceph_mdsc_lease_send_msg(struct ceph_mds_session *session, > memcpy((void *)(lease + 1) + 4, > dentry->d_name.name, dentry->d_name.len); > spin_unlock(&dentry->d_lock); > - /* > - * if this is a preemptive lease RELEASE, no need to > - * flush request stream, since the actual request will > - * soon follow. > - */ > - msg->more_to_follow = (action == CEPH_MDS_LEASE_RELEASE); > > ceph_con_send(&session->s_con, msg); > } It might be possible to trim this function back further. There's only one caller and it always calls this with the same "action" value. Still, this looks fine... Reviewed-by: Jeff Layton <jlayton@kernel.org>
On 3/28/22 6:28 PM, Jeff Layton wrote: > On Mon, 2022-03-28 at 10:25 +0800, xiubli@redhat.com wrote: >> From: Xiubo Li <xiubli@redhat.com> >> >> The ceph_mdsc_lease_release() has been removed by commit(8aa152c77890) >> and the CEPH_MDS_LEASE_RELEASE will never be used. >> > Like it says in Documentation/process/5.Posting.rst: > > "...and please provide both the commit ID and the title when citing > commits" > > You might want to reword this with something like: > > "ceph_mdsc_lease_release was removed by commit 8aa152c77890 (ceph: > remove ceph_mdsc_lease_release). ceph_mdsc_lease_send_msg will never > call this function with CEPH_MDS_LEASE_RELEASE." Sure, will fix it. >> Signed-off-by: Xiubo Li <xiubli@redhat.com> >> --- >> fs/ceph/mds_client.c | 6 ------ >> 1 file changed, 6 deletions(-) >> >> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c >> index 459c6f23915f..a89ee866ebbb 100644 >> --- a/fs/ceph/mds_client.c >> +++ b/fs/ceph/mds_client.c >> @@ -4424,12 +4424,6 @@ void ceph_mdsc_lease_send_msg(struct ceph_mds_session *session, >> memcpy((void *)(lease + 1) + 4, >> dentry->d_name.name, dentry->d_name.len); >> spin_unlock(&dentry->d_lock); >> - /* >> - * if this is a preemptive lease RELEASE, no need to >> - * flush request stream, since the actual request will >> - * soon follow. >> - */ >> - msg->more_to_follow = (action == CEPH_MDS_LEASE_RELEASE); >> >> ceph_con_send(&session->s_con, msg); >> } > It might be possible to trim this function back further. There's only > one caller and it always calls this with the same "action" value. Still, > this looks fine... Yeah, I checked the commit history in both kernel and ceph, it seems never used since this was introduced. Maybe I missed something. The user space clients is use the CEPH_MDS_LEASE_RELEASE as the lease revoke ack. In kernel we are using the CEPH_MDS_LEASE_REVOKE_ACK instead. Thanks. > > Reviewed-by: Jeff Layton <jlayton@kernel.org> >
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c index 459c6f23915f..a89ee866ebbb 100644 --- a/fs/ceph/mds_client.c +++ b/fs/ceph/mds_client.c @@ -4424,12 +4424,6 @@ void ceph_mdsc_lease_send_msg(struct ceph_mds_session *session, memcpy((void *)(lease + 1) + 4, dentry->d_name.name, dentry->d_name.len); spin_unlock(&dentry->d_lock); - /* - * if this is a preemptive lease RELEASE, no need to - * flush request stream, since the actual request will - * soon follow. - */ - msg->more_to_follow = (action == CEPH_MDS_LEASE_RELEASE); ceph_con_send(&session->s_con, msg); }