On 5/31/21 9:11 AM, Faiyaz Mohammed wrote:

On 5/26/2021 5:43 PM, Vlastimil Babka wrote:

quoted

On 5/26/21 1:48 PM, Greg KH wrote:

quoted

On Wed, May 26, 2021 at 01:38:55PM +0200, Vlastimil Babka wrote:

quoted

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.

quoted

quoted

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.

quoted

thanks,

greg k-h