[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250502094313.000055d1@huawei.com>
Date: Fri, 2 May 2025 09:43:13 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Erick Karanja <karanja99erick@...il.com>
CC: <manivannan.sadhasivam@...aro.org>, <kw@...ux.com>, <kishon@...nel.org>,
<bhelgaas@...gle.com>, <linux-pci@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <julia.lawall@...ia.fr>
Subject: Re: [PATCH 2/2] PCI: endpoint: Use scoped_guard for manual mutex
lock/unlock
On Thu, 1 May 2025 18:56:12 +0300
Erick Karanja <karanja99erick@...il.com> wrote:
> This refactor replaces manual mutex lock/unlock with scoped_guard()
> in places where early exits use goto. Using scoped_guard()
> avoids error-prone unlock paths and simplifies control flow.
>
> Signed-off-by: Erick Karanja <karanja99erick@...il.com>
> ---
> drivers/pci/endpoint/pci-epc-core.c | 53 +++++++++++++----------------
> 1 file changed, 24 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index beabea00af91..3f3ff36fa8ab 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -709,7 +709,6 @@ int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf,
> {
> struct list_head *list;
> u32 func_no;
> - int ret = 0;
>
> if (IS_ERR_OR_NULL(epc) || epf->is_vf)
> return -EINVAL;
> @@ -720,36 +719,32 @@ int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf,
> if (type == SECONDARY_INTERFACE && epf->sec_epc)
> return -EBUSY;
>
> - mutex_lock(&epc->list_lock);
> - func_no = find_first_zero_bit(&epc->function_num_map,
> - BITS_PER_LONG);
> - if (func_no >= BITS_PER_LONG) {
> - ret = -EINVAL;
> - goto ret;
> - }
> -
> - if (func_no > epc->max_functions - 1) {
> - dev_err(&epc->dev, "Exceeding max supported Function Number\n");
> - ret = -EINVAL;
> - goto ret;
> + scoped_guard(mutex, &epc->list_lock) {
This one is better, but using
guard(mutex)(&epc->list_lock);
Is going to make for an easier to read patch and lower indent etc.
Unless there is some subsystem related reason that scoped_guard() is
preferred then I'd go that way.
> + func_no = find_first_zero_bit(&epc->function_num_map,
> + BITS_PER_LONG);
> + if (func_no >= BITS_PER_LONG)
> + return -EINVAL;
> +
> + if (func_no > epc->max_functions - 1) {
> + dev_err(&epc->dev, "Exceeding max supported Function Number\n");
> + return -EINVAL;
> + }
> +
> + set_bit(func_no, &epc->function_num_map);
> + if (type == PRIMARY_INTERFACE) {
> + epf->func_no = func_no;
> + epf->epc = epc;
> + list = &epf->list;
> + } else {
> + epf->sec_epc_func_no = func_no;
> + epf->sec_epc = epc;
> + list = &epf->sec_epc_list;
> + }
> +
> + list_add_tail(list, &epc->pci_epf);
> }
>
> - set_bit(func_no, &epc->function_num_map);
> - if (type == PRIMARY_INTERFACE) {
> - epf->func_no = func_no;
> - epf->epc = epc;
> - list = &epf->list;
> - } else {
> - epf->sec_epc_func_no = func_no;
> - epf->sec_epc = epc;
> - list = &epf->sec_epc_list;
> - }
> -
> - list_add_tail(list, &epc->pci_epf);
> -ret:
> - mutex_unlock(&epc->list_lock);
> -
> - return ret;
> + return 0;
> }
> EXPORT_SYMBOL_GPL(pci_epc_add_epf);
>
Powered by blists - more mailing lists