Re: [RFC PATCH 4.19.y-cip 24/50] PCI: endpoint: Replace spinlock with mutex


Lad Prabhakar
 

Hi Pavel,

-----Original Message-----
From: Pavel Machek <pavel@...>
Sent: 20 October 2020 21:44
To: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...>
Cc: cip-dev@...; Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@...>; Pavel Machek <pavel@...>; Biju Das
<biju.das.jz@...>
Subject: Re: [RFC PATCH 4.19.y-cip 24/50] PCI: endpoint: Replace spinlock with mutex

Hi!

commit 3d3248dbd018502f654064c78efcd2e165ab3486 upstream.

The pci_epc_ops is not intended to be invoked from interrupt context.
Hence replace spin_lock_irqsave and spin_unlock_irqrestore with
mutex_lock and mutex_unlock respectively.
Could I get some kind of explanation why this is good idea?
Apart of one mentioned above other point I would add is on a single core machine mutex_lock/unlock would be good choice.

Also to add the callbacks in controller driver might sleep. For example in raise_irq callback [1], [2].

As long as code protected by the locks does not sleep, spinlocks are
okay... (but they should not need "_irqsave" variants).

They are likely to have better performance, too, when protected code
is small and fast.
I do agree with the above two points *if the code isn't sleeping*.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/cip/linux-cip.git/tree/drivers/pci/controller/pcie-rockchip-ep.c?h=linux-4.19.y-cip#n410
[2] https://git.kernel.org/pub/scm/linux/kernel/git/cip/linux-cip.git/tree/drivers/pci/controller/pcie-cadence-ep.c?h=linux-4.19.y-cip#n310

Cheers,
Prabhakar

Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Join cip-dev@lists.cip-project.org to automatically receive all group messages.