Bug 208301 - debugfs: Directory 'loop0' with parent 'block' already present!
Summary: debugfs: Directory 'loop0' with parent 'block' already present!
Status: NEW
Alias: None
Product: IO/Storage
Classification: Unclassified
Component: Block Layer (show other bugs)
Hardware: All Linux
: P1 low
Assignee: Jens Axboe
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-06-23 16:55 UTC by Luis Chamberlain
Modified: 2020-06-23 16:56 UTC (History)
2 users (show)

See Also:
Kernel Version: v5.7.5
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description Luis Chamberlain 2020-06-23 16:55:46 UTC
This bug is present *very* likely on older stable kernels as well. It should be easy to test.

As part of the work to fix korg#205713 a series of fixes are now merged on the block branch, scheduled to be merged for v5.9. Despite these fixes there is still one lingering issue, if one runs break-blktrace [1] run_0004.sh, even with all the fixes as of linux-next tag 20200622, one still observes the following on the kernel log:

[235530.144343] debugfs: Directory 'loop0' with parent 'block' already present![235530.149477] blktrace: debugfs_dir not present for loop0 so skipping
[235530.232328] debugfs: Directory 'loop0' with parent 'block' already present![235530.238962] blktrace: debugfs_dir not present for loop0 so skipping

I have inspected the block layer locking and the only thing I can think of is that there is a discrepancy / race in between a sysfs addition of a block device name Vs the creation / removal of the respective debugfs_dir for make_request drivers (multiqueue). After the fixes were merged, this would imply this should also be possible as a race with request-based block devices as well, as of the future v5.9 or on linux-next, as we now create the debugfs_dir also on initialization for them as well, however I have not confirmed this.

When this happens the dentry returned should be:

dentry = ERR_PTR(-EEXIST);

Given:

static struct dentry *start_creating(const char *name, struct dentry *parent)   
{
   ...
        if (!IS_ERR(dentry) && d_really_is_positive(dentry)) {                  
                if (d_is_dir(dentry))                                           
                        pr_err("Directory '%s' with parent '%s' already present!\n",
                               name, parent->d_name.name);                      
                else                                                            
                        pr_err("File '%s' in directory '%s' already present!\n",
                               name, parent->d_name.name);                      
                dput(dentry);                                                   
                dentry = ERR_PTR(-EEXIST);                                      
        } 
        if (IS_ERR(dentry)) {                                                   
                inode_unlock(d_inode(parent));                                  
                simple_release_fs(&debugfs_mount, &debugfs_mount_count);        
        }                                                                       
                                                                                
        return dentry; 
}                                                                      

Since we now check for the debugfs_dir early, but not on initialization this should mean we simply disable blktrace for the life time of that device. Prior to the fixes merged, I *suspect* it may have been possible for dentries to be populated on the debugfs root directory instead of the debugfs_dir and cleaned up upon removal, but I have no confirmed this.

[0] https://bugzilla.kernel.org/show_bug.cgi?id=205713
[1] https://github.com/mcgrof/break-blktrace

Note You need to log in before you can comment on or make changes to this bug.