Re: [Xen-devel] [PATCH] grant table map error in __gnttab_map_grant_ref

[Liuyongan <liuyongan@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Monday, February 06, 2012 5:03 PM
> To: Liuyongan
> Cc: hanweidong; Zhanghaoyu; Wang Liang; Qianhuibin; Andres Lagar-
> Cavilla; xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH] grant table map error in
> __gnttab_map_grant_ref
> 
>    >>> On 04.02.12 at 03:43, Liuyongan <liuyongan@xxxxxxxxxx> wrote:
> > In file grant_table.c function __gnttab_map_grant_ref, if
> __get_paged_frame
> > failed, the effect of _set_status  previously called should be
> rollback, so
> > the flag GTF_reading and _GTF_writing will be recovered.
> 
> Not knowing too much about these, but isn't __acquire_grant_for_copy()
> in need of a similar adjustment?
  Well, I need to check it further.
> 
> Further, aren't the status flags cumulative (and hence isn't it wrong
> to
> simply clear flags without considering their original state)? 
  When undo_out branch is executed, the flag set by _set_status function
  will be rolled back by:
       if ( !(op->flags & GNTMAP_readonly) &&
         !(act->pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
        gnttab_clear_flag(_GTF_writing, status);

       if ( !act->pin )
        gnttab_clear_flag(_GTF_reading, status);
  so action added by this patch should be correct. 

  And anyway, the  reading and writing flag should be recovered when mapping 
  grant reference failed, so the up layer(eg. Netback driver) can decide 
  freely whether retry mapping or fail directly.

> (I realize
> that this is not a new issue introduced with the proposed patch, but
> certainly with more ways of getting that code path run through, it
> becomes more important for it to be correct.)
> 
> Jan
> 
> > Signed-off-by: Haoyu Zhang<haoyu.zhang@xxxxxxxxxx>; Liang
> > Wang<hzwangliang.wang@xxxxxxxxxx>
> >
> > diff -r cbed91e1c878 xen-4.1.2/xen/common/grant_table.c
> > --- a/xen-4.1.2/xen/common/grant_table.c    Sat Feb 04 18:36:13
> 2012 +0800
> > +++ b/xen-4.1.2/xen/common/grant_table.c    Sat Feb 04 18:40:02
> 2012 +0800
> > @@ -566,7 +566,7 @@
> >              gfn = sha1 ? sha1->frame : sha2->full_page.frame;
> >              rc = __get_paged_frame(gfn, &frame, !!(op->flags &
> > GNTMAP_readonly), rd);
> >              if ( rc != GNTST_okay )
> > -                goto unlock_out;
> > +                goto unlock_out_clear;
> >              act->gfn = gfn;
> >              act->domid = ld->domain_id;
> >              act->frame = frame;
> > @@ -721,7 +721,8 @@
> >      if ( op->flags & GNTMAP_host_map )
> >          act->pin -= (op->flags & GNTMAP_readonly) ?
> >              GNTPIN_hstr_inc : GNTPIN_hstw_inc;
> > -
> > +
> > + unlock_out_clear:
> >      if ( !(op->flags & GNTMAP_readonly) &&
> >           !(act->pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
> >          gnttab_clear_flag(_GTF_writing, status);
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel