| Message ID | 20171219031703.23420-3-sameer.goel@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SMMUv3 driver | expand |
Hi Sameer, On 19/12/17 03:16, Sameer Goel wrote: > Changing the name of the macro from LOG_2 to ilog2.This makes the function name > similar to its Linux counterpart. Since, this is not used in mutiple places, so the code churn is minimal. s/mutiple/multiple/ > This change helps in porting unchanged code from Linux. Can you wrap the commit message correctly? Cheers, > > Signed-off-by: Sameer Goel <sameer.goel@linaro.org> > --- > xen/arch/x86/x86_64/asm-offsets.c | 2 +- > xen/include/xen/bitops.h | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c > index e136af6b99..4bccbc9bdf 100644 > --- a/xen/arch/x86/x86_64/asm-offsets.c > +++ b/xen/arch/x86/x86_64/asm-offsets.c > @@ -157,7 +157,7 @@ void __dummy__(void) > BLANK(); > #endif > > - DEFINE(IRQSTAT_shift, LOG_2(sizeof(irq_cpustat_t))); > + DEFINE(IRQSTAT_shift, ilog2(sizeof(irq_cpustat_t))); > OFFSET(IRQSTAT_softirq_pending, irq_cpustat_t, __softirq_pending); > BLANK(); > > diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h > index e2019b02a3..a103e49089 100644 > --- a/xen/include/xen/bitops.h > +++ b/xen/include/xen/bitops.h > @@ -223,7 +223,7 @@ static inline __u32 ror32(__u32 word, unsigned int shift) > #define __L4(_x) (((_x) & 0x0000000c) ? ( 2 + __L2( (_x)>> 2)) : __L2( _x)) > #define __L8(_x) (((_x) & 0x000000f0) ? ( 4 + __L4( (_x)>> 4)) : __L4( _x)) > #define __L16(_x) (((_x) & 0x0000ff00) ? ( 8 + __L8( (_x)>> 8)) : __L8( _x)) > -#define LOG_2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : __L16(_x)) > +#define ilog2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : __L16(_x)) > > /** > * for_each_set_bit - iterate over every set bit in a memory region >
On Mon, Dec 18, 2017 at 08:16:57PM -0700, Sameer Goel wrote: > diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h > index e2019b02a3..a103e49089 100644 > --- a/xen/include/xen/bitops.h > +++ b/xen/include/xen/bitops.h > @@ -223,7 +223,7 @@ static inline __u32 ror32(__u32 word, unsigned int shift) > #define __L4(_x) (((_x) & 0x0000000c) ? ( 2 + __L2( (_x)>> 2)) : __L2( _x)) > #define __L8(_x) (((_x) & 0x000000f0) ? ( 4 + __L4( (_x)>> 4)) : __L4( _x)) > #define __L16(_x) (((_x) & 0x0000ff00) ? ( 8 + __L8( (_x)>> 8)) : __L8( _x)) > -#define LOG_2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : __L16(_x)) > +#define ilog2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : __L16(_x)) Er, why not add a: #define ilog2 LOG_2 On the code that you have to import? Roger.
Hi Roger, On 23/01/18 11:39, Roger Pau Monné wrote: > On Mon, Dec 18, 2017 at 08:16:57PM -0700, Sameer Goel wrote: >> diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h >> index e2019b02a3..a103e49089 100644 >> --- a/xen/include/xen/bitops.h >> +++ b/xen/include/xen/bitops.h >> @@ -223,7 +223,7 @@ static inline __u32 ror32(__u32 word, unsigned int shift) >> #define __L4(_x) (((_x) & 0x0000000c) ? ( 2 + __L2( (_x)>> 2)) : __L2( _x)) >> #define __L8(_x) (((_x) & 0x000000f0) ? ( 4 + __L4( (_x)>> 4)) : __L4( _x)) >> #define __L16(_x) (((_x) & 0x0000ff00) ? ( 8 + __L8( (_x)>> 8)) : __L8( _x)) >> -#define LOG_2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : __L16(_x)) >> +#define ilog2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : __L16(_x)) > > Er, why not add a: > > #define ilog2 LOG_2 > > On the code that you have to import? There is exactly on caller of LOG_2. So what is the benefits to provide 2 names? More that I would expect ilog2 to be used in any code coming from Linux. Cheers, > > Roger. > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel >
On Tue, Jan 23, 2018 at 11:44:30AM +0000, Julien Grall wrote: > Hi Roger, > > On 23/01/18 11:39, Roger Pau Monné wrote: > > On Mon, Dec 18, 2017 at 08:16:57PM -0700, Sameer Goel wrote: > > > diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h > > > index e2019b02a3..a103e49089 100644 > > > --- a/xen/include/xen/bitops.h > > > +++ b/xen/include/xen/bitops.h > > > @@ -223,7 +223,7 @@ static inline __u32 ror32(__u32 word, unsigned int shift) > > > #define __L4(_x) (((_x) & 0x0000000c) ? ( 2 + __L2( (_x)>> 2)) : __L2( _x)) > > > #define __L8(_x) (((_x) & 0x000000f0) ? ( 4 + __L4( (_x)>> 4)) : __L4( _x)) > > > #define __L16(_x) (((_x) & 0x0000ff00) ? ( 8 + __L8( (_x)>> 8)) : __L8( _x)) > > > -#define LOG_2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : __L16(_x)) > > > +#define ilog2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : __L16(_x)) > > > > Er, why not add a: > > > > #define ilog2 LOG_2 > > > > On the code that you have to import? > > There is exactly on caller of LOG_2. So what is the benefits to provide 2 > names? More that I would expect ilog2 to be used in any code coming from > Linux. I don't think we should be renaming macros just because we want to import verbatim code from Linux or anywhere else, I would rather add a linuxkpi.h or similar in order to do the translation, but that's just my opinion. Roger.
Hi Roger, On 23/01/18 12:10, Roger Pau Monné wrote: > On Tue, Jan 23, 2018 at 11:44:30AM +0000, Julien Grall wrote: >> Hi Roger, >> >> On 23/01/18 11:39, Roger Pau Monné wrote: >>> On Mon, Dec 18, 2017 at 08:16:57PM -0700, Sameer Goel wrote: >>>> diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h >>>> index e2019b02a3..a103e49089 100644 >>>> --- a/xen/include/xen/bitops.h >>>> +++ b/xen/include/xen/bitops.h >>>> @@ -223,7 +223,7 @@ static inline __u32 ror32(__u32 word, unsigned int shift) >>>> #define __L4(_x) (((_x) & 0x0000000c) ? ( 2 + __L2( (_x)>> 2)) : __L2( _x)) >>>> #define __L8(_x) (((_x) & 0x000000f0) ? ( 4 + __L4( (_x)>> 4)) : __L4( _x)) >>>> #define __L16(_x) (((_x) & 0x0000ff00) ? ( 8 + __L8( (_x)>> 8)) : __L8( _x)) >>>> -#define LOG_2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : __L16(_x)) >>>> +#define ilog2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : __L16(_x)) >>> >>> Er, why not add a: >>> >>> #define ilog2 LOG_2 >>> >>> On the code that you have to import? >> >> There is exactly on caller of LOG_2. So what is the benefits to provide 2 >> names? More that I would expect ilog2 to be used in any code coming from >> Linux. > > I don't think we should be renaming macros just because we want to > import verbatim code from Linux or anywhere else, I would rather add a > linuxkpi.h or similar in order to do the translation, but that's just > my opinion. I would have agreed if there was 10-30 callsite in Xen code. For 1 callsite it sounds like a bit overkill to request to have 2 different name. You can see this as renaming very similar to series that rename field. Cheers,
diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c index e136af6b99..4bccbc9bdf 100644 --- a/xen/arch/x86/x86_64/asm-offsets.c +++ b/xen/arch/x86/x86_64/asm-offsets.c @@ -157,7 +157,7 @@ void __dummy__(void) BLANK(); #endif - DEFINE(IRQSTAT_shift, LOG_2(sizeof(irq_cpustat_t))); + DEFINE(IRQSTAT_shift, ilog2(sizeof(irq_cpustat_t))); OFFSET(IRQSTAT_softirq_pending, irq_cpustat_t, __softirq_pending); BLANK(); diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h index e2019b02a3..a103e49089 100644 --- a/xen/include/xen/bitops.h +++ b/xen/include/xen/bitops.h @@ -223,7 +223,7 @@ static inline __u32 ror32(__u32 word, unsigned int shift) #define __L4(_x) (((_x) & 0x0000000c) ? ( 2 + __L2( (_x)>> 2)) : __L2( _x)) #define __L8(_x) (((_x) & 0x000000f0) ? ( 4 + __L4( (_x)>> 4)) : __L4( _x)) #define __L16(_x) (((_x) & 0x0000ff00) ? ( 8 + __L8( (_x)>> 8)) : __L8( _x)) -#define LOG_2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : __L16(_x)) +#define ilog2(_x) (((_x) & 0xffff0000) ? (16 + __L16((_x)>>16)) : __L16(_x)) /** * for_each_set_bit - iterate over every set bit in a memory region
Changing the name of the macro from LOG_2 to ilog2.This makes the function name similar to its Linux counterpart. Since, this is not used in mutiple places, so the code churn is minimal. This change helps in porting unchanged code from Linux. Signed-off-by: Sameer Goel <sameer.goel@linaro.org> --- xen/arch/x86/x86_64/asm-offsets.c | 2 +- xen/include/xen/bitops.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)