From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-101.freemail.mail.aliyun.com (out30-101.freemail.mail.aliyun.com [115.124.30.101]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E94AB1CA93 for ; Wed, 20 Dec 2023 08:07:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R451e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018046059;MF=lulie@linux.alibaba.com;NM=1;PH=DS;RN=14;SR=0;TI=SMTPD_---0VyteJLE_1703059645; Received: from 30.221.128.103(mailfrom:lulie@linux.alibaba.com fp:SMTPD_---0VyteJLE_1703059645) by smtp.aliyun-inc.com; Wed, 20 Dec 2023 16:07:26 +0800 Message-ID: <3b2805c0-daa1-4d2f-a3b3-4376a51ab219@linux.alibaba.com> Date: Wed, 20 Dec 2023 16:07:24 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH] relay: avoid relay_open_buf inproperly fails in buffer-only mode From: Philo Lu To: linux-kernel@vger.kernel.org Cc: akpm@linux-foundation.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 References: <20231220074725.23211-1-lulie@linux.alibaba.com> In-Reply-To: <20231220074725.23211-1-lulie@linux.alibaba.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Sorry, the email address of Andrew was wrongly spelled, so CC again. On 2023/12/20 15:47, Philo Lu wrote: > 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 > --- > 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