On 5/31/21 9:11 AM, Faiyaz Mohammed wrote: > > > 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 Empty data is fine, when the cache is going away anyway. > debugfs_slab_release() first and then do other stuff in shutdown_cache(). Everything above list_del(&s->list) should be OK, it's equivalent to normal cache operations which the debugfs files must cope with anyway. list_del(&s->list) is OK as the debugfs handlers don't go through the list. It's slab_kmem_cache_release() that matters. >>> 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 >>> >> >
