Re: [Xen-devel] [PATCH] slightly consolidate code in free_domheap_pages()

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Tue, 2014-06-24 at 11:25 +0100, Jan Beulich wrote:
> >>> On 24.06.14 at 12:04, <Ian.Campbell@xxxxxxxxxxxxx> wrote:
> > On Fri, 2014-06-20 at 13:40 +0100, Jan Beulich wrote:
> > 
> >> +        if ( likely(d) && likely(d != dom_cow) )
> > 
> > OOI is this more or less efficient than a single likely around the
> > entire thing?
> 
> likely()/unlikely() around && or || expressions is never having the
> intended effect unless these expressions can be guaranteed to
> result in only a single branch instruction in the compiled code.
> That's because branch likelihood needs to be determined for each
> branch instruction individually (and e.g. likely(x && y) doesn't
> necessarily mean likely(x) && likely(y), i.e. it may only be the
> particular combination of the two that is likely).

Make sense, thanks.

> >> +        else
> >> +        {
> >> +            ASSERT(!d || !order);
> > 
> > Is this effectively replacing the ASSERT(order == 0) In the previous d
> > == dom_cow case?
> 
> Yes.
> 
> > If so, can d at this point ever be anything other than dom_cow or NULL?
> > I don't think so. Given that I think ASSERT(!(d == dom_cow && order !=
> > 0)) would more clearly capture the intent of the test (with the spelling
> > out of the conditions being more important than the de morganing of the
> > expression).
> 
> Indeed, d can only be NULL or dom_cow here (being in the else part
> of the if() you quoted at the top). So an alternative might indeed be
> ASSERT(d != dom_cow || !order), but that seems less desirable to
> me as it opens up ways to pass the ASSERT() with d != NULL should
> the if() condition ever get modified. I.e. I'd prefer the assertion to be
> as restrictive as possible, getting relaxed only when in fact necessary.

Since the original if involves d == dom_cow but nothing to do with order
it seemed that the check was somehow specific to dom_cow's relationship
to higher order allocations.

I suppose the question is what relationship would a non-NULL d have to
the order of the allocation. i.e. if the if were changed to also
consider dom_foo why would we expect now that dom_foo had any order
requirements?

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel