On Wed, 2006-06-21 at 18:49 -0400, Jimi Xenidis wrote:
> I don't think you definition of clear_entry_flag will work, see below
>
> On Jun 21, 2006, at 2:49 PM, Hollis Blanchard wrote:
> > diff -r ab08d443113d xen/include/asm-ppc/grant_table.h
> > --- a/xen/include/asm-ppc/grant_table.h Wed Jun 21 13:36:49 2006 -0500
> > +++ b/xen/include/asm-ppc/grant_table.h Wed Jun 21 13:42:23 2006 -0500
> > @@ -6,4 +6,6 @@
> > #define mark_dirty(d, f) ((void )0)
> > #include "../asm-x86/grant_table.h"
> >
> > +#define clear_entry_flag(nr, flags) clear_bit((nr), (int *)(flags));
>
> 1) clear_bit takes an 'unsigned long *' so this should have cause a
> warning
Oops, yeah, that was a last-minute change.
> 2) bitops are designed to find the ulong array element (nr /
> BITS_PER_LONG) then set the bit like (1 << (nr % BITS_PER_LONG)) so
> that the word feels like 'C' order.
>
> So I think the definition should be:
> static inline void clear_entry_flag(unsigned long nr, volatile
> uint16_t *addr) {
> volatile unsigned long *laddr;
> unsigned long lnr;
> #ifdef SAFETY_DANCE
> BUG_ON((ulong)addr % sizeof (ulong));
> #endif
> lnr = (BITS_PER_LONG - (sizeof (*addr) * 8)) + nr;
> laddr = (volatile unsigned long *)addr;
> clear_bit(lnr, laddr);
> }
>
> You could macro-tize this, but why.
Does the bit order matter if all users go through this macro? If they
don't go through this macro, we will catch them as link errors like we
do now. I guess that only covers cmpxchg callers though; anybody
directly dereferencing the field directly would break.
--
Hollis Blanchard
IBM Linux Technology Center
_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel
|
Home