[LU-12923] replace CLASSERT() with BUILD_BUG_ON()

• Status: Resolved
• Project: Lustre
• Affects Version/s: Lustre 2.14.0
• Fix Version/s: Lustre 2.14.0
• Type: Technical task
• Priority: Minor
• Reporter: Andreas Dilger
• Assignee: Arshad Hussain
• Resolution: Fixed
• Labels: easy

Description

The Lustre-specific compile-time assertion CLASSERT() should be replaced with the standard kernel BUILD_BUG_ON() macro that does the same thing.

NOTE that the logic of CLASSERT() is ensuring that the expression is true while BUILD_BUG_ON() is verifying if the expression is false, so the logic of each check should be reversed.

All of these expressions are evaluated at compile time, so it is possible to mark all patches fixing this with Test-Parameters: trivial.

The first patch in the series should update contrib/scripts/spelling.txt to add CLASSERT||BUILD_BUG_ON() so that new patches get a warning if they are using the old CLASSERT() macro. The last patch in the series should remove the CLASSERT() macro. There are currently 480 instances of CLASSERT() in the master branch, but a majority of them (318) are in the automatically-generated wirecheck.c, pack-generic.c and api-ni.c file, so most could be easily replaced by fixing the wiretest.c scripts.

Issue Links

• is related to: LU-9859 libcfs simplification

Comments

[James A Simmons]

Are you going to take this work or should I. I have been planning to do this.

[Andreas Dilger]

I'm not planning on it, but added it as a separate task marked easy for tracking purposes. I was going to add a CLASSERT() in one of my patches and then thought I should use BUILD_BUG_ON() instead, and that it makes sense to formalize this transition.

[Arshad Hussain]

Andreas,James, - One more volunteer for this. Let me know if I can take this one. 

Thanks.

[Andreas Dilger]

arshad512 I'd be very grateful if you have the time to do this. No rush, as we haven't yet opened master for patch landings until after 2.13 is released, but that should be in a week or two.

[Arshad Hussain]

Andreas, more than glad to do this patch. And let me add this...as always ... Thank you!

Few patches will come this weekend , next set of patch batches around next weekend. 

Thanks

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36645
Subject: LU-12923 contrib: Update spelling.txt to add BUILD_BUG_ON() correction
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: e8942c4da1012fbdd0655b5b3ca56343cb3fee9d

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36646
Subject: LU-12923 mdc: Replace CLASSERT() with BUILD_BUG_ON() for file mdc_lib.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: c4c1a27d2bc0947b914c7a7f6e4cf762f90502e6

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36647
Subject: LU-12923 utils: Replace CLASSERT() with BUILD_BUG_ON() for wiretest.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 010469666ca82f1c3eaebe41de7e7fd3f8083513

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36648
Subject: LU-12923 pltrpc: Replace CLASSERT() with BUILD_BUG_ON() for wiretest.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 867fb4f9bc31fc8b876362ea9275920e7f49091b

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36649
Subject: LU-12923 ptlrpc: Replace CLASSERT() with BUILD_BUG_ON() for pack_generic.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 09b42fcdc4831e714348990942400db8b534106f

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36690
Subject: LU-12923 utils: Use BUILD_BUG_ON() for wirecheck.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 16f2e758e3e9004f6d88817071242a257ef9d6f6

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36711
Subject: LU-12923 utils: Use BUILD_BUG_ON() for wirehdr.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 9a4e99fe0d7401e8ce21c88f6a31eb0cdcd3d7ad

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36717
Subject: LU-12923 mdc: Use BUILD_BUG_ON() for mdc_reint.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 1835f99b8ceafbd7dadff9945977b55691b51d00

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36718
Subject: LU-12923 lov: Use BUILD_BUG_ON() for lov_pack.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: d1a5c95d1d29652cfcb25fca67a09478bc3de1d6

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36719
Subject: LU-12923 mdd: Use with BUILD_BUG_ON() for mdd_object.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 0b6e5fe4da3a81321aed3020f9a4f728e6068e4b

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36720
Subject: LU-12923 ldlm: Use BUILD_BUG_ON() for ldlm_resource.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 8c7fb9225e17926e20d2150c2c1700029c68f746

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36721
Subject: LU-12923 target: Use BUILD_BUG_ON() for tgt_lastrcvd.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 3b8ad3eb99014316b340b7272afd5c676e0efee3

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36722
Subject: LU-12923 mdc: Use BUILD_BUG_ON() for mdc_request.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: c400865bcc4393b1ed617f0b3fc1ce7e122c9d15

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36725
Subject: LU-12923 lustre: Replace CLASSERT() with BUILD_BUG_ON()
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 7cb1fe9bcac8ef241749cf634c0d1a4d4e3fb174

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/36902
Subject: LU-12923 libcfs: Use BUILD_BUG_ON() for hash.c
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 8128b0e74060bab239e2718a66f2cb4a4d6ed06e

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36645/
Subject: LU-12923 contrib: Update spelling.txt to add BUILD_BUG_ON()
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: d22399313afd98a953da596fab025fccf0a779a0

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36646/
Subject: LU-12923 mdc: Use BUILD_BUG_ON() for mdc_lib.c
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 478724469976b6e9b6f6d38253a30d5c0b57243c

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36647/
Subject: LU-12923 utils: Use BUILD_BUG_ON() for wiretest.c
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 38392dc0ed32368cc195558a74c6606465beb53b

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36648/
Subject: LU-12923 pltrpc: Use BUILD_BUG_ON() for wiretest.c
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: e2126c7858da90a233b08baa10917e71a72fc6b7

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36717/
Subject: LU-12923 mdc: Use BUILD_BUG_ON() for mdc_reint.c
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 9b9903816c96c87685273bbdb3aa3853666a37de

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36718/
Subject: LU-12923 lov: Use BUILD_BUG_ON() for lov_pack.c
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: f6b5483170a3d729d1163a0d6a0da63c6ba926fe

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36719/
Subject: LU-12923 mdd: Use BUILD_BUG_ON() for mdd_object.c
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 0978020c92f0b2b623f12cf2e5c9a9e0e7fc6411

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36720/
Subject: LU-12923 ldlm: Use BUILD_BUG_ON() for ldlm_resource.c
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: db25e324ad6b59b75db001a4f86d8467bed81ad2

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36721/
Subject: LU-12923 target: Use BUILD_BUG_ON() for tgt_lastrcvd.c
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 8bbc27d31389cabe992d68dd9b0900bb26971e66

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36722/
Subject: LU-12923 mdc: Use BUILD_BUG_ON() for mdc_request.c
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: f4408ad5377a104aae869ef8bf02796fdfa8fb79

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36649/
Subject: LU-12923 ptlrpc: Use BUILD_BUG_ON() for pack_generic.c
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: a1bf9876a328aec3b1d307e8b20d997b30377402

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36725/
Subject: LU-12923 lustre: Replace CLASSERT() with BUILD_BUG_ON()
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 6c6ca1fbfb1b2be0e6a9675f9214f98e59bc8088

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36711/
Subject: LU-12923 utils: Use BUILD_BUG_ON() for wirehdr.c & wirecheck.c
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 5a3f9f4cc3c31b0f521932df80a9ac541edec073

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/36902/
Subject: LU-12923 libcfs: Use BUILD_BUG_ON() for hash.c
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: eb3d0504f2d7eed8c8c584af82ad609455071bd2

[Peter Jones]

Everything currently in flight has landed - is this task now complete?

[Arshad Hussain]

Hi Peter, I was away traveling. I am catching up on my mails and messages now, sorry for delay response.

Unfortunately, as I double checked it looks like few instances under files which escaped my scrutiny.  So, this LU is almost done but not complete. I will address this quickly.

 

[lustre-release]$ find ./lustre/ -name "*.c" | xargs grep CLASSERT
./lustre/osd-zfs/osd_index.c: CLASSERT(sizeof(zde->lzd_reg) == 8);
./lustre/osd-zfs/osd_index.c: CLASSERT(sizeof(*zde) % 8 == 0);
./lustre/osd-zfs/osd_index.c: CLASSERT(sizeof(za->za_name) <= sizeof(it->ozi_name));
./lustre/ptlrpc/nodemap_storage.c: CLASSERT(sizeof(union nodemap_rec) == 32);
./lustre/mdd/mdd_object.c: CLASSERT(ARRAY_SIZE(info->mti_buf) >= 4);

Where I went wrong was, I was using below command to verify. Which unfortunately was give me false positives.

$ find ./lustre -name "*.c" | xargs ./contrib/scripts/checkpatch.pl -f --no-tree | grep CLASSERT | wc -l

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/37111
Subject: LU-12923 lustre: Replace CLASSERT() with BUILD_BUG_ON()
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: de937279e835c26b61bbb6c248279c53852d631d

[Aurelien Degremont (Inactive)]

Looks like lnet also needs one last patch:

$ git grep -c CLASSERT lnet/
lnet/lnet/api-ni.c:96
lnet/utils/wirecheck.c:2

 

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/37113
Subject: LU-12923 lustre: Replace CLASSERT() with BUILD_BUG_ON()
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 51a21905a4a815a80497b59496ccdeeda9a3f3b8

[Arshad Hussain]

Aurelien, Thanks for pointing out.

Updated remaining CLASSERT() under lnet.

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/37111/
Subject: LU-12923 lustre: Replace CLASSERT() with BUILD_BUG_ON()
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: f0fa794d0fc35be8fb0839731d122a73395f2a05

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/37113/
Subject: LU-12923 lnet: Replace CLASSERT() with BUILD_BUG_ON()
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 81fff75016dae08332d1a6cdd5137fc4e3097665

[Peter Jones]

Landed for 2.14

[Aurelien Degremont (Inactive)]

What should we do with the last 2 occurrences?

$ git grep CLASSERT
contrib/scripts/spelling.txt:CLASSERT||BUILD_BUG_ON()
libcfs/include/libcfs/libcfs_private.h:#define CLASSERT(cond) do {switch (1) {case (cond): case 0: break; } } while (0)

I think they should also be removed?

[James A Simmons]

spelling.txt will remain since it tells people not use CLASSERT anymore. The define in libcfs_private.h does need to be removed but we should wait a bit since other patches in flight might land that have a CLASSERT.

[Arshad Hussain]

Aurelien,
As James pointed out already, "spelling.txt" remains as it helps checkpatch to flag warning if any CLASSERT is left behind. For libcfs_private.h - this was missed ((again!), which is a bummer ) - Sorry. I will get this in.
 
James,
Only patch on master:branch open which is referring 'libcfs_private.h' is https://review.whamcloud.com/#/c/36975/ - This also does not touch CLASSERT(). What I found out, updating libcfs_private.h immediately will not affect any patch. I am going ahead with the change.

[Gerrit Updater]

Arshad Hussain ([e-mail]) uploaded a new patch: https://review.whamcloud.com/37188
Subject: LU-12923 libcfs: Remove CLASSERT() for libcfs_private.h
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: f4c373f4485db07b98d74ebe8005b08f2064ca3a

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/37188/
Subject: LU-12923 libcfs: Remove CLASSERT() for libcfs_private.h
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: fa70e91747dc712f3083649cb82194bd7c0c856f

[Peter Jones]

For real complete now?

[Arshad Hussain]

Beyond a doubt.

[lustre-release]# git grep -c CLASSERT
contrib/scripts/spelling.txt:1
[lustre-release]#