Re: [Xen-devel] [PATCH] add page_get_owner_and_reference() related ASSERT()s

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.08.15 at 12:12, <tim@xxxxxxx> wrote:
> At 03:46 -0600 on 13 Aug (1439437600), Jan Beulich wrote:
>> >>> On 13.08.15 at 11:31, <tim@xxxxxxx> wrote:
>> > At 09:50 +0100 on 13 Aug (1439459451), Ian Campbell wrote:
>> >> On Fri, 2015-07-24 at 06:37 -0600, Jan Beulich wrote:
>> >> > The function shouldn't return NULL after having obtained a reference,
>> >> > or else the caller won't know to drop it.
>> >> 
>> >> > Also its result shouldn't be ignored - if calling code is certain that
>> >> > a page already has a non-zero refcount, it better ASSERT()s so.
>> >> 
>> >> How is page_get_owner_and_reference sure the reference count was not zero
>> >> on entry? (WRT the call to page_get_owner in page_get_owner_and_reference
>> >> itself) Or have I misunderstood the assertion being made here?
>> > 
>> > That path is fine -- if the count was zero, it will return NULL before
>> > trying to get the owner.  But I'm not convinced that the assertion is
>> > correct -- are there really no pages anywhere that have a recount but
>> > no owner?  What about, e.g. shadow pool pages or other uses of
>> > MEMF_no_owner?  If a guest can cause Xen to try to get_page() one of
>> > those, then we'd hit the new assertion.
>> > 
>> > How about unlikely(!owner) path that drops the taken ref instead?
>> 
>> That would be dead code - a page the ref count of wasn't zero
>> prior to the function taking an extra ref shouldn't be unowned.
> 
> Right.  That being the case, Reviewed-by: Tim Deegan <tim@xxxxxxx>.

Btw, as I was about to add a comment as you asked for along with
adding the one Ian wants I realized that
page_get_owner_and_reference() is a helper of get_page(), i.e.
their behavior of not always acquiring a reference is the same.
Which - to me - makes adding such a comment pretty pointless.

Jan


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