lkml.org 
[lkml]   [2022]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 3/5] PCI: endpoint: Use a separate lock for protecting epc->pci_epf list
From
Date


On 06/10/22 7:19 pm, Manivannan Sadhasivam wrote:
> The EPC controller maintains a list of EPF drivers added to it. For
> protecting this list against the concurrent accesses, the epc->lock
> (used for protecting epc_ops) has been used so far. Since there were
> no users trying to use epc_ops and modify the pci_epf list simultaneously,
> this was not an issue.
>
> But with the addition of callback mechanism for passing the events, this
> will be a problem. Because the pci_epf list needs to be iterated first
> for getting hold of the EPF driver and then the relevant event specific
> callback needs to be called for the driver.
>
> If the same epc->lock is used, then it will result in a deadlock scenario.
>
> For instance,
>
> ...
> mutex_lock(&epc->lock);
> list_for_each_entry(epf, &epc->pci_epf, list) {
> epf->event_ops->core_init(epf);
> |
> |-> pci_epc_set_bar();
> |
> |-> mutex_lock(&epc->lock) # DEADLOCK
> ...
>
> So to fix this issue, use a separate lock called "list_lock" for
> protecting the pci_epf list against the concurrent accesses. This lock
> will also be used by the callback mechanism.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/pci/endpoint/pci-epc-core.c | 9 +++++----
> include/linux/pci-epc.h | 2 ++
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 3bc9273d0a08..6cce430d431b 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -613,7 +613,7 @@ 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->lock);
> + mutex_lock(&epc->list_lock);
> func_no = find_first_zero_bit(&epc->function_num_map,
> BITS_PER_LONG);
> if (func_no >= BITS_PER_LONG) {
> @@ -640,7 +640,7 @@ int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf,
>
> list_add_tail(list, &epc->pci_epf);
> ret:
> - mutex_unlock(&epc->lock);
> + mutex_unlock(&epc->list_lock);
>
> return ret;
> }
> @@ -672,11 +672,11 @@ void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf,
> list = &epf->sec_epc_list;
> }
>
> - mutex_lock(&epc->lock);
> + mutex_lock(&epc->list_lock);
> clear_bit(func_no, &epc->function_num_map);
> list_del(list);
> epf->epc = NULL;
> - mutex_unlock(&epc->lock);
> + mutex_unlock(&epc->list_lock);
> }
> EXPORT_SYMBOL_GPL(pci_epc_remove_epf);
>
> @@ -773,6 +773,7 @@ __pci_epc_create(struct device *dev, const struct pci_epc_ops *ops,
> }
>
> mutex_init(&epc->lock);
> + mutex_init(&epc->list_lock);
> INIT_LIST_HEAD(&epc->pci_epf);
> ATOMIC_INIT_NOTIFIER_HEAD(&epc->notifier);
>
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index a48778e1a4ee..fe729dfe509b 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -122,6 +122,7 @@ struct pci_epc_mem {
> * struct pci_epc - represents the PCI EPC device
> * @dev: PCI EPC device
> * @pci_epf: list of endpoint functions present in this EPC device
> + * list_lock: Mutex for protecting pci_epf list
> * @ops: function pointers for performing endpoint operations
> * @windows: array of address space of the endpoint controller
> * @mem: first window of the endpoint controller, which corresponds to
> @@ -139,6 +140,7 @@ struct pci_epc_mem {
> struct pci_epc {
> struct device dev;
> struct list_head pci_epf;
> + struct mutex list_lock;
> const struct pci_epc_ops *ops;
> struct pci_epc_mem **windows;
> struct pci_epc_mem *mem;
>

\
 
 \ /
  Last update: 2022-10-11 14:41    [W:0.139 / U:0.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site