Re: [Xen-devel] [PATCH] x86/mm: use put_page_type_preemptible in put_page_from_l{3, 4}e

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Sep 04, 2017 at 07:06:51AM -0600, Jan Beulich wrote:
> >>> On 04.09.17 at 13:42, <wei.liu2@xxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/mm.c
> > +++ b/xen/arch/x86/mm.c
> > @@ -1376,7 +1376,7 @@ static int put_page_from_l3e(l3_pgentry_t l3e, 
> > unsigned long pfn,
> >      if ( unlikely(partial > 0) )
> >      {
> >          ASSERT(!defer);
> > -        return __put_page_type(pg, 1);
> > +        return put_page_type_preemptible(pg);
> >      }
> >  
> >      if ( defer )
> > @@ -1399,7 +1399,7 @@ static int put_page_from_l4e(l4_pgentry_t l4e, 
> > unsigned long pfn,
> >          if ( unlikely(partial > 0) )
> >          {
> >              ASSERT(!defer);
> > -            return __put_page_type(pg, 1);
> > +            return put_page_type_preemptible(pg);
> >          }
> 
> Is this really a good idea? put_page_type_preemptible() is just
> a thin wrapper around __put_page_type(), so that the latter
> can remain private to mm.c. By going through the wrapper you
> add another branch into a path that I would guess isn't executed
> frequently enough for its constituents to remain in the uops
> cache, but possibly frequently enough for the extra branch to
> matter. Otoh I see we use get_page_type_preemptible() in mm.c
> too in two places (albeit that one also has an extra ASSERT())...
> 

This patch is taken from my mm.c refactoring series. 

Like you said, __put_page_type should remain private to mm.c. By making
this change I can  move put_page_from_l{2,3,4}e to pv/mm.c and leave
__put_page_type in mm.c. Is this a good enough reason for you?

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