Re: [RFC PATCH v2 3/4] cxl: Abort background operation in case of timeout

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Wed, 16 Oct 2024 05:00:00 +0000
Ravis OpenSrc <Ravis.OpenSrc@micron.com> wrote:

> Adding support for aborting timed out background operations
> 
> CXL r3.1 8.2.9.1.5 Request Abort Background Operation.
> 
> If the status of a mailbox command is identified as timedout,
> an abort background operation request is sent to the device.
> 
> Link:
> https://lore.kernel.org/linux-cxl/66035c2e8ba17_770232948b@dwillia2-xfh.jf.intel.com.notmuch/
> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Ajay Joshi <ajay.opensrc@micron.com>
> Signed-off-by: Ravi Shankar <ravis.opensrc@micron.com>

I was kind of expecting that we'd do this more on an ondemand
basis. So if nothing is going on, the command can have ages.
If the kernel wants to access the device and it has had a
reasonable amount of time we abort.

I'm not against this simpler approach though if it turns out
to be good enough for likely use cases.

My worry is error paths crossing with a slow command where
we want to find out what went wrong as quick as possible
Is 5 seconds ok for that?  Maybe not.

Jonathan

> ---
>  drivers/cxl/cxlmem.h |  1 +
>  drivers/cxl/pci.c    | 11 +++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index d8c0894797ac..808fb8712145 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -516,6 +516,7 @@ to_cxl_memdev_state(struct cxl_dev_state *cxlds)
>  enum cxl_opcode {
>          CXL_MBOX_OP_INVALID             = 0x0000,
>          CXL_MBOX_OP_RAW                 = CXL_MBOX_OP_INVALID,
> +       CXL_MBOX_OP_REQ_ABRT_BACKGROUND_OPERATION    = 0x0005,
>          CXL_MBOX_OP_GET_EVENT_RECORD    = 0x0100,
>          CXL_MBOX_OP_CLEAR_EVENT_RECORD  = 0x0101,
>          CXL_MBOX_OP_GET_EVT_INT_POLICY  = 0x0102,
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index d5d6142f6aa3..95c1f329bca2 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -394,6 +394,17 @@ static int cxl_pci_mbox_send(struct cxl_mailbox *cxl_mbox,
>  
>          mutex_lock_io(&cxl_mbox->mbox_mutex);
>          rc = __cxl_pci_mbox_send_cmd(cxl_mbox, cmd);
> +       if (rc == -ETIMEDOUT &&
> +               cmd->return_code == CXL_MBOX_CMD_RC_BACKGROUND) {

align cmd just after ( - that is c is under the r of rc
After fixing the tabs thing.

> +               struct cxl_mbox_cmd abort_cmd = {
> +                       .opcode = CXL_MBOX_OP_REQ_ABRT_BACKGROUND_OPERATION
> +               };
> +
> +               rc = __cxl_pci_mbox_send_cmd(cxl_mbox, &abort_cmd);
> +               if (!rc)
> +                       rc = -ECANCELED;
> +       }
> +
>          mutex_unlock(&cxl_mbox->mbox_mutex);
>  
>          return rc;