[PATCH] libceph: fix misleading ceph_osdc_cancel_request() comment

[PATCH] libceph: fix misleading ceph_osdc_cancel_request() comment

[Ilya Dryomov]

cancel_request() never guaranteed that after its return the OSD
client would be completely done with the OSD request.  The callback
(if specified) can still be invoked and a ref can still be held.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
 net/ceph/osd_client.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
index 4b88f2a4a6e2..9d82bb42e958 100644
--- a/net/ceph/osd_client.c
+++ b/net/ceph/osd_client.c
@@ -4591,8 +4591,13 @@ 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:
- * ->r_result isn't set and __complete_request() isn't called.
+ * Unregister request.  If @req was registered, it isn't completed:
+ * r_result isn't set and __complete_request() isn't invoked.
+ *
+ * If @req wasn't registered, this call may have raced with
+ * handle_reply(), in which case r_result would already be set and
+ * __complete_request() would be getting invoked, possibly even
+ * concurrently with this call.
  */
 void ceph_osdc_cancel_request(struct ceph_osd_request *req)
 {
-- 
2.19.2

Re: [PATCH] libceph: fix misleading ceph_osdc_cancel_request() comment

[Xiubo Li]

On 5/17/22 5:55 PM, Ilya Dryomov wrote:
> cancel_request() never guaranteed that after its return the OSD
> client would be completely done with the OSD request.  The callback
> (if specified) can still be invoked and a ref can still be held.
>
> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
> ---
>   net/ceph/osd_client.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
> index 4b88f2a4a6e2..9d82bb42e958 100644
> --- a/net/ceph/osd_client.c
> +++ b/net/ceph/osd_client.c
> @@ -4591,8 +4591,13 @@ 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:
> - * ->r_result isn't set and __complete_request() isn't called.
> + * Unregister request.  If @req was registered, it isn't completed:
> + * r_result isn't set and __complete_request() isn't invoked.
> + *
> + * If @req wasn't registered, this call may have raced with
> + * handle_reply(), in which case r_result would already be set and
> + * __complete_request() would be getting invoked, possibly even
> + * concurrently with this call.
>    */
>   void ceph_osdc_cancel_request(struct ceph_osd_request *req)
>   {

Reviewed-by: Xiubo Li <xiubli@redhat.com>

Re: [PATCH] libceph: fix misleading ceph_osdc_cancel_request() comment

[Jeff Layton]

On Tue, 2022-05-17 at 11:55 +0200, Ilya Dryomov wrote:
> cancel_request() never guaranteed that after its return the OSD
> client would be completely done with the OSD request.  The callback
> (if specified) can still be invoked and a ref can still be held.
> 
> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
> ---
>  net/ceph/osd_client.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
> index 4b88f2a4a6e2..9d82bb42e958 100644
> --- a/net/ceph/osd_client.c
> +++ b/net/ceph/osd_client.c
> @@ -4591,8 +4591,13 @@ 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:
> - * ->r_result isn't set and __complete_request() isn't called.
> + * Unregister request.  If @req was registered, it isn't completed:
> + * r_result isn't set and __complete_request() isn't invoked.
> + *
> + * If @req wasn't registered, this call may have raced with
> + * handle_reply(), in which case r_result would already be set and
> + * __complete_request() would be getting invoked, possibly even
> + * concurrently with this call.
>   */
>  void ceph_osdc_cancel_request(struct ceph_osd_request *req)
>  {

Reviewed-by: Jeff Layton <jlayton@kernel.org>