| Message ID | 20230801222529.674721-1-idryomov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | libceph: fix potential hang in ceph_osdc_notify() | expand |
LGTM. Reviewed-by: Xiubo Li <xiubli@redhat.com> On 8/2/23 06:25, Ilya Dryomov wrote: > If the cluster becomes unavailable, ceph_osdc_notify() may hang even > with osd_request_timeout option set because linger_notify_finish_wait() > waits for MWatchNotify NOTIFY_COMPLETE message with no associated OSD > request in flight -- it's completely asynchronous. > > Introduce an additional timeout, derived from the specified notify > timeout. While at it, switch both waits to killable which is more > correct. > > Cc: stable@vger.kernel.org > Signed-off-by: Ilya Dryomov <idryomov@gmail.com> > --- > net/ceph/osd_client.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c > index 11c04e7d928e..658a6f2320cf 100644 > --- a/net/ceph/osd_client.c > +++ b/net/ceph/osd_client.c > @@ -3334,17 +3334,24 @@ static int linger_reg_commit_wait(struct ceph_osd_linger_request *lreq) > int ret; > > dout("%s lreq %p linger_id %llu\n", __func__, lreq, lreq->linger_id); > - ret = wait_for_completion_interruptible(&lreq->reg_commit_wait); > + ret = wait_for_completion_killable(&lreq->reg_commit_wait); > return ret ?: lreq->reg_commit_error; > } > > -static int linger_notify_finish_wait(struct ceph_osd_linger_request *lreq) > +static int linger_notify_finish_wait(struct ceph_osd_linger_request *lreq, > + unsigned long timeout) > { > - int ret; > + long left; > > dout("%s lreq %p linger_id %llu\n", __func__, lreq, lreq->linger_id); > - ret = wait_for_completion_interruptible(&lreq->notify_finish_wait); > - return ret ?: lreq->notify_finish_error; > + left = wait_for_completion_killable_timeout(&lreq->notify_finish_wait, > + ceph_timeout_jiffies(timeout)); > + if (left <= 0) > + left = left ?: -ETIMEDOUT; > + else > + left = lreq->notify_finish_error; /* completed */ > + > + return left; > } > > /* > @@ -4896,7 +4903,8 @@ int ceph_osdc_notify(struct ceph_osd_client *osdc, > linger_submit(lreq); > ret = linger_reg_commit_wait(lreq); > if (!ret) > - ret = linger_notify_finish_wait(lreq); > + ret = linger_notify_finish_wait(lreq, > + msecs_to_jiffies(2 * timeout * MSEC_PER_SEC)); > else > dout("lreq %p failed to initiate notify %d\n", lreq, ret); >
在 2023/8/2 星期三 上午 6:25, Ilya Dryomov 写道: > If the cluster becomes unavailable, ceph_osdc_notify() may hang even > with osd_request_timeout option set because linger_notify_finish_wait() > waits for MWatchNotify NOTIFY_COMPLETE message with no associated OSD > request in flight -- it's completely asynchronous. > > Introduce an additional timeout, derived from the specified notify > timeout. While at it, switch both waits to killable which is more > correct. > > Cc: stable@vger.kernel.org > Signed-off-by: Ilya Dryomov <idryomov@gmail.com> Reviewed-by: Dongsheng Yang <dongsheng.yang@easystack.cn> > --- > net/ceph/osd_client.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c > index 11c04e7d928e..658a6f2320cf 100644 > --- a/net/ceph/osd_client.c > +++ b/net/ceph/osd_client.c > @@ -3334,17 +3334,24 @@ static int linger_reg_commit_wait(struct ceph_osd_linger_request *lreq) > int ret; > > dout("%s lreq %p linger_id %llu\n", __func__, lreq, lreq->linger_id); > - ret = wait_for_completion_interruptible(&lreq->reg_commit_wait); > + ret = wait_for_completion_killable(&lreq->reg_commit_wait); > return ret ?: lreq->reg_commit_error; > } > > -static int linger_notify_finish_wait(struct ceph_osd_linger_request *lreq) > +static int linger_notify_finish_wait(struct ceph_osd_linger_request *lreq, > + unsigned long timeout) > { > - int ret; > + long left; > > dout("%s lreq %p linger_id %llu\n", __func__, lreq, lreq->linger_id); > - ret = wait_for_completion_interruptible(&lreq->notify_finish_wait); > - return ret ?: lreq->notify_finish_error; > + left = wait_for_completion_killable_timeout(&lreq->notify_finish_wait, > + ceph_timeout_jiffies(timeout)); > + if (left <= 0) > + left = left ?: -ETIMEDOUT; > + else > + left = lreq->notify_finish_error; /* completed */ > + > + return left; > } > > /* > @@ -4896,7 +4903,8 @@ int ceph_osdc_notify(struct ceph_osd_client *osdc, > linger_submit(lreq); > ret = linger_reg_commit_wait(lreq); > if (!ret) > - ret = linger_notify_finish_wait(lreq); > + ret = linger_notify_finish_wait(lreq, > + msecs_to_jiffies(2 * timeout * MSEC_PER_SEC)); > else > dout("lreq %p failed to initiate notify %d\n", lreq, ret); > >
diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c index 11c04e7d928e..658a6f2320cf 100644 --- a/net/ceph/osd_client.c +++ b/net/ceph/osd_client.c @@ -3334,17 +3334,24 @@ static int linger_reg_commit_wait(struct ceph_osd_linger_request *lreq) int ret; dout("%s lreq %p linger_id %llu\n", __func__, lreq, lreq->linger_id); - ret = wait_for_completion_interruptible(&lreq->reg_commit_wait); + ret = wait_for_completion_killable(&lreq->reg_commit_wait); return ret ?: lreq->reg_commit_error; } -static int linger_notify_finish_wait(struct ceph_osd_linger_request *lreq) +static int linger_notify_finish_wait(struct ceph_osd_linger_request *lreq, + unsigned long timeout) { - int ret; + long left; dout("%s lreq %p linger_id %llu\n", __func__, lreq, lreq->linger_id); - ret = wait_for_completion_interruptible(&lreq->notify_finish_wait); - return ret ?: lreq->notify_finish_error; + left = wait_for_completion_killable_timeout(&lreq->notify_finish_wait, + ceph_timeout_jiffies(timeout)); + if (left <= 0) + left = left ?: -ETIMEDOUT; + else + left = lreq->notify_finish_error; /* completed */ + + return left; } /* @@ -4896,7 +4903,8 @@ int ceph_osdc_notify(struct ceph_osd_client *osdc, linger_submit(lreq); ret = linger_reg_commit_wait(lreq); if (!ret) - ret = linger_notify_finish_wait(lreq); + ret = linger_notify_finish_wait(lreq, + msecs_to_jiffies(2 * timeout * MSEC_PER_SEC)); else dout("lreq %p failed to initiate notify %d\n", lreq, ret);
If the cluster becomes unavailable, ceph_osdc_notify() may hang even with osd_request_timeout option set because linger_notify_finish_wait() waits for MWatchNotify NOTIFY_COMPLETE message with no associated OSD request in flight -- it's completely asynchronous. Introduce an additional timeout, derived from the specified notify timeout. While at it, switch both waits to killable which is more correct. Cc: stable@vger.kernel.org Signed-off-by: Ilya Dryomov <idryomov@gmail.com> --- net/ceph/osd_client.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)