[PATCH] cxl/pci: Replace mutex_lock_io() w mutex_lock() for mailbox access

[PATCH] cxl/pci: Replace mutex_lock_io() w mutex_lock() for mailbox access

[alison.schofield]

From: Alison Schofield <alison.schofield@intel.com>

mutex_lock_io() differs from mutex_lock() in that it may call
io_schedule() when a task must sleep waiting for the lock. This
distinction only makes sense in block I/O or memory reclaim paths,
where giving I/O a chance to make progress is useful.

At this call site, cxl_pci_mbox_send(), the mutex protects an MMIO
mailbox. The task holding the lock is not blocking I/O progress, so
calling io_schedule(), as mutex_lock_io() may do, has no practical
effect.

Although there is no functional change, using the correct locking
primitive, that more accurately reflects the semantics and intended
use of the lock, improves code clarity and avoids misleading readers
and tools.

Reported-by: Alok Tiwari <alok.a.tiwari@oracle.com>
Closes: https://lore.kernel.org/linux-cxl/0d2af1e8-7f1b-438c-a090-fd366c8c63e0@oracle.com/
Suggested-by: Dan Williams <dan.j.williams@intel.com>
Fixes: 8adaf747c9f0 ("cxl/mem: Find device capabilities")
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 drivers/cxl/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 785aa2af5eaa..bd100ac31672 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -379,7 +379,7 @@ static int cxl_pci_mbox_send(struct cxl_mailbox *cxl_mbox,
 {
 	int rc;
 
-	mutex_lock_io(&cxl_mbox->mbox_mutex);
+	mutex_lock(&cxl_mbox->mbox_mutex);
 	rc = __cxl_pci_mbox_send_cmd(cxl_mbox, cmd);
 	mutex_unlock(&cxl_mbox->mbox_mutex);
 

base-commit: 0ff41df1cb268fc69e703a08a57ee14ae967d0ca
-- 
2.37.3

Re: [PATCH] cxl/pci: Replace mutex_lock_io() w mutex_lock() for mailbox access

[Dan Williams]

alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> mutex_lock_io() differs from mutex_lock() in that it may call
> io_schedule() when a task must sleep waiting for the lock. This
> distinction only makes sense in block I/O or memory reclaim paths,
> where giving I/O a chance to make progress is useful.
> 
> At this call site, cxl_pci_mbox_send(), the mutex protects an MMIO
> mailbox. The task holding the lock is not blocking I/O progress, so
> calling io_schedule(), as mutex_lock_io() may do, has no practical
> effect.
> 
> Although there is no functional change, using the correct locking
> primitive, that more accurately reflects the semantics and intended
> use of the lock, improves code clarity and avoids misleading readers
> and tools.
> 
> Reported-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> Closes: https://lore.kernel.org/linux-cxl/0d2af1e8-7f1b-438c-a090-fd366c8c63e0@oracle.com/
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Fixes: 8adaf747c9f0 ("cxl/mem: Find device capabilities")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Looks good to me, I would suggest that Dave drop the Fixes: tag on applying
since there is no need to backport this.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

Re: [PATCH] cxl/pci: Replace mutex_lock_io() w mutex_lock() for mailbox access

[Davidlohr Bueso]

On Thu, 29 May 2025, alison.schofield@intel.com wrote:

>From: Alison Schofield <alison.schofield@intel.com>
>
>mutex_lock_io() differs from mutex_lock() in that it may call
>io_schedule() when a task must sleep waiting for the lock. This
>distinction only makes sense in block I/O or memory reclaim paths,
>where giving I/O a chance to make progress is useful.
>
>At this call site, cxl_pci_mbox_send(), the mutex protects an MMIO
>mailbox. The task holding the lock is not blocking I/O progress, so
>calling io_schedule(), as mutex_lock_io() may do, has no practical
>effect.
>
>Although there is no functional change, using the correct locking
>primitive, that more accurately reflects the semantics and intended
>use of the lock, improves code clarity and avoids misleading readers
>and tools.
>
>Reported-by: Alok Tiwari <alok.a.tiwari@oracle.com>
>Closes: https://lore.kernel.org/linux-cxl/0d2af1e8-7f1b-438c-a090-fd366c8c63e0@oracle.com/
>Suggested-by: Dan Williams <dan.j.williams@intel.com>
>Fixes: 8adaf747c9f0 ("cxl/mem: Find device capabilities")
>Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Yes, please.

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

Re: [PATCH] cxl/pci: Replace mutex_lock_io() w mutex_lock() for mailbox access

[Dave Jiang]

On 5/29/25 1:51 PM, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> mutex_lock_io() differs from mutex_lock() in that it may call
> io_schedule() when a task must sleep waiting for the lock. This
> distinction only makes sense in block I/O or memory reclaim paths,
> where giving I/O a chance to make progress is useful.
> 
> At this call site, cxl_pci_mbox_send(), the mutex protects an MMIO
> mailbox. The task holding the lock is not blocking I/O progress, so
> calling io_schedule(), as mutex_lock_io() may do, has no practical
> effect.
> 
> Although there is no functional change, using the correct locking
> primitive, that more accurately reflects the semantics and intended
> use of the lock, improves code clarity and avoids misleading readers
> and tools.
> 
> Reported-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> Closes: https://lore.kernel.org/linux-cxl/0d2af1e8-7f1b-438c-a090-fd366c8c63e0@oracle.com/
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Fixes: 8adaf747c9f0 ("cxl/mem: Find device capabilities")
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Applied to cxl/next with Fixes tag dropped.

> ---
>  drivers/cxl/pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 785aa2af5eaa..bd100ac31672 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -379,7 +379,7 @@ static int cxl_pci_mbox_send(struct cxl_mailbox *cxl_mbox,
>  {
>  	int rc;
>  
> -	mutex_lock_io(&cxl_mbox->mbox_mutex);
> +	mutex_lock(&cxl_mbox->mbox_mutex);
>  	rc = __cxl_pci_mbox_send_cmd(cxl_mbox, cmd);
>  	mutex_unlock(&cxl_mbox->mbox_mutex);
>  
> 
> base-commit: 0ff41df1cb268fc69e703a08a57ee14ae967d0ca