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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4FF07C433F5 for ; Tue, 29 Mar 2022 14:31:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236913AbiC2Odf (ORCPT ); Tue, 29 Mar 2022 10:33:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35244 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231230AbiC2Ode (ORCPT ); Tue, 29 Mar 2022 10:33:34 -0400 Received: from mail-pj1-x102e.google.com (mail-pj1-x102e.google.com [IPv6:2607:f8b0:4864:20::102e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BD6401BEA8 for ; Tue, 29 Mar 2022 07:31:51 -0700 (PDT) Received: by mail-pj1-x102e.google.com with SMTP id h23-20020a17090a051700b001c9c1dd3acbso3049720pjh.3 for ; Tue, 29 Mar 2022 07:31:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=lsP70CCaXDaE2udz5nTAsHVpadgeSMC46x2B6kOKiJQ=; b=fMwa8vv5SwPhBGFC75W4BQQ5WY2CI99tuPzzJZ+X5ITg8Lh3RLEe1zM2G8WxMWv6IE j3GrtHftNkEbR3cU8tDBFJqgvT5/9GepeX7aPPwB8Zilv9QWyBgkArgiXlOdsjtb8gqa h/5CmpSX2N7oJqhoUqTJQiMf6Ll7bg8vlozCOc1w0fVPuwdV7fXwG0uNIg6CDR4yYsfB iBfHjakcc9pqzL2JJZytwv/EqaJIV4NeS0kTb606qFvhlPdVY4RJECWl5o84knLFk92r pLN4W+298EFOqb+EDY5FHclBHUxo6fG39kjg8v1nzl1QC/4ZdjBsx93cw8Rzq3YjHYWz m3og== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=lsP70CCaXDaE2udz5nTAsHVpadgeSMC46x2B6kOKiJQ=; b=l0m5ZEmD0URandByjUHLdQkZNZZObbGfEV+1xD3SnCGyKBO533CjXLUVVDGcFTRh0x +qfzL7Me+rWyestHiFnCR6KrofKQ59kff9PcL6cnvcx7vlBLGGGphbxN1Vo16tLm+yfw TPOMn01sAbIzxS24A82frYQb4JD1VwtZJoxAzWo7QQXnArBO3mc5HI/bRGmd3AhfceHN auqdu2++fymmpVCFOBo7BFf4/4F8qVsHDFzDeJCI8vLigOMgS/gpMFtozP+WJ+MViSLa PgNV65/ZalmlqCjfS6OfzjCA6zHNJP9QAxn8KC6vwVrZYNcSofl+662vR9GEjDCTVUCy BHKg== X-Gm-Message-State: AOAM532V9OSa4Aw75zrG092yapmy2UoSr6HGpoThlLn1ZRzRr0yYCpHm q2o6kHvFxVMcNCUQpVgwe2+y X-Google-Smtp-Source: ABdhPJzpE1FYDJsc5tTxrz8in/qSw0DgAFt3quMPCz0DVaHZ3rP6uWeZiHIBljgpxDUit+IE1ERO0g== X-Received: by 2002:a17:90b:4a83:b0:1c6:f037:bc73 with SMTP id lp3-20020a17090b4a8300b001c6f037bc73mr4811286pjb.44.1648564311071; Tue, 29 Mar 2022 07:31:51 -0700 (PDT) Received: from thinkpad ([117.217.181.81]) by smtp.gmail.com with ESMTPSA id c18-20020a056a000ad200b004cdccd3da08sm20983552pfl.44.2022.03.29.07.31.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Mar 2022 07:31:50 -0700 (PDT) Date: Tue, 29 Mar 2022 20:01:46 +0530 From: Manivannan Sadhasivam To: Mathieu Poirier Cc: bjorn.andersson@linaro.org, linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] remoteproc: Don't bother checking the return value of debugfs_create* Message-ID: <20220329143146.GA2137@thinkpad> References: <20220324181224.21542-1-manivannan.sadhasivam@linaro.org> <20220328155123.GA3722211@p14s> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220328155123.GA3722211@p14s> Precedence: bulk List-ID: X-Mailing-List: linux-remoteproc@vger.kernel.org Hi Mathieu, On Mon, Mar 28, 2022 at 09:51:23AM -0600, Mathieu Poirier wrote: > Hi Mani, > > On Thu, Mar 24, 2022 at 11:42:24PM +0530, Manivannan Sadhasivam wrote: > > DebugFS APIs are designed to return only the error pointers and not NULL > > in the case of failure. So these return pointers are safe to be passed on > > to the successive debugfs_create* APIs. > > > > Therefore, let's just get rid of the checks. > > > > Signed-off-by: Manivannan Sadhasivam > > --- > > drivers/remoteproc/remoteproc_debugfs.c | 17 ++--------------- > > 1 file changed, 2 insertions(+), 15 deletions(-) > > > > diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c > > index b5a1e3b697d9..2e2c4a31c154 100644 > > --- a/drivers/remoteproc/remoteproc_debugfs.c > > +++ b/drivers/remoteproc/remoteproc_debugfs.c > > @@ -386,16 +386,8 @@ void rproc_remove_trace_file(struct dentry *tfile) > > struct dentry *rproc_create_trace_file(const char *name, struct rproc *rproc, > > struct rproc_debug_trace *trace) > > { > > - struct dentry *tfile; > > - > > - tfile = debugfs_create_file(name, 0400, rproc->dbg_dir, trace, > > + return debugfs_create_file(name, 0400, rproc->dbg_dir, trace, > > &trace_rproc_ops); > > - if (!tfile) { > > - dev_err(&rproc->dev, "failed to create debugfs trace entry\n"); > > - return NULL; > > - } > > - > > - return tfile; > > Please see this thread [1] for an earlier conversation on this topic. > > [1]. https://lore.kernel.org/lkml/20220105131022.25247-1-linmq006@gmail.com/T/ > Thanks for the pointer! I believe the conclusion was to return 0 here and ignore the return from debugfs_create_file(). If that's the case, it looks fine to me and I'll send a follow-up patch. > > } > > > > void rproc_delete_debug_dir(struct rproc *rproc) > > @@ -411,8 +403,6 @@ void rproc_create_debug_dir(struct rproc *rproc) > > return; > > > > rproc->dbg_dir = debugfs_create_dir(dev_name(dev), rproc_dbg); > > - if (!rproc->dbg_dir) > > - return; > > > > debugfs_create_file("name", 0400, rproc->dbg_dir, > > rproc, &rproc_name_ops); > > @@ -430,11 +420,8 @@ void rproc_create_debug_dir(struct rproc *rproc) > > > > void __init rproc_init_debugfs(void) > > { > > - if (debugfs_initialized()) { > > + if (debugfs_initialized()) > > rproc_dbg = debugfs_create_dir(KBUILD_MODNAME, NULL); > > - if (!rproc_dbg) > > - pr_err("can't create debugfs dir\n"); > > - } > > The above two are fine since debugfs_create_file() and debugfs_create_dir() can > deal with @parent being an error code. > debugfs_create_* APIs would never return NULL, so these checks are wrong. Moreover, Greg recommends not to check the return value for any of these functions. I've found the mail thread where Greg explained the reasoning behind it: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1907800.html Thanks, Mani > Thanks, > Mathieu > > > } > > > > void __exit rproc_exit_debugfs(void) > > -- > > 2.25.1 > >