Uploaded image for project: 'Lustre'
  1. Lustre
  2. LU-16890

allow OBD_FREE() to ignore NULL pointers

Details

    • Improvement
    • Resolution: Fixed
    • Minor
    • Lustre 2.16.0
    • None
    • 3
    • 9223372036854775807

    Description

      The OBD_FREE(ptr) macro will internally (via OBD_FREE_PRE()) assert that ptr != NULL, even though the kernel accepts kfree(NULL) without complaint.

      It would make sense to remove the LASSERT(ptr) call from OBD_FREE_PRE() and change the callers to silently accept this case:

      #define OBD_FREE(ptr, size)                                                  \
if (likely(ptr)) {                                                           \
        OBD_FREE_PRE(ptr, size, "kfreed");                                   \
        POISON(ptr, 0x5a, size);                                             \
        kfree(ptr);                                                          \
        POISON_PTR(ptr);                                                     \
}

#define OBD_SLAB_FREE(ptr, slab, size)                                       \
if (likely(ptr)) {                                                           \
        OBD_FREE_PRE(ptr, size, "slab-freed");                               \
        POISON(ptr, 0x5a, size);                                             \
        kmem_cache_free(slab, ptr);                                          \
        POISON_PTR(ptr);                                                     \
}

      It isn't strictly needed for OBD_FREE_LARGE() since is_vmalloc_addr(ptr) will return false for a NULL pointer and fall back to OBD_FREE().

      Attachments

        Issue Links

          is related to

          Bug - A problem which impairs or prevents the functions of the product. LU-16688 Client crash due to an assertion in lite_lib.c:3394

          • Critical - Crashes, loss of data, severe memory leak.
          • Resolved

          Task - A task that needs to be done. LU-18141 Free'ing macros already check NULL

          • Minor - Minor loss of function, or other problem where easy workaround is present.
          • Resolved

          Activity

            [LU-16890] allow OBD_FREE() to ignore NULL pointers
            adilger Andreas Dilger made changes -
            Link New: This issue is related to LU-18141 [ LU-18141 ]
            bobijam Zhenyu Xu made changes -
            Link New: This issue is related to LU-16688 [ LU-16688 ]
            pjones Peter Jones made changes -
            Fix Version/s New: Lustre 2.16.0 [ 15190 ]
            Resolution New: Fixed [ 1 ]
            Status Original: Open [ 1 ] New: Resolved [ 5 ]
            pjones Peter Jones added a comment -

            Landed for 2.16

            pjones Peter Jones added a comment - Landed for 2.16
            gerrit Gerrit Updater added a comment -

            "Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/51332/
            Subject: LU-16890 obd: OBD_FREE_PRE() to ignore NULL pointers
            Project: fs/lustre-release
            Branch: master
            Current Patch Set:
            Commit: 46a9abf4330e7a12fde41fb922d6dfc4547c6243

            gerrit Gerrit Updater added a comment - "Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/51332/ Subject: LU-16890 obd: OBD_FREE_PRE() to ignore NULL pointers Project: fs/lustre-release Branch: master Current Patch Set: Commit: 46a9abf4330e7a12fde41fb922d6dfc4547c6243
            gerrit Gerrit Updater added a comment -

            "Arshad Hussain <arshad.hussain@aeoncomputing.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/51332
            Subject: LU-16890 obd: OBD_FREE_PRE() to ignore NULL pointers
            Project: fs/lustre-release
            Branch: master
            Current Patch Set: 1
            Commit: 9f42be0e1c77aa1e3914a52854f51ac07ab37f4a

            gerrit Gerrit Updater added a comment - "Arshad Hussain <arshad.hussain@aeoncomputing.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/51332 Subject: LU-16890 obd: OBD_FREE_PRE() to ignore NULL pointers Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: 9f42be0e1c77aa1e3914a52854f51ac07ab37f4a
            adilger Andreas Dilger made changes -
            Description Original: The {{OBD_FREE(ptr)}} macro will internally (via {{{}OBD_FREE_PRE(){}}}) assert that {{{}ptr != NULL{}}}, even though the kernel accepts {{kfree(NULL)}} without complaint.

            It would make sense to remove the {{LASSERT(ptr)}} call from {{OBD_FREE_PRE()}} and change the callers to silently accept this case:
            {code:java}
            #define OBD_FREE(ptr, size) \
            if (likely(ptr)) { \
                    OBD_FREE_PRE(ptr, size, "kfreed"); \
                    POISON(ptr, 0x5a, size); \
                    kfree(ptr); \
                    POISON_PTR(ptr); \
            }

            #define OBD_SLAB_FREE(ptr, slab, size)                                      \
            if (likely(ptr)) {                                                          \
                    OBD_FREE_PRE(ptr, size, "slab-freed");                              \
                    POISON(ptr, 0x5a, size);                                            \
                    kmem_cache_free(slab, ptr);                                        \
                    POISON_PTR(ptr);                                                    \
            }
            {code}

            It isn't strictly needed for {{OBD_SLAB_FREE()}} since {{is_vmalloc_addr(ptr)}} will return false for a NULL pointer and fall back to {{OBD_FREE()}}.             		New: The {{OBD_FREE(ptr)}} macro will internally (via {{{}OBD_FREE_PRE(){}}}) assert that {{{}ptr != NULL{}}}, even though the kernel accepts {{kfree(NULL)}} without complaint.

            It would make sense to remove the {{LASSERT(ptr)}} call from {{OBD_FREE_PRE()}} and change the callers to silently accept this case:
            {code:java}
            #define OBD_FREE(ptr, size) \
            if (likely(ptr)) { \
                    OBD_FREE_PRE(ptr, size, "kfreed"); \
                    POISON(ptr, 0x5a, size); \
                    kfree(ptr); \
                    POISON_PTR(ptr); \
            }

            #define OBD_SLAB_FREE(ptr, slab, size)                                      \
            if (likely(ptr)) {                                                          \
                    OBD_FREE_PRE(ptr, size, "slab-freed");                              \
                    POISON(ptr, 0x5a, size);                                            \
                    kmem_cache_free(slab, ptr);                                        \
                    POISON_PTR(ptr);                                                    \
            }
            {code}
            It isn't strictly needed for {{OBD_FREE_LARGE()}} since {{is_vmalloc_addr(ptr)}} will return false for a NULL pointer and fall back to {{{}OBD_FREE(){}}}.
            simmonsja James A Simmons made changes -
            Assignee Original: WC Triage [ wc-triage ] New: Arshad Hussain [ arshad512 ]
            adilger Andreas Dilger made changes -
            Labels New: easy
            adilger Andreas Dilger created issue -

            People

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

              Dates

                Created:
                Updated:
                Resolved: