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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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())...

Bottom line - since you've got Andrew's approval, I don't mean
to outright object to the change, but I like this extra aspect to
be taken into account before deciding whether to really put it in.

Jan


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