From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F0F2CC3815B for ; Mon, 20 Apr 2020 11:40:42 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id AC4FD21473 for ; Mon, 20 Apr 2020 11:40:42 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="0qFzLT8y" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AC4FD21473 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linuxfoundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 432F68E0006; Mon, 20 Apr 2020 07:40:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3E3858E0003; Mon, 20 Apr 2020 07:40:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2D25F8E0006; Mon, 20 Apr 2020 07:40:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0120.hostedemail.com [216.40.44.120]) by kanga.kvack.org (Postfix) with ESMTP id 129A98E0003 for ; Mon, 20 Apr 2020 07:40:42 -0400 (EDT) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id C525F181AEF23 for ; Mon, 20 Apr 2020 11:40:41 +0000 (UTC) X-FDA: 76728041082.21.spade80_7a719c0330043 X-HE-Tag: spade80_7a719c0330043 X-Filterd-Recvd-Size: 4911 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf26.hostedemail.com (Postfix) with ESMTP for ; Mon, 20 Apr 2020 11:40:41 +0000 (UTC) Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 1B254206D4; Mon, 20 Apr 2020 11:40:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1587382840; bh=puYU9WIOHWq+Hgrh+zyx6scXqHgdvOkozi99sNN0Pog=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=0qFzLT8yL6d5XnLUkMcfIVddV0ucPWMlKxCK5PWu01WNoEQ8Dx9OMoRUarC9rdG4K KTmbYy+MiyyJZMtFK2b/YyvMX5uVz3oiPsCkCNr0Q4TUvNsrM++UDpTV2QQMb7eh4P 1MaNZn4etW6JBXajC/+CZSVlIXfmPk6wAY+wwnTc= Date: Mon, 20 Apr 2020 13:40:38 +0200 From: Greg KH To: Bart Van Assche Cc: Luis Chamberlain , axboe@kernel.dk, viro@zeniv.linux.org.uk, rostedt@goodmis.org, mingo@redhat.com, jack@suse.cz, ming.lei@redhat.com, nstange@suse.de, akpm@linux-foundation.org, mhocko@suse.com, yukuai3@huawei.com, linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 08/10] blktrace: add checks for created debugfs files on setup Message-ID: <20200420114038.GE3906674@kroah.com> References: <20200419194529.4872-1-mcgrof@kernel.org> <20200419194529.4872-9-mcgrof@kernel.org> <38240225-e48e-3035-0baa-4929948b23a3@acm.org> <20200419230537.GG11244@42.do-not-panic.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sun, Apr 19, 2020 at 04:17:46PM -0700, Bart Van Assche wrote: > 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 > > > > --- > > > > 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? Short answer, yes, it can return NULL. Correct answer is, you don't care, don't check the value and don't do anything about it. It's debugging code, userspace doesn't care, so just keep moving on. thanks, greg k-h