Re: [Xen-devel] [PATCH 2/3] gnttab: Drop the frame parameter from get_paged_frame()

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Aug 24, 2017 at 06:55:54PM +0100, Andrew Cooper wrote:
> It is redundant with the *page parameter.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>

> ---
> CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Tim Deegan <tim@xxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  xen/common/grant_table.c | 50 
> +++++++++++++++++++++---------------------------
>  1 file changed, 22 insertions(+), 28 deletions(-)
> 
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 188c477..d8307b7 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -257,13 +257,13 @@ static inline void active_entry_release(struct 
> active_grant_entry *act)
>      spin_unlock(&act->lock);
>  }
>  
> -/* Check if the page has been paged out, or needs unsharing.
> -   If rc == GNTST_okay, *page contains the page struct with a ref taken.
> -   Caller must do put_page(*page).
> -   If any error, *page = NULL, *frame = INVALID_MFN, no ref taken. */
> -static int get_paged_frame(unsigned long gfn, unsigned long *frame,
> -                           struct page_info **page, bool readonly,
> -                           struct domain *rd)
> +/*
> + * Check if the page has been paged out, or needs unsharing.
> + * If rc == GNTST_okay, *page contains the page struct with a ref taken.
> + * Caller must do put_page(*page). If any error, *page = NULL, no ref taken.
> + */
> +static int get_paged_frame(unsigned long gfn, struct page_info **page,
> +                           bool readonly, struct domain *rd)
>  {
>      int rc = GNTST_okay;
>  #if defined(P2M_PAGED_TYPES) || defined(P2M_SHARED_TYPES)
> @@ -273,7 +273,6 @@ static int get_paged_frame(unsigned long gfn, unsigned 
> long *frame,
>                                (readonly) ? P2M_ALLOC : P2M_UNSHARE);
>      if ( !(*page) )
>      {
> -        *frame = mfn_x(INVALID_MFN);
>          if ( p2m_is_shared(p2mt) )
>              return GNTST_eagain;
>          if ( p2m_is_paging(p2mt) )
> @@ -283,13 +282,12 @@ static int get_paged_frame(unsigned long gfn, unsigned 
> long *frame,
>          }
>          return GNTST_bad_page;
>      }
> -    *frame = page_to_mfn(*page);
>  #else
> -    *frame = mfn_x(gfn_to_mfn(rd, _gfn(gfn)));
> -    *page = mfn_valid(_mfn(*frame)) ? mfn_to_page(*frame) : NULL;
> +    mfn_t mfn = gfn_to_mfn(rd, _gfn(gfn));
> +
> +    *page = mfn_valid(mfn) ? mfn_to_page(mfn_x(mfn)) : NULL;
>      if ( (!(*page)) || (!get_page(*page, rd)) )

Mind dropping those unneeded parentheses?

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