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

libcfs_debug_mb is not set correctly if module has not been initialized

    XMLWordPrintable

Details

    • 3
    • 9223372036854775807

    Description

      If libcfs_debug_mb parameter is specified to insmod (i.e. set before module is initialized) then it does not get initialized correctly. This appears to be a regression from:

      commit 8b78a3ffb5220330f41b4fa8576a05c4e017cfb1
      Author: Mr NeilBrown <neilb@suse.de>
      Date:   Sun Mar 8 21:35:57 2020 -0400
      
         LU-9859 libcfs: always range-check libcfs_debug_mb setting.
      
      diff --git a/libcfs/libcfs/debug.c b/libcfs/libcfs/debug.c
      index 99518ab116..e5bff2e18a 100644
      --- a/libcfs/libcfs/debug.c
      +++ b/libcfs/libcfs/debug.c
      @@ -65,21 +65,15 @@ static int libcfs_param_debug_mb_set(const char *val,
              if (rc < 0)
                      return rc;
      
      -/*
      - * RHEL6 does not support any kind of locking so we have to provide
      - * our own
      - */
      -       if (!*((unsigned int *)kp->arg)) {
      -               *((unsigned int *)kp->arg) = num;
      -               return 0;
      -       }
      +       num = cfs_trace_set_debug_mb(num);
      
      -       rc = cfs_trace_set_debug_mb(num);
      -
      -       if (!rc)
      -               *((unsigned int *)kp->arg) = cfs_trace_get_debug_mb();
      +       *((unsigned int *)kp->arg) = num;
      +       num = cfs_trace_get_debug_mb();
      +       if (num)
      +               /* This value is more precise */
      +               *((unsigned int *)kp->arg) = num;
      
      -       return rc;
      +       return 0;
       }
      

      I think the idea is that cfs_trace_get_debug_mb() would only return non-zero if the module had been initialized, so it's return value should be used instead of the return val from cfs_trace_set_debug_mb(). However, cfs_trace_get_debug_mb() never returns 0:

      int cfs_trace_get_debug_mb(void)
      {
      ...
              return (total_pages >> (20 - PAGE_SHIFT)) + 1;
      }
      

      We can see it will always return at least 1.

      Attachments

        Activity

          People

            hornc Chris Horn
            hornc Chris Horn
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: