Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Michael Ellerman <mpe@ellerman.id.au>
To:     Christophe Leroy <christophe.leroy@csgroup.eu>,
        "zhang.songyi@zte.com.cn" <zhang.songyi@zte.com.cn>,
        "arnd@arndb.de" <arnd@arndb.de>
Cc:     "npiggin@gmail.com" <npiggin@gmail.com>,
        "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        <deng.changcheng@zte.com.cn>
Subject: Re: [PATCH linux-next] powerpc/cell/axon_msi: replace
 DEFINE_SIMPLE_ATTRIBUTE with DEFINE_DEBUGFS_ATTRIBUTE
In-Reply-To: <9f6a3690-3ac3-8e13-a80c-b0571826c3d7@csgroup.eu>
References: <202211231706198927223@zte.com.cn>
 <9f6a3690-3ac3-8e13-a80c-b0571826c3d7@csgroup.eu>
Date:   Fri, 25 Nov 2022 16:30:08 +1100
Message-ID: <87tu2nbnz3.fsf@mpe.ellerman.id.au>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Hi,
>
> Le 23/11/2022 =C3=A0 10:06, zhang.songyi@zte.com.cn a =C3=A9crit=C2=A0:
>> From: zhang songyi <zhang.songyi@zte.com.cn>
>>=20
>> Fix the following coccicheck warning:
>> /arch/powerpc/platforms/cell/axon_msi.c:457:0-23: WARNING:
>> fops_msic should be defined with DEFINE_DEBUGFS_ATTRIBUTE
>
> What's the difference between this new patch and the one that is already=
=20
> awaiting application here :=20
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20211222090655.48=
4551-1-deng.changcheng@zte.com.cn/=20
> ?
>
> The only difference I see it that the already existing patch has a more=20
> complete description of the change, so unless I'm missing something it=20
> would be nice to avoid sending the same changes again and again.

Both patches switch the code to use a function called "unsafe" without
adequately explaining why that is OK.

The commit that added the cocci check script says:

  If the original struct file_operations are known to be safe against remov=
al
  races by themselves already, the proxy creation may be bypassed by creati=
ng
  the files through debugfs_create_file_unsafe().

None of these conversion patches ever contain any explanation which
speaks to that.

In this case I *think* the change is OK and there is no race because the
debugfs file is never removed. But I really wish the submitter would
tell me that in the change log.

cheers