Use 'percpu_ref_tryget_live_rcu()' instead of 'percpu_ref_tryget_live()' to
save a few cycles when it is known that the rcu lock is already
taken/released.
Signed-off-by: Christophe JAILLET <[email protected]>
---
drivers/pci/p2pdma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index 8d47cb7218d1..081c391690d4 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -710,7 +710,7 @@ void *pci_alloc_p2pmem(struct pci_dev *pdev, size_t size)
if (!ret)
goto out;
- if (unlikely(!percpu_ref_tryget_live(ref))) {
+ if (unlikely(!percpu_ref_tryget_live_rcu(ref))) {
gen_pool_free(p2pdma->pool, (unsigned long) ret, size);
ret = NULL;
goto out;
--
2.30.2
Hi Christophe,
Thank you for another nice patch!
> Use 'percpu_ref_tryget_live_rcu()' instead of 'percpu_ref_tryget_live()' to
> save a few cycles when it is known that the rcu lock is already
> taken/released.
If possible, we should explain why are we using this new API, especially
since percpu_ref_tryget_live_rcu() is a relatively new addition [1], so
it's obvious that its users already manage the RCU lock accordingly, and
that there is no need to hold the RCU read lock again (which can in turn
lead to performance improvement), which is what the percpu_ref_tryget_live()
does internally.
What do you think?
1. 3b13c168186c ("percpu_ref: percpu_ref_tryget_live() version holding RCU")
> if (!ret)
> goto out;
>
> - if (unlikely(!percpu_ref_tryget_live(ref))) {
> + if (unlikely(!percpu_ref_tryget_live_rcu(ref))) {
> gen_pool_free(p2pdma->pool, (unsigned long) ret, size);
> ret = NULL;
> goto out;
Thank you!
Reviewed-by: Krzysztof Wilczyński <[email protected]>
Krzysztof
[+cc Logan, Eric]
On Wed, Nov 03, 2021 at 10:16:53PM +0100, Christophe JAILLET wrote:
> Use 'percpu_ref_tryget_live_rcu()' instead of 'percpu_ref_tryget_live()' to
> save a few cycles when it is known that the rcu lock is already
> taken/released.
>
> Signed-off-by: Christophe JAILLET <[email protected]>
Added Logan and Eric since Logan is the author and de facto maintainer
of this file and Eric recently converted this to RCU.
Maybe we need a MAINTAINERS entry for P2PDMA?
> ---
> drivers/pci/p2pdma.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 8d47cb7218d1..081c391690d4 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -710,7 +710,7 @@ void *pci_alloc_p2pmem(struct pci_dev *pdev, size_t size)
> if (!ret)
> goto out;
>
> - if (unlikely(!percpu_ref_tryget_live(ref))) {
> + if (unlikely(!percpu_ref_tryget_live_rcu(ref))) {
> gen_pool_free(p2pdma->pool, (unsigned long) ret, size);
> ret = NULL;
> goto out;
> --
> 2.30.2
>
On 2021-12-15 10:35 a.m., Bjorn Helgaas wrote:
> [+cc Logan, Eric]
>
> On Wed, Nov 03, 2021 at 10:16:53PM +0100, Christophe JAILLET wrote:
>> Use 'percpu_ref_tryget_live_rcu()' instead of 'percpu_ref_tryget_live()' to
>> save a few cycles when it is known that the rcu lock is already
>> taken/released.
>>
>> Signed-off-by: Christophe JAILLET <[email protected]>
>
> Added Logan and Eric since Logan is the author and de facto maintainer
> of this file and Eric recently converted this to RCU.
Looks fine to me:
Reviewed-by: Logan Gunthorpe <[email protected]>
> Maybe we need a MAINTAINERS entry for P2PDMA?
I'm not opposed to this. Would it be a duplicate of the PCI SUBSYSTEM
just with my name added as maintainer? I could send a patch if so.
Logan
On Wed, Dec 15, 2021 at 02:37:51PM -0700, Logan Gunthorpe wrote:
> On 2021-12-15 10:35 a.m., Bjorn Helgaas wrote:
> > Maybe we need a MAINTAINERS entry for P2PDMA?
>
> I'm not opposed to this. Would it be a duplicate of the PCI SUBSYSTEM
> just with my name added as maintainer? I could send a patch if so.
Maybe something like this? Are there other relevant files? I just
want to make sure that you see updates to p2pdma stuff.
diff --git a/MAINTAINERS b/MAINTAINERS
index 7a2345ce8521..3180160fcc28 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14717,6 +14717,20 @@ L: [email protected]
S: Supported
F: Documentation/PCI/pci-error-recovery.rst
+PCI PEER-TO-PEER DMA (P2PDMA)
+M: Bjorn Helgaas <[email protected]>
+M: Logan Gunthorpe <[email protected]>
+L: [email protected]
+S: Supported
+Q: https://patchwork.kernel.org/project/linux-pci/list/
+B: https://bugzilla.kernel.org
+C: irc://irc.oftc.net/linux-pci
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git
+F: Documentation/PCI/
+F: Documentation/devicetree/bindings/pci/
+F: drivers/pci/p2pdma.c
+F: include/linux/pci-p2pdma.h
+
PCI MSI DRIVER FOR ALTERA MSI IP
M: Joyce Ooi <[email protected]>
L: [email protected]
On 2021-12-15 2:47 p.m., Bjorn Helgaas wrote:
> On Wed, Dec 15, 2021 at 02:37:51PM -0700, Logan Gunthorpe wrote:
>> On 2021-12-15 10:35 a.m., Bjorn Helgaas wrote:
>>> Maybe we need a MAINTAINERS entry for P2PDMA?
>>
>> I'm not opposed to this. Would it be a duplicate of the PCI SUBSYSTEM
>> just with my name added as maintainer? I could send a patch if so.
>
> Maybe something like this? Are there other relevant files? I just
> want to make sure that you see updates to p2pdma stuff.
Largely looks good to me.
The one missing file is:
Documentation/driver-api/pci/p2pdma.rst
Do you want me to put that in a patch or will you handle it?
Thanks,
Logan
On Wed, Nov 03, 2021 at 10:16:53PM +0100, Christophe JAILLET wrote:
> Use 'percpu_ref_tryget_live_rcu()' instead of 'percpu_ref_tryget_live()' to
> save a few cycles when it is known that the rcu lock is already
> taken/released.
>
> Signed-off-by: Christophe JAILLET <[email protected]>
Applied to pci/p2pdma for v5.17, thanks!
PCI/P2PDMA: Use percpu_ref_tryget_live_rcu() inside RCU critical section
Since pci_alloc_p2pmem() has already called rcu_read_lock(), we're in an
RCU read-side critical section and don't need to take the lock again. Use
percpu_ref_tryget_live_rcu() instead of percpu_ref_tryget_live() to save a
few cycles.
> ---
> drivers/pci/p2pdma.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 8d47cb7218d1..081c391690d4 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -710,7 +710,7 @@ void *pci_alloc_p2pmem(struct pci_dev *pdev, size_t size)
> if (!ret)
> goto out;
>
> - if (unlikely(!percpu_ref_tryget_live(ref))) {
> + if (unlikely(!percpu_ref_tryget_live_rcu(ref))) {
> gen_pool_free(p2pdma->pool, (unsigned long) ret, size);
> ret = NULL;
> goto out;
> --
> 2.30.2
>
On Wed, Dec 15, 2021 at 02:51:51PM -0700, Logan Gunthorpe wrote:
>
>
> On 2021-12-15 2:47 p.m., Bjorn Helgaas wrote:
> > On Wed, Dec 15, 2021 at 02:37:51PM -0700, Logan Gunthorpe wrote:
> >> On 2021-12-15 10:35 a.m., Bjorn Helgaas wrote:
> >>> Maybe we need a MAINTAINERS entry for P2PDMA?
> >>
> >> I'm not opposed to this. Would it be a duplicate of the PCI SUBSYSTEM
> >> just with my name added as maintainer? I could send a patch if so.
> >
> > Maybe something like this? Are there other relevant files? I just
> > want to make sure that you see updates to p2pdma stuff.
>
> Largely looks good to me.
>
> The one missing file is:
>
> Documentation/driver-api/pci/p2pdma.rst
>
> Do you want me to put that in a patch or will you handle it?
I put the patch below on pci/p2pdma for v5.17, let me know if you want
any other tweaks.
I had mistakenly included these, which don't include any P2PDMA
content, so I dropped them so you don't get inundated with other
random changes:
+F: Documentation/PCI/
+F: Documentation/devicetree/bindings/pci/
commit bdebed96bd4d ("MAINTAINERS: Add Logan Gunthorpe as P2PDMA maintainer")
Author: Bjorn Helgaas <[email protected]>
Date: Wed Dec 15 15:43:04 2021 -0600
MAINTAINERS: Add Logan Gunthorpe as P2PDMA maintainer
Add a P2PDMA entry to make sure Logan is aware of changes to that area.
Signed-off-by: Bjorn Helgaas <[email protected]>
diff --git a/MAINTAINERS b/MAINTAINERS
index 7a2345ce8521..ea59e32e1e81 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14717,6 +14717,19 @@ L: [email protected]
S: Supported
F: Documentation/PCI/pci-error-recovery.rst
+PCI PEER-TO-PEER DMA (P2PDMA)
+M: Bjorn Helgaas <[email protected]>
+M: Logan Gunthorpe <[email protected]>
+L: [email protected]
+S: Supported
+Q: https://patchwork.kernel.org/project/linux-pci/list/
+B: https://bugzilla.kernel.org
+C: irc://irc.oftc.net/linux-pci
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git
+F: Documentation/driver-api/pci/p2pdma.rst
+F: drivers/pci/p2pdma.c
+F: include/linux/pci-p2pdma.h
+
PCI MSI DRIVER FOR ALTERA MSI IP
M: Joyce Ooi <[email protected]>
L: [email protected]
On 2021-12-15 3:04 p.m., Bjorn Helgaas wrote:
> On Wed, Dec 15, 2021 at 02:51:51PM -0700, Logan Gunthorpe wrote:
>> Largely looks good to me.
>>
>> The one missing file is:
>>
>> Documentation/driver-api/pci/p2pdma.rst
>>
>> Do you want me to put that in a patch or will you handle it?
>
> I put the patch below on pci/p2pdma for v5.17, let me know if you want
> any other tweaks.
>
> I had mistakenly included these, which don't include any P2PDMA
> content, so I dropped them so you don't get inundated with other
> random changes:
>
> +F: Documentation/PCI/
> +F: Documentation/devicetree/bindings/pci/
Yup, ok. Looks good to me. If you want or need my Acked-by or whatever,
you can add it:
Acked-by: Logan Gunthorpe <[email protected]>
Thanks,
Logan