Details

    • Technical task
    • Resolution: Fixed
    • Minor
    • Lustre 2.14.0
    • Lustre 2.14.0
    • 9223372036854775807

    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.

      Attachments

        Issue Links

          Activity

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

            Oleg Drokin (green@whamcloud.com) 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

            gerrit Gerrit Updater added a comment - Oleg Drokin (green@whamcloud.com) 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

            Oleg Drokin (green@whamcloud.com) 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 Gerrit Updater added a comment - Oleg Drokin (green@whamcloud.com) 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

            Aurelien, Thanks for pointing out.

            Updated remaining CLASSERT() under lnet.

            arshad512 Arshad Hussain added a comment - Aurelien, Thanks for pointing out. Updated remaining CLASSERT() under lnet.

            Arshad Hussain (arshad.super@gmail.com) 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

            gerrit Gerrit Updater added a comment - Arshad Hussain (arshad.super@gmail.com) 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

            Looks like lnet also needs one last patch:

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

             

            degremoa Aurelien Degremont (Inactive) added a comment - - edited Looks like lnet also needs one last patch: $ git grep -c CLASSERT lnet/ lnet/lnet/api-ni.c:96 lnet/utils/wirecheck.c:2  

            Arshad Hussain (arshad.super@gmail.com) 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

            gerrit Gerrit Updater added a comment - Arshad Hussain (arshad.super@gmail.com) 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

            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
            arshad512 Arshad Hussain added a comment - 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
            pjones Peter Jones added a comment -

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

            pjones Peter Jones added a comment - Everything currently in flight has landed - is this task now complete?

            Oleg Drokin (green@whamcloud.com) 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

            gerrit Gerrit Updater added a comment - Oleg Drokin (green@whamcloud.com) 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

            Oleg Drokin (green@whamcloud.com) 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 Gerrit Updater added a comment - Oleg Drokin (green@whamcloud.com) 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

            Oleg Drokin (green@whamcloud.com) 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 Gerrit Updater added a comment - Oleg Drokin (green@whamcloud.com) 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

            People

              arshad512 Arshad Hussain
              adilger Andreas Dilger
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: