Re: [Xen-devel] [PATCH v2] xen/mm.h: add helper function to test-and-clear _PGC_allocated

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <JBeulich@xxxxxxxx>
> Sent: 15 July 2019 10:24
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Julien Grall <julien.grall@xxxxxxx>; 
> Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>; Roger Pau Monne <roger.pau@xxxxxxxxxx>; 
> VolodymyrBabchuk
> <Volodymyr_Babchuk@xxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>; Ian 
> Jackson
> <Ian.Jackson@xxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Konrad 
> Rzeszutek Wilk
> <konrad.wilk@xxxxxxxxxx>; Tamas K Lengyel <tamas@xxxxxxxxxxxxx>; Tim 
> (Xen.org) <tim@xxxxxxx>; Wei Liu
> <wl@xxxxxxx>
> Subject: Re: [PATCH v2] xen/mm.h: add helper function to test-and-clear 
> _PGC_allocated
> 
> On 15.07.2019 11:17, Paul Durrant wrote:
> > The _PGC_allocated flag is set on a page when it is assigned to a domain
> > along with an initial reference count of at least 1. To clear this
> > 'allocation' reference it is necessary to test-and-clear _PGC_allocated and
> > then only drop the reference if the test-and-clear succeeds. This is open-
> > coded in many places. It is also unsafe to test-and-clear _PGC_allocated
> > unless the caller holds an additional reference.
> >
> > This patch adds a helper function, put_page_alloc_ref(), to replace all the
> > open-coded test-and-clear/put_page occurrences and incorporates in that a
> > BUG_ON() an additional page reference not being held.
> 
> This last sentence reads somewhat strange to me - are there words
> missing and/or mis-ordered?

Perhaps it reads better if 'BUG_ON()' is substituted with 'BUG() on'? I just 
wanted to express that there was a new check in the helper function that the 
necessary additional reference is held.

> 
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> 
> With the commit message aspect clarified

I am happy for you to re-word it if you feel it is not clear. With the extra 
comment in the helper function in v2 then perhaps it is not really necessary to 
have any additional explanation in the commit comment anyway?

> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks,

  Paul

> 
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel