On 4/19/20 4:05 PM, Luis Chamberlain wrote:
On Sun, Apr 19, 2020 at 03:57:58PM -0700, Bart Van Assche wrote:
On 4/19/20 12:45 PM, Luis Chamberlain wrote:
Even though debugfs can be disabled, enabling BLK_DEV_IO_TRACE will
select DEBUG_FS, and blktrace exposes an API which userspace uses
relying on certain files created in debugfs. If files are not created
blktrace will not work correctly, so we do want to ensure that a
blktrace setup creates these files properly, and otherwise inform
userspace.
Signed-off-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
---
kernel/trace/blktrace.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 9cc0153849c3..fc32a8665ce8 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -552,17 +552,19 @@ static int blk_trace_create_debugfs_files(struct blk_user_trace_setup *buts,
struct dentry *dir,
struct blk_trace *bt)
{
- int ret = -EIO;
-
bt->dropped_file = debugfs_create_file("dropped", 0444, dir, bt,
&blk_dropped_fops);
+ if (!bt->dropped_file)
+ return -ENOMEM;
bt->msg_file = debugfs_create_file("msg", 0222, dir, bt, &blk_msg_fops);
+ if (!bt->msg_file)
+ return -ENOMEM;
bt->rchan = relay_open("trace", dir, buts->buf_size,
buts->buf_nr, &blk_relay_callbacks, bt);
if (!bt->rchan)
- return ret;
+ return -EIO;
return 0;
}
I should have had a look at this patch before I replied to the previous
patch.
Do you agree that the following code can be triggered by
debugfs_create_file() and also that debugfs_create_file() never returns
NULL?
If debugfs is enabled, and not that we know it is in this blktrace code,
as we select it, it can return ERR_PTR(-ERROR) if an error occurs.
This is what I found in include/linux/debugfs.h in case debugfs is disabled:
static inline struct dentry *debugfs_create_file(const char *name,
umode_t mode, struct dentry *parent, void *data,
const struct file_operations *fops)
{
return ERR_PTR(-ENODEV);
}
I have not found any code path that can cause debugfs_create_file() to
return NULL. Did I perhaps overlook something? If not, it's not clear to
me why the above patch adds checks that check whether
debugfs_create_file() returns NULL?
Thanks,
Bart.
