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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.02.12 at 04:53, Liuyongan <liuyongan@xxxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> >>> On 06.02.12 at 12:38, Liuyongan <liuyongan@xxxxxxxxxx> wrote:
>> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> >>    >>> 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.
> 
>    The caller of __acquire_grant_for_copy() will make sure similar rollback
> by calling __release_grant_for_copy().

Ah, okay. Thanks for clarifying that.

>> >> 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.
>> 
>> But this is not a roll-back, here the flags get simply cleared (whereas
>> "roll-back" to me means the restoration of their previous value).
> 
>   According to my analysis, flag will be cleared only if it is set 
> previously 
> in _set_status() of this function.

But still without regard to the previous value these flags had, while
it is my current understanding that these flags provide information
on the kind of uses (note the plural) that a particular grant is in (not
sure what confusion would arise if these flags don't reflect the
actual state, but if the flags getting out of sync was benign, they
could as well be removed).

Jan


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