| Message ID | 20170209121815.4244-1-jlayton@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> On 9 Feb 2017, at 20:18, Jeff Layton <jlayton@redhat.com> wrote: > > ...and while we're at it, let's ensure that no one inadvertantly > leaves this thing on the unsafe list. There's no reason it should > ever be on a s_unsafe list but not in the request tree. > > Cc: stable@vger.kernel.org > Link: http://tracker.ceph.com/issues/18474 > Signed-off-by: Jeff Layton <jlayton@redhat.com> > --- > fs/ceph/mds_client.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > index 52521f339745..b47e97680a66 100644 > --- a/fs/ceph/mds_client.c > +++ b/fs/ceph/mds_client.c > @@ -628,6 +628,8 @@ static void __unregister_request(struct ceph_mds_client *mdsc, > { > dout("__unregister_request %p tid %lld\n", req, req->r_tid); > > + WARN_ON_ONCE(!list_empty(&req->r_unsafe_item)); > + > if (req->r_tid == mdsc->oldest_tid) { > struct rb_node *p = rb_next(&req->r_node); > mdsc->oldest_tid = 0; > @@ -3512,6 +3514,7 @@ static void wait_requests(struct ceph_mds_client *mdsc) > while ((req = __get_oldest_req(mdsc))) { > dout("wait_requests timed out on tid %llu\n", > req->r_tid); > + list_del_init(&req->r_unsafe_item); > __unregister_request(mdsc, req); > } > } Reviewed-by: Yan, Zheng <zyan@redhat.com> > -- > 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-09 at 07:18 -0500, Jeff Layton wrote: > ...and while we're at it, let's ensure that no one inadvertantly > leaves this thing on the unsafe list. There's no reason it should > ever be on a s_unsafe list but not in the request tree. > > Cc: stable@vger.kernel.org > Link: http://tracker.ceph.com/issues/18474 > Signed-off-by: Jeff Layton <jlayton@redhat.com> > --- > fs/ceph/mds_client.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c > index 52521f339745..b47e97680a66 100644 > --- a/fs/ceph/mds_client.c > +++ b/fs/ceph/mds_client.c > @@ -628,6 +628,8 @@ static void __unregister_request(struct ceph_mds_client *mdsc, > { > dout("__unregister_request %p tid %lld\n", req, req->r_tid); > > + WARN_ON_ONCE(!list_empty(&req->r_unsafe_item)); > + > if (req->r_tid == mdsc->oldest_tid) { > struct rb_node *p = rb_next(&req->r_node); > mdsc->oldest_tid = 0; > @@ -3512,6 +3514,7 @@ static void wait_requests(struct ceph_mds_client *mdsc) > while ((req = __get_oldest_req(mdsc))) { > dout("wait_requests timed out on tid %llu\n", > req->r_tid); > + list_del_init(&req->r_unsafe_item); > __unregister_request(mdsc, req); > } > } After going over the kernel code more thoroughly, I started to get concerned that there were some cases (particularly those involving fatal signals) where we could end up leaving it on the list before calling __unregister_request. I've just sent a v2 patch, that moves the list_del_init into __unregister_request itself, which ensures that it's always removed from the list in those cases as well.
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c index 52521f339745..b47e97680a66 100644 --- a/fs/ceph/mds_client.c +++ b/fs/ceph/mds_client.c @@ -628,6 +628,8 @@ static void __unregister_request(struct ceph_mds_client *mdsc, { dout("__unregister_request %p tid %lld\n", req, req->r_tid); + WARN_ON_ONCE(!list_empty(&req->r_unsafe_item)); + if (req->r_tid == mdsc->oldest_tid) { struct rb_node *p = rb_next(&req->r_node); mdsc->oldest_tid = 0; @@ -3512,6 +3514,7 @@ static void wait_requests(struct ceph_mds_client *mdsc) while ((req = __get_oldest_req(mdsc))) { dout("wait_requests timed out on tid %llu\n", req->r_tid); + list_del_init(&req->r_unsafe_item); __unregister_request(mdsc, req); } }
...and while we're at it, let's ensure that no one inadvertantly leaves this thing on the unsafe list. There's no reason it should ever be on a s_unsafe list but not in the request tree. Cc: stable@vger.kernel.org Link: http://tracker.ceph.com/issues/18474 Signed-off-by: Jeff Layton <jlayton@redhat.com> --- fs/ceph/mds_client.c | 3 +++ 1 file changed, 3 insertions(+)