Date:   Sun, 14 Nov 2021 05:24:18 +0100
From:   Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>
To:     Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc:     bhelgaas@google.com, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] PCI/P2PDMA: Save a few cycles in 'pci_alloc_p2pmem()'
Message-ID: <YZCPcusrcCIJvYdn@rocinante>
References: <ab80164f4d5b32f9e6240aa4863c3a147ff9c89f.1635974126.git.christophe.jaillet@wanadoo.fr>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ab80164f4d5b32f9e6240aa4863c3a147ff9c89f.1635974126.git.christophe.jaillet@wanadoo.fr>
Precedence: bulk

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 <kw@linux.com>

	Krzysztof