[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH, RFC] Re: x86: gnttab_clear_flag() abusing clear_bit()
>>> On 07.02.12 at 06:10, Keir Fraser <keir.xen@xxxxxxxxx> wrote: > On 07/02/2012 10:34, "Jan Beulich" <JBeulich@xxxxxxxx> wrote: > >>>>> On 06.02.12 at 18:06, "Jan Beulich" <JBeulich@xxxxxxxx> wrote: >>> Back in c/s 17194:af33f2054f47 bitops got restricted to 4-bytes and >>> larger operands only. gnttab_clear_flag() cheats in casting a uint16_t * >>> to unsigned long * - how is that not a problem in the context of the >>> goal that said c/s set, in particular for v2 of the interface? (Given that >>> this is being implemented as arch-specific routine - so far for reasons >>> that escape me - this should be simple to fix by using inline assembly >>> rather than clear_bit().) >>> >>> Further I just spotted one instance where the or of two bit positions >>> gets passed to this function - that's quite definitely a bug I would say. >>> >>> And, quite the opposite, __fixup_status_for_pin() ands out the >>> negation of bit positions rather than masks... (Which also raises >>> the question whether it really would need to be clear_bit() above >>> instead of the cheaper __clear_bit().) >> >> Below the tentative fix for all of the above problems. In the light >> of the comment at the top of x86's bitops.h I'm awaiting our gcc >> experts' response regarding the safety of using "+m" here. > > Looks fine to me, in principle. I would add a comment to the x86 > gnttab_clear_flag() explaining why we have to open code something that looks > a lot like clear_bit(). That one I already did, will submit soon (desiring clarification on the below). As to the "+m" constraint - I'm being told that "+m" (var) is equivalent to "=m" (var) : "m" (var), no matter what the documentation says regarding '+' (but they're also not seeing a need to adjust the docs accordingly). The question is whether we should go with the (documentation-wise correct) form, or the shorter one (which they're unlikely to change the meaning of, given in how many places "+m" is used in e.g. Linux). Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |