Re: [PATCH v3 1/6] xen: add reference counter support

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Julien Grall <julien@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [PATCH v3 1/6] xen: add reference counter support
Date: Thu, 16 Mar 2023 17:21:31 +0100	[thread overview]
Message-ID: <ZBNCCyzjLlPl3Su4@Air-de-Roger> (raw)
In-Reply-To: <0be85ad4-f0d9-a5c7-e7bc-c546558df835@suse.com>

On Thu, Mar 16, 2023 at 03:03:42PM +0100, Jan Beulich wrote:
> On 16.03.2023 14:54, Roger Pau Monné wrote:
> > On Tue, Mar 14, 2023 at 08:56:29PM +0000, Volodymyr Babchuk wrote:
> >> +{
> >> +    return atomic_read(&refcnt->refcnt);
> >> +}
> >> +
> >> +static inline void refcnt_get(refcnt_t *refcnt)
> >> +{
> >> +    int old = atomic_add_unless(&refcnt->refcnt, 1, 0);
> >> +
> >> +    if ( unlikely(old < 0) || unlikely (old + 1 < 0) )
> >                                          ^ extra space
> > 
> > You want a single unlikely for both conditions.
> 
> Are you sure? My experience was generally the other way around: likely()
> and unlikely() become ineffectual as soon as the compiler needs more
> than one branch for the inner construct (ie generally for and && or ||).

Oh, OK, never mind then. We have examples of both in the code base.

Roger.