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

Adapt to 3.10 upstream kernel proc_dir_entry change

    XMLWordPrintable

Details

    • Improvement
    • Resolution: Fixed
    • Minor
    • Lustre 2.6.0
    • Lustre 2.5.0
    • None
    • 8278

    Description

      In kernel 3.10 merge window, there is an important change in procfs subsystem that hides all details of proc_dir_entry structure.

      This affects Lustre in two ways. First is that the read_proc_t and write_proc_t members are now removed so Lustre has to be converted to use seq_file approach. The second one is that Lustre can no longer do child entry lookup inside lprocfs_srch.

      With the private proc_dir_entry structure and related exported APIs, the only possible/safe way to do child lookup is through path walking. Each pde has a atomic counter that is initialized to 1. And remove_proc_entry will decrease the counter to 0 and the free the pde. The counter is only increased by procfs when the corresponding proc dentry is instantiated during path walk. And it is decreased when the proc inode is evicted. There is no way an external module can hold a ref count on the dentry thus a pde can go away at any time if remove_proc_entry is called on it.

      Therefore, callers has to make sure that they don't use any removed pde directly. OTOH, Lustre saves its created pde and passes it around in many places. Later those pointers are used without any protection. Would it be a problem? The situation is true for pre-3.10 kernel as well because Lustre never holds references on proc_dir_entry.

      The existence of LPROCFS_ENTRY_AND_CHECK() also seems confusing. It indicates that a pde may be already invalidated when accessed by Lustre. However, all callers of LPROCFS_ENTRY_AND_CHECK() are in fact part of pde->proc_fops, which is called in side of proc_reg_file_ops where pde is protected by pde->in_use counter (pde_users for older kernels). When pde->in_use is positive, there is no way a pde can be removed. So IMOH LPROCFS_ENTRY_AND_CHECK() seems unnecessary.

      Also, it seems that _lprocfs_lock is to protect child pde insert/remove/search, whereas kernel already has proc_subdir_lock doing the same job. But lprocfs_srch is the only exception. If we can get rid of lprocfs_srch, I think we can get rid of _lprocfs_lock as well. Since 3.10 is still under development, we can make a patch to procfs exporting a search function and send it to upstream. In this way, lprocfs_srch can be removed.

      Also, removing LPROCFS_ENTRY_AND_CHECK/_lprocfs_lock or not, we cannot dereference a removed pde directly. Is there any existing mechanism to make sure it doesn't happen? Many proc_dir_entry are created by Lustre and passed around without protection. It seems dangerous to me but since it is working so far, maybe I am missing something?

      Attachments

        1. 0001-proc-add-a-child-pde-search-helper.patch
          2 kB
          Peng Tao
        2. 0002-staging-lustre-adapt-proc_dir_entry-change.patch
          216 kB
          Peng Tao
        3. proc.c
          0.6 kB
          Peng Tao
        4. remove_lprocfs_srch.patch
          19 kB
          Peng Tao

        Issue Links

          Activity

            People

              laisiyao Lai Siyao
              bergwolf Peng Tao
              Votes:
              0 Vote for this issue
              Watchers:
              19 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: