On 06/25/2014 12:16 PM, Ilya Dryomov wrote:
> Introduce ceph_osdc_cancel_request() intended for canceling requests
> from the higher layers (rbd and cephfs).  Because higher layers are in
> charge and are supposed to know what and when they are canceling, the
> request is not completed, only unref'ed and removed from the libceph
> data structures.

This seems reasonable.

But you make two changes here that I'd like to see a little
more explanation on.  They seem significant enough to warrant
a little more attention in the commit description.

- __cancel_request() is no longer called when
  ceph_osdc_wait_request() fails due to an
  an interrupt.  This is my main concern, and I
  plan to work through it but I'm in a small hurry
  right now.
- __unregister_linger_request() is now called when
  a request is canceled, but it wasn't before.  (Since
  we're consistent about setting the r_linger flag
  this should be fine, but it *is* a behavior change.)

I'm going to keep looking at this; I have about
20 minutes before I have to leave for a while.

                                        -Alex

> Signed-off-by: Ilya Dryomov <ilya.dryo...@inktank.com>
> ---
>  include/linux/ceph/osd_client.h |    1 +
>  net/ceph/osd_client.c           |   31 +++++++++++++++++++++++++------
>  2 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/ceph/osd_client.h b/include/linux/ceph/osd_client.h
> index a8d5652f589d..de09cad7b7c7 100644
> --- a/include/linux/ceph/osd_client.h
> +++ b/include/linux/ceph/osd_client.h
> @@ -334,6 +334,7 @@ extern void ceph_osdc_put_request(struct ceph_osd_request 
> *req);
>  extern int ceph_osdc_start_request(struct ceph_osd_client *osdc,
>                                  struct ceph_osd_request *req,
>                                  bool nofail);
> +extern void ceph_osdc_cancel_request(struct ceph_osd_request *req);
>  extern int ceph_osdc_wait_request(struct ceph_osd_client *osdc,
>                                 struct ceph_osd_request *req);
>  extern void ceph_osdc_sync(struct ceph_osd_client *osdc);
> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
> index 8104db24bc09..ef44cc0bbc6a 100644
> --- a/net/ceph/osd_client.c
> +++ b/net/ceph/osd_client.c
> @@ -2470,6 +2470,25 @@ int ceph_osdc_start_request(struct ceph_osd_client 
> *osdc,
>  EXPORT_SYMBOL(ceph_osdc_start_request);
>  
>  /*
> + * Unregister a registered request.  The request is not completed (i.e.
> + * no callbacks or wakeups) - higher layers are supposed to know what
> + * they are canceling.
> + */
> +void ceph_osdc_cancel_request(struct ceph_osd_request *req)
> +{
> +     struct ceph_osd_client *osdc = req->r_osdc;
> +
> +     mutex_lock(&osdc->request_mutex);
> +     if (req->r_linger)
> +             __unregister_linger_request(osdc, req);
> +     __unregister_request(osdc, req);
> +     mutex_unlock(&osdc->request_mutex);
> +
> +     dout("%s %p tid %llu canceled\n", __func__, req, req->r_tid);
> +}
> +EXPORT_SYMBOL(ceph_osdc_cancel_request);
> +
> +/*
>   * wait for a request to complete
>   */
>  int ceph_osdc_wait_request(struct ceph_osd_client *osdc,
> @@ -2477,18 +2496,18 @@ int ceph_osdc_wait_request(struct ceph_osd_client 
> *osdc,
>  {
>       int rc;
>  
> +     dout("%s %p tid %llu\n", __func__, req, req->r_tid);
> +
>       rc = wait_for_completion_interruptible(&req->r_completion);
>       if (rc < 0) {
> -             mutex_lock(&osdc->request_mutex);
> -             __cancel_request(req);
> -             __unregister_request(osdc, req);
> -             mutex_unlock(&osdc->request_mutex);
> +             dout("%s %p tid %llu interrupted\n", __func__, req, req->r_tid);
> +             ceph_osdc_cancel_request(req);
>               complete_request(req);
> -             dout("wait_request tid %llu canceled/timed out\n", req->r_tid);
>               return rc;
>       }
>  
> -     dout("wait_request tid %llu result %d\n", req->r_tid, req->r_result);
> +     dout("%s %p tid %llu result %d\n", __func__, req, req->r_tid,
> +          req->r_result);
>       return req->r_result;
>  }
>  EXPORT_SYMBOL(ceph_osdc_wait_request);
> 

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to