All of lore.kernel.org
 help / color / mirror / Atom feed
From: Philo Lu <lulie@linux.alibaba.com>
To: linux-kernel@vger.kernel.org
Cc: akpm@linux-foudation.org, surenb@google.com, rppt@kernel.org,
	zhou.kete@h3c.com, zhao_lei1@hoperun.com, kunyu@nfschina.com,
	zhang.zhengming@h3c.com, gregkh@linuxfoundation.org,
	xuanzhuo@linux.alibaba.com, dust.li@linux.alibaba.com,
	alibuda@linux.alibaba.com, guwen@linux.alibaba.com,
	hengqi@linux.alibaba.com
Subject: [RFC PATCH] relay: avoid relay_open_buf inproperly fails in buffer-only mode
Date: Wed, 20 Dec 2023 15:47:25 +0800	[thread overview]
Message-ID: <20231220074725.23211-1-lulie@linux.alibaba.com> (raw)

In buffer-only mode, relay_open(NULL, NULL, ...) is used to create the
buffer first, where chan->has_base_filename is not set. Though we still
need to call chan->cb->create_buf_file in relay_open_buf() to retrieve
global info for global buffer, the create_buf_file callback should
return NULL. With IS_ERR_OR_NULL() check, relay_open fails because of
the returned NULL dentry, so this patch reverts back to the WARN_ON()
version and add a comment for this behavior.

Here is an example after fix:
```
struct dentry *my_create_buf_file(const char *filename,
            struct dentry *parent, umode_t mode,
            struct rchan_buf *buf, int *is_global)
{
    if (!filename)
        return NULL;

    return debugfs_create_file(filename, mode, parent, buf,
                &relay_file_operations);
}

relay_cb.create_buf_file = my_create_buf_file
relay_chan = relay_open(NULL, NULL,
                    subbuf_size, subbuf_num,
                    &relay_cb, NULL);
relay_late_setup_files(relay_chan, filename, parent);
```

But before fix, the create_buf_file callback must be something like:
```
struct dentry *my_create_buf_file(const char *filename,
            struct dentry *parent, umode_t mode,
            struct rchan_buf *buf, int *is_global)
{
    if (!filename)
        return ERR_PTR(1); // a valid ptr is necessary for relay_open

    return debugfs_create_file(filename, mode, parent, buf,
                &relay_file_operations);
}
```

I'm not sure if this revertion proper because it may break existing use
cases. I think we can also remove the WARN_ON check instead.

Fixes: 2c1cf00eeacb ("relay: check return of create_buf_file() properly")
Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
---
 kernel/relay.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/relay.c b/kernel/relay.c
index 83fe0325cde1..0700745447c1 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -395,7 +395,8 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
 		dentry = chan->cb->create_buf_file(NULL, NULL,
 						   S_IRUSR, buf,
 						   &chan->is_global);
-		if (IS_ERR_OR_NULL(dentry))
+		/* has_base_filename not set, so dentry should be NULL */
+		if (WARN_ON(dentry))
 			goto free_buf;
 	}

--
2.32.0.3.g01195cf9f


             reply	other threads:[~2023-12-20  7:47 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-20  7:47 Philo Lu [this message]
2023-12-20  8:07 ` [RFC PATCH] relay: avoid relay_open_buf inproperly fails in buffer-only mode Philo Lu
2024-01-12  3:00   ` Philo Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231220074725.23211-1-lulie@linux.alibaba.com \
    --to=lulie@linux.alibaba.com \
    --cc=akpm@linux-foudation.org \
    --cc=alibuda@linux.alibaba.com \
    --cc=dust.li@linux.alibaba.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=guwen@linux.alibaba.com \
    --cc=hengqi@linux.alibaba.com \
    --cc=kunyu@nfschina.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rppt@kernel.org \
    --cc=surenb@google.com \
    --cc=xuanzhuo@linux.alibaba.com \
    --cc=zhang.zhengming@h3c.com \
    --cc=zhao_lei1@hoperun.com \
    --cc=zhou.kete@h3c.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.