use guard(mutex) for scope based resource management of mutex.
This would make the code simpler and easier to maintain.
More details on lock guards can be found at
https://lore.kernel.org/all/20230612093537.614161713@infradead.org/T/#u
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
arch/powerpc/platforms/powernv/ocxl.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
index 64a9c7125c29..f8139948348e 100644
--- a/arch/powerpc/platforms/powernv/ocxl.c
+++ b/arch/powerpc/platforms/powernv/ocxl.c
@@ -172,12 +172,11 @@ static void pnv_ocxl_fixup_actag(struct pci_dev *dev)
if (phb->type != PNV_PHB_NPU_OCAPI)
return;
- mutex_lock(&links_list_lock);
+ guard(mutex)(&links_list_lock);
link = find_link(dev);
if (!link) {
dev_warn(&dev->dev, "couldn't update actag information\n");
- mutex_unlock(&links_list_lock);
return;
}
@@ -206,7 +205,6 @@ static void pnv_ocxl_fixup_actag(struct pci_dev *dev)
dev_dbg(&dev->dev, "total actags for function: %d\n",
link->fn_desired_actags[PCI_FUNC(dev->devfn)]);
- mutex_unlock(&links_list_lock);
}
DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pnv_ocxl_fixup_actag);
@@ -253,12 +251,11 @@ int pnv_ocxl_get_actag(struct pci_dev *dev, u16 *base, u16 *enabled,
{
struct npu_link *link;
- mutex_lock(&links_list_lock);
+ guard(mutex)(&links_list_lock);
link = find_link(dev);
if (!link) {
dev_err(&dev->dev, "actag information not found\n");
- mutex_unlock(&links_list_lock);
return -ENODEV;
}
/*
@@ -274,7 +271,6 @@ int pnv_ocxl_get_actag(struct pci_dev *dev, u16 *base, u16 *enabled,
*enabled = link->fn_actags[PCI_FUNC(dev->devfn)].count;
*supported = link->fn_desired_actags[PCI_FUNC(dev->devfn)];
- mutex_unlock(&links_list_lock);
return 0;
}
EXPORT_SYMBOL_GPL(pnv_ocxl_get_actag);
@@ -293,12 +289,11 @@ int pnv_ocxl_get_pasid_count(struct pci_dev *dev, int *count)
*
* We only support one AFU-carrying function for now.
*/
- mutex_lock(&links_list_lock);
+ guard(mutex)(&links_list_lock);
link = find_link(dev);
if (!link) {
dev_err(&dev->dev, "actag information not found\n");
- mutex_unlock(&links_list_lock);
return -ENODEV;
}
@@ -309,7 +304,6 @@ int pnv_ocxl_get_pasid_count(struct pci_dev *dev, int *count)
break;
}
- mutex_unlock(&links_list_lock);
dev_dbg(&dev->dev, "%d PASIDs available for function\n",
rc ? 0 : *count);
return rc;
--
2.39.3
On 3/14/25 11:15, Shrikanth Hegde wrote:
> use guard(mutex) for scope based resource management of mutex.
> This would make the code simpler and easier to maintain.
>
> More details on lock guards can be found at
> https://lore.kernel.org/all/20230612093537.614161713@infradead.org/T/#u
>
> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> ---
> arch/powerpc/platforms/powernv/ocxl.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
> index 64a9c7125c29..f8139948348e 100644
> --- a/arch/powerpc/platforms/powernv/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/ocxl.c
> @@ -172,12 +172,11 @@ static void pnv_ocxl_fixup_actag(struct pci_dev *dev)
> if (phb->type != PNV_PHB_NPU_OCAPI)
> return;
>
> - mutex_lock(&links_list_lock);
> + guard(mutex)(&links_list_lock);
>
> link = find_link(dev);
> if (!link) {
> dev_warn(&dev->dev, "couldn't update actag information\n");
> - mutex_unlock(&links_list_lock);
> return;
> }
>
> @@ -206,7 +205,6 @@ static void pnv_ocxl_fixup_actag(struct pci_dev *dev)
> dev_dbg(&dev->dev, "total actags for function: %d\n",
> link->fn_desired_actags[PCI_FUNC(dev->devfn)]);
>
> - mutex_unlock(&links_list_lock);
> }
> DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pnv_ocxl_fixup_actag);
>
> @@ -253,12 +251,11 @@ int pnv_ocxl_get_actag(struct pci_dev *dev, u16 *base, u16 *enabled,
> {
> struct npu_link *link;
>
> - mutex_lock(&links_list_lock);
> + guard(mutex)(&links_list_lock);
>
> link = find_link(dev);
> if (!link) {
> dev_err(&dev->dev, "actag information not found\n");
> - mutex_unlock(&links_list_lock);
> return -ENODEV;
> }
> /*
> @@ -274,7 +271,6 @@ int pnv_ocxl_get_actag(struct pci_dev *dev, u16 *base, u16 *enabled,
> *enabled = link->fn_actags[PCI_FUNC(dev->devfn)].count;
> *supported = link->fn_desired_actags[PCI_FUNC(dev->devfn)];
>
> - mutex_unlock(&links_list_lock);
> return 0;
> }
> EXPORT_SYMBOL_GPL(pnv_ocxl_get_actag);
> @@ -293,12 +289,11 @@ int pnv_ocxl_get_pasid_count(struct pci_dev *dev, int *count)
> *
> * We only support one AFU-carrying function for now.
> */
> - mutex_lock(&links_list_lock);
> + guard(mutex)(&links_list_lock);
>
> link = find_link(dev);
> if (!link) {
> dev_err(&dev->dev, "actag information not found\n");
> - mutex_unlock(&links_list_lock);
> return -ENODEV;
> }
>
> @@ -309,7 +304,6 @@ int pnv_ocxl_get_pasid_count(struct pci_dev *dev, int *count)
> break;
> }
>
> - mutex_unlock(&links_list_lock);
Hi. Andrew,
After this change below dev_dbg will be called with mutex held still. Is
that a concern? I don't see the mutex being used in that path.
Since using scoped_guard cause more code churn here, I would prefer not
use it.
> dev_dbg(&dev->dev, "%d PASIDs available for function\n",
> rc ? 0 : *count);
> return rc;
On Fri, 2025-03-14 at 15:00 +0530, Shrikanth Hegde wrote: > > Hi. Andrew, > > After this change below dev_dbg will be called with mutex held still. > Is > that a concern? I don't see the mutex being used in that path. > > Since using scoped_guard cause more code churn here, I would prefer > not > use it. I think this is fine. -- Andrew Donnellan OzLabs, ADL Canberra ajd@linux.ibm.com IBM Australia Limited
On 3/14/25 15:00, Shrikanth Hegde wrote:
>
>
> On 3/14/25 11:15, Shrikanth Hegde wrote:
>> use guard(mutex) for scope based resource management of mutex.
>> This would make the code simpler and easier to maintain.
>>
>> More details on lock guards can be found at
>> https://lore.kernel.org/all/20230612093537.614161713@infradead.org/T/#u
>>
>> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
>> ---
>> arch/powerpc/platforms/powernv/ocxl.c | 12 +++---------
>> 1 file changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/
>> platforms/powernv/ocxl.c
>> index 64a9c7125c29..f8139948348e 100644
>> --- a/arch/powerpc/platforms/powernv/ocxl.c
>> +++ b/arch/powerpc/platforms/powernv/ocxl.c
>> @@ -172,12 +172,11 @@ static void pnv_ocxl_fixup_actag(struct pci_dev
>> *dev)
>> if (phb->type != PNV_PHB_NPU_OCAPI)
>> return;
>> - mutex_lock(&links_list_lock);
>> + guard(mutex)(&links_list_lock);
>> link = find_link(dev);
>> if (!link) {
>> dev_warn(&dev->dev, "couldn't update actag information\n");
>> - mutex_unlock(&links_list_lock);
>> return;
>> }
>> @@ -206,7 +205,6 @@ static void pnv_ocxl_fixup_actag(struct pci_dev *dev)
>> dev_dbg(&dev->dev, "total actags for function: %d\n",
>> link->fn_desired_actags[PCI_FUNC(dev->devfn)]);
>> - mutex_unlock(&links_list_lock);
>> }
>> DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pnv_ocxl_fixup_actag);
>> @@ -253,12 +251,11 @@ int pnv_ocxl_get_actag(struct pci_dev *dev, u16
>> *base, u16 *enabled,
>> {
>> struct npu_link *link;
>> - mutex_lock(&links_list_lock);
>> + guard(mutex)(&links_list_lock);
>> link = find_link(dev);
>> if (!link) {
>> dev_err(&dev->dev, "actag information not found\n");
>> - mutex_unlock(&links_list_lock);
>> return -ENODEV;
>> }
>> /*
>> @@ -274,7 +271,6 @@ int pnv_ocxl_get_actag(struct pci_dev *dev, u16
>> *base, u16 *enabled,
>> *enabled = link->fn_actags[PCI_FUNC(dev->devfn)].count;
>> *supported = link->fn_desired_actags[PCI_FUNC(dev->devfn)];
>> - mutex_unlock(&links_list_lock);
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(pnv_ocxl_get_actag);
>> @@ -293,12 +289,11 @@ int pnv_ocxl_get_pasid_count(struct pci_dev
>> *dev, int *count)
>> *
>> * We only support one AFU-carrying function for now.
>> */
>> - mutex_lock(&links_list_lock);
>> + guard(mutex)(&links_list_lock);
>> link = find_link(dev);
>> if (!link) {
>> dev_err(&dev->dev, "actag information not found\n");
>> - mutex_unlock(&links_list_lock);
>> return -ENODEV;
>> }
>> @@ -309,7 +304,6 @@ int pnv_ocxl_get_pasid_count(struct pci_dev *dev,
>> int *count)
>> break;
>> }
>> - mutex_unlock(&links_list_lock);
>
> Hi. Andrew,
>
> After this change below dev_dbg will be called with mutex held still. Is
> that a concern? I don't see the mutex being used in that path.
>
> Since using scoped_guard cause more code churn here, I would prefer not
> use it.
I see current code in pnv_ocxl_fixup_actag calls dev_dbg with mutex
held. So likely not a concern of using just guard in
pnv_ocxl_get_pasid_count as well.
Assuming that, let me send out v2 with corrected commit subject. :w
>
>> dev_dbg(&dev->dev, "%d PASIDs available for function\n",
>> rc ? 0 : *count);
>> return rc;
>
On Fri, 2025-03-14 at 11:15 +0530, Shrikanth Hegde wrote: > use guard(mutex) for scope based resource management of mutex. > This would make the code simpler and easier to maintain. > > More details on lock guards can be found at > https://lore.kernel.org/all/20230612093537.614161713@infradead.org/T/#u > > Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com> The subject line of this patch misspells powernv and ocxl. Otherwise this looks like a nice cleanup. -- Andrew Donnellan OzLabs, ADL Canberra ajd@linux.ibm.com IBM Australia Limited
On 3/14/25 11:36, Andrew Donnellan wrote: > On Fri, 2025-03-14 at 11:15 +0530, Shrikanth Hegde wrote: >> use guard(mutex) for scope based resource management of mutex. >> This would make the code simpler and easier to maintain. >> >> More details on lock guards can be found at >> https://lore.kernel.org/all/20230612093537.614161713@infradead.org/T/#u >> >> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com> > > The subject line of this patch misspells powernv and ocxl. Ah. my bad. will correct it. > > Otherwise this looks like a nice cleanup. Thanks.
© 2016 - 2025 Red Hat, Inc.