Re: [Xen-devel] [PATCH v2] xen: grant-table: Simplify get_paged_frame

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Julien Grall [mailto:julien.grall@xxxxxxx]
> Sent: 19 September 2017 10:40
> To: Jan Beulich <JBeulich@xxxxxxxx>; Paul Durrant
> <Paul.Durrant@xxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>; sstabellini@xxxxxxxxxx; xen-devel@xxxxxxxxxxxxx; Tim
> (Xen.org) <tim@xxxxxxx>
> Subject: Re: [Xen-devel] [PATCH v2] xen: grant-table: Simplify
> get_paged_frame
> 
> Hi,
> 
> On 19/09/17 09:56, Jan Beulich wrote:
> >>>> On 19.09.17 at 10:34, <Paul.Durrant@xxxxxxxxxx> wrote:
> >> I do wonder whether this function belongs in the grant table code though.
> >> Getting the page from a (d, gfn) tuple is probably something that's
> needed in
> >> a few places and hence putting the code in common/memory.c (with
> suitable
> >> adjustment to the error values) would seem more appropriate.
> >
> > That's been true from the very beginning of the existence of
> > the function, I think.
> 
> I am not sure how this function would fit in common/memory.c code. We
> already have get_page_from_gfn to get a page from the tuple (d, gfn).
> 

But that doesn't have the extra logic for populating and unsharing right? 
Otherwise why would get_paged_frame() need to exist at all?

> This function adds more check that may not fit everyone. The only place
> I could see potential usage is prepare_ring_for_helper. But what would
> be a suitable name given?
> 

I would leave the name alone and just move the code.

  Paul

> Cheers,
> 
> --
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel