On 2015/09/21, 4:46 PM, "Mike Rapoport" <mike.rapoport@xxxxxxxxx> wrote:
>On Mon, Sep 21, 2015 at 12:54:54PM +0000, Dilger, Andreas wrote:
>> On 2015/09/21, 2:25 PM, "Mike Rapoport" <mike.rapoport@xxxxxxxxx> wrote:
>>
>> >Use BIT_ULL() macro instead of long hexadecimal constants with only
>> >single bit set.
>> >
>> >Signed-off-by: Mike Rapoport <mike.rapoport@xxxxxxxxx>
>> >---
>> > .../lustre/lustre/include/lustre_dlm_flags.h | 80
>> >+++++++++++-----------
>> > 1 file changed, 40 insertions(+), 40 deletions(-)
>> >
>> >diff --git a/drivers/staging/lustre/lustre/include/lustre_dlm_flags.h
>> >b/drivers/staging/lustre/lustre/include/lustre_dlm_flags.h
>> >index d27bdae0..a247cbf 100644
>> >--- a/drivers/staging/lustre/lustre/include/lustre_dlm_flags.h
>> >+++ b/drivers/staging/lustre/lustre/include/lustre_dlm_flags.h
>> >@@ -50,7 +50,7 @@
>> > #define LDLM_FL_ON_WIRE_MASK 0x00000000C08F932FULL
>> >
>> > /** extent, mode, or resource changed */
>> >-#define LDLM_FL_LOCK_CHANGED 0x0000000000000001ULL // bit
>> 0
>> >+#define LDLM_FL_LOCK_CHANGED BIT_ULL(0)
>>
>> IMHO, this is a step backward in usability. When I see a debug log
>> that prints these lock flags in hex, for example 0x200801220, it is
>> easier to look at those flags and see it is FL_SKIPPED, FL_TEST_LOCK,
>> FL_HAS_INTENT, FL_INTENT_ONLY, FL_AST_SENT. If they are declared as
>> "BIT_ULL(33)" that is not very useful for humans actually looking at
>> this code.
>
>I think it's personal and a matter of habbit. In your example,
>0x200801220 may be seen as "bits 5, 9, 12, 23 and 33 are set", and then
>one may look for bit definitions...
Sure, but I'm the person more likely to be debugging Lustre problems,
so I'd really rather keep it the way it is today.
>BTW, somewhat semi-related quiestion:
>I've grepped for usage of ldlm_{is,set,clear}* and I've found only
>three places that are using them. Is there a reason to keep the defines
>or replacing those three occurences with open-coded versions and
>dropping ldlm_{is,set,clear}* macros would be Ok?
They are used a lot in the server code, so they should be kept.
>
>> I don't see any benefits to changing these declarations to use
>>BIT_ULL().
>
>The obvoius benifit of these changes is removal of C99 comments :)
No objection to fixing the C99 comments. :-)
Cheers, Andreas
--
Andreas Dilger
Lustre Software Architect
Intel High Performance Data Division
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
