On 5/26/2021 5:43 PM, Vlastimil Babka wrote: > On 5/26/21 1:48 PM, Greg KH wrote: >> On Wed, May 26, 2021 at 01:38:55PM +0200, Vlastimil Babka wrote: >>> >>> alias_list a single list and both slab_sysfs_init() and slab_debugfs_init() >>> flush it. So only the init call that happens to be called first, does actually >>> find an unflushed list. I think you >>> need to use a separate list for debugfs (simpler) or a shared list with both >>> sysfs and debugfs processing (probably more complicated). >>> >>> And finally a question, perhaps also for Greg. With sysfs, we hand out the >>> lifecycle of struct kmem_cache to sysfs, to ensure we are not reading sysfs >>> files of a cache that has been removed. >>> >>> But with debugfs, what are the guarantees that things won't blow up when a >>> debugfs file is being read while somebody calls kmem_cache_destroy() on the cache? >> >> It's much harder, but usually the default debugfs_file_create() will >> handle this for you. See the debugfs_file_create_unsafe() for the >> "other" variant where you know you can tear things down "safely". > > Right, so IIUC debugfs will guarantee that while somebody reads the files, the > debugfs cleanup will block, as debugfs_file_get() comment explains. > > In that case I think we have the cleanup order wrong in this patch: > > shutdown_cache() should first do debugfs_slab_release() (which would block) and > only then proceed with slab_kmem_cache_release() which destroys the fundamental > structures such as kmem_cache_node, which are also accessed by the debugfs file > handlers. > If user is trying to read the data during shutdown_cache(), then I think it's possible user will get empty data, to avoid that we can call debugfs_slab_release() first and then do other stuff in shutdown_cache(). >> That being said, yes there are still issues in this area, be careful >> about what tools you expect to be constantly hitting debugfs files. > > FWIW, the files are accessible only to root. > >> thanks, >> >> greg k-h >> >
