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 lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3B212C7EE24 for ; Sat, 3 Jun 2023 22:35:27 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4QYZSs3PJsz3cKj for ; Sun, 4 Jun 2023 08:35:25 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20221208 header.b=soghk3Eh; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gmail.com (client-ip=2607:f8b0:4864:20::c34; helo=mail-oo1-xc34.google.com; envelope-from=mirimmad17@gmail.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20221208 header.b=soghk3Eh; dkim-atps=neutral Received: from mail-oo1-xc34.google.com (mail-oo1-xc34.google.com [IPv6:2607:f8b0:4864:20::c34]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4QYMnH2ZD7z3c9R for ; Sun, 4 Jun 2023 00:33:54 +1000 (AEST) Received: by mail-oo1-xc34.google.com with SMTP id 006d021491bc7-558a79941c6so152802eaf.3 for ; Sat, 03 Jun 2023 07:33:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1685802829; x=1688394829; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=VqN/Cc43meDsfCoFH2RDmQekbZeIEiHB84/yvFErx6U=; b=soghk3EhGicini6WykbdCmbI1n6uHBbqq9W77HJd9a7XV0rYXvGmCD/9g6EoOLdVOO htYi98d9wMy4TScovrXnLUbjnYiEkXdw8mnJo/W9qiag8ZJn4c5BTHF/smN1w9bXQ0aN I9dgYYknNWs7vj5b3bpNWZMSwZg+jA04BTBR6cCg6KXFJexwQi0BtJSMvr2Lx910LDI0 xqQeOWmNeTkqBLz+x7vBecYMv0ZtKnEcWcwu6S8YNjKu59152DUV7kmJmfDd4HmHDNdh Pc2DOHl0FPq9+buxmOq9OjX0mLaPJbEYkCrXjBmptKL/qCpq0HykDWQPsdLqnmbPP5Hf Mn9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685802829; x=1688394829; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=VqN/Cc43meDsfCoFH2RDmQekbZeIEiHB84/yvFErx6U=; b=kVv32cQ/Cs/PlnfeXlA4TwKWyVgdpUpwzt4l85yOLO8xVqA3/bigf4kUEEP6SZccQH 6WHQtdDZUDKnaEYsOWZQufUu473AVsCjKUi485ROExK+UIQYCCrK1hYZnKOsvqjSgEzv tZytzVH4vJ2Mz4MGMXIr6dkMBDEhiOcA+8NTGkXGmf4ulScdZVPsKhu6QP8VXwuOXXIg RGySjbZduHO7N4PsE8GM4qiSARoZVK5UmiOzIxOapU4HTbXyN6iY0FaPPtBwlxA2NklW WINH8FdMU0axkTL7IU1j4bb2IBIOIkQcSiHI4Bjt67fPUeLa+vv3TMZ47lFgZ61gz6so nttw== X-Gm-Message-State: AC+VfDwLdsBn0N0wzYF+aRvysRw+wolDsm9A5n0idgV6Mz/JZ1gvoxiG PN+AqiqonglTg2/QB/s+jPR3ftm6w6qw9bV3QOI= X-Google-Smtp-Source: ACHHUZ6FfpI3WQwqrBxXpue9Y0Mig/aUvPVUiEsoYRmLue8TS/SK9CAoA5MabvpTHq1Agaqj7AA1GhWDsPVt6aL27+s= X-Received: by 2002:a4a:4548:0:b0:555:7c8b:910e with SMTP id y69-20020a4a4548000000b005557c8b910emr9771069ooa.1.1685802829092; Sat, 03 Jun 2023 07:33:49 -0700 (PDT) MIME-Version: 1.0 References: <2023052835-oxidant-doily-404f@gregkh> <87zg5mrt8m.fsf@mail.lhotse> In-Reply-To: <87zg5mrt8m.fsf@mail.lhotse> From: Immad Mir Date: Sat, 3 Jun 2023 20:03:37 +0530 Message-ID: Subject: Re: [PATCH] powerpc: fix debugfs_create_dir error checking To: Michael Ellerman Content-Type: multipart/alternative; boundary="000000000000ca1c1305fd3a8e97" X-Mailman-Approved-At: Sun, 04 Jun 2023 08:34:39 +1000 X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Ivan Orlov , Greg KH , mirimmad@outlook.com, linux-kernel@vger.kernel.org, Nicholas Piggin , linuxppc-dev@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" --000000000000ca1c1305fd3a8e97 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable > Still I think this patch is an improvement so I'll plan to merge it. Please let me know when you commit it. Thanks Immad. On Tue, May 30, 2023 at 4:17=E2=80=AFPM Michael Ellerman wrote: > Greg KH writes: > > On Sun, May 28, 2023 at 01:16:44PM +0530, mirimmad@outlook.com wrote: > >> From: Immad Mir > >> > >> The debugfs_create_dir returns ERR_PTR incase of an error and the > >> correct way of checking it by using the IS_ERR inline function, and > >> not the simple null comparision. This patch fixes this. > >> > >> Suggested-By: Ivan Orlov > >> Signed-off-by: Immad Mir > >> --- > >> arch/powerpc/platforms/powernv/opal-xscom.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/powerpc/platforms/powernv/opal-xscom.c > b/arch/powerpc/platforms/powernv/opal-xscom.c > >> index 6b4eed2ef..262cd6fac 100644 > >> --- a/arch/powerpc/platforms/powernv/opal-xscom.c > >> +++ b/arch/powerpc/platforms/powernv/opal-xscom.c > >> @@ -168,7 +168,7 @@ static int scom_debug_init_one(struct dentry *root= , > struct device_node *dn, > >> ent->path.size =3D strlen((char *)ent->path.data); > >> > >> dir =3D debugfs_create_dir(ent->name, root); > >> - if (!dir) { > >> + if (IS_ERR(dir)) { > >> kfree(ent->path.data); > >> kfree(ent); > >> return -1; > > > > Why is this driver caring if debugfs is working or not at all? It > > should just ignore the error and keep moving forward. > > It's creating directories and then creating files in those directories. > So I think it makes sense that it checks that the directory was created > successfully. It doesn't check whether the files were created. > > > And -1 is not a valid error number :( > > It's EPERM :) - but yeah probably not really the right error in this > case. > > Still I think this patch is an improvement so I'll plan to merge it. > > cheers > --000000000000ca1c1305fd3a8e97 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
> Still I think this patch is an improvement so I&= #39;ll plan to merge it.

Please let me know when y= ou commit it.

Thanks
Immad.

On = Tue, May 30, 2023 at 4:17=E2=80=AFPM Michael Ellerman <mpe@ellerman.id.au> wrote:
Greg KH <gregkh@linuxfoundation.org> = writes:
> On Sun, May 28, 2023 at 01:16:44PM +0530, mirimmad@outlook.com wrote:
>> From: Immad Mir <mirimmad17@gmail.com>
>>
>> The debugfs_create_dir returns ERR_PTR incase of an error and the<= br> >> correct way of checking it by using the IS_ERR inline function, an= d
>> not the simple null comparision. This patch fixes this.
>>
>> Suggested-By: Ivan Orlov <ivan.orlov0322@gmail.com>
>> Signed-off-by: Immad Mir <mirimmad17@gmail.com>
>> ---
>>=C2=A0 arch/powerpc/platforms/powernv/opal-xscom.c | 4 ++--
>>=C2=A0 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/opal-xscom.c b/arch/po= werpc/platforms/powernv/opal-xscom.c
>> index 6b4eed2ef..262cd6fac 100644
>> --- a/arch/powerpc/platforms/powernv/opal-xscom.c
>> +++ b/arch/powerpc/platforms/powernv/opal-xscom.c
>> @@ -168,7 +168,7 @@ static int scom_debug_init_one(struct dentry *= root, struct device_node *dn,
>>=C2=A0 =C2=A0 =C2=A0 ent->path.size =3D strlen((char *)ent->p= ath.data);
>>
>>=C2=A0 =C2=A0 =C2=A0 dir =3D debugfs_create_dir(ent->name, root)= ;
>> -=C2=A0 =C2=A0 if (!dir) {
>> +=C2=A0 =C2=A0 if (IS_ERR(dir)) {
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 kfree(ent->path= .data);
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 kfree(ent);
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -1;
>
> Why is this driver caring if debugfs is working or not at all?=C2=A0 I= t
> should just ignore the error and keep moving forward.

It's creating directories and then creating files in those directories.=
So I think it makes sense that it checks that the directory was created
successfully. It doesn't check whether the files were created.

> And -1 is not a valid error number :(

It's EPERM :) - but yeah probably not really the right error in this case.

Still I think this patch is an improvement so I'll plan to merge it.
cheers
--000000000000ca1c1305fd3a8e97--