Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Neil Brown <neilb@suse.de>
CC:     David Disseldorp <ddiss@suse.de>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        Jeff Layton <jlayton@redhat.com>
Subject: Re: [PATCH] exportfs: use pr_debug for unreachable debug statements
Thread-Topic: [PATCH] exportfs: use pr_debug for unreachable debug statements
Thread-Index: AQHY5UgGkwnmp58/L0OX2xyj/M+Kj64cjjqAgAE8nYCAAFVLAIAENJmA
Date:   Thu, 27 Oct 2022 14:23:14 +0000
Message-ID: <1D5F6D51-DE1C-4486-8907-12B9993CCAB7@oracle.com>
References: <20221021122414.20555-1-ddiss@suse.de>
 <166656308707.12462.9861114416829680469@noble.neil.brown.name>
 <000A2614-4C72-444A-A1D3-7B259D99C70A@oracle.com>
 <166664939677.12462.18426253960350585268@noble.neil.brown.name>
In-Reply-To: <166664939677.12462.18426253960350585268@noble.neil.brown.name>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?6NdsQGGmilcLKOq3gAeS9FZWuhg+9X6hUI+AE4ywwksAnmp2L7UwqLVsx/nJ?=
 =?us-ascii?Q?qntKNgbo/6CkDYFqgujau64YgW5y//BYCwUN/QOmsy8QPPotda+thUNsdwNt?=
 =?us-ascii?Q?f1kfS5bD0srvFePB/8lZ0fg8tlSA1etf6CjRvSm+pC0Y73rwWMm/LeylyO72?=
 =?us-ascii?Q?D0uIg4Y6LkMBaoSUZemFWK71IAdccztIfAQmFjMEiXWF7yanv40mSCXkCUUC?=
 =?us-ascii?Q?NGjEEtECwWlW4CuScP+jZkIg0YZYDwvrknEUlq5rdLrWCyNXnJFZ/r3j8uuX?=
 =?us-ascii?Q?ug4wkx8kXuDeM2E6L+7w7JH5bSChBee05amlr//z90geLE2830PaLF8j+NSq?=
 =?us-ascii?Q?xY7WtwFoWqjTz4HK91tJrm0phPtzhjprojh39CaQgX0JL2Xk4HBw9fVOz6o6?=
 =?us-ascii?Q?aFV5Pz2UdLXGoTkjW4Ry3oSuqeUCrnCnWKifr9eDMalWmpsE3LwIIgxxtqKa?=
 =?us-ascii?Q?O83Tizf3cfhp9njmS5BAUV2PuJRpw8yhr0mSppgA7HeaML3kvzP/nrO+kFYC?=
 =?us-ascii?Q?0/NlquQtiwte+8q48N7WNRtxd2bRA4Q1WS/0Cu1f6r3aBjXZvUWsdyf2/art?=
 =?us-ascii?Q?/KW6cRZP1+3HuR59gzkk1qfdJ1E/QaHwdbnnYoaAct9+zjbfd26+rEbhwg0x?=
 =?us-ascii?Q?ZPHOWinBAg/NQNlrNEgrLeh39EJq7xzH9NTsX1FkCISnzm3FV2OzPFjXd2o0?=
 =?us-ascii?Q?3K0qJqh5tkLT2x+ivf4zqDcFIP6MBbmKLuHzVrRaMMbUWH5t0AM1WCyn8wCl?=
 =?us-ascii?Q?tyywtOpBQJvxwG6H4efLBdbU/zTXFM77n+eyIYkDKA/5J+hv/RAV5P9LA4Rx?=
 =?us-ascii?Q?pom9Mqci4VPB7FaW255QX0L/ZSoYyv2oSsX/hEZiUrw135kDOTA1J/T9wP4z?=
 =?us-ascii?Q?YapAtmLirA/oXGVJm7esePuZowNcHhfVWzX4mbmQmOely5lKGdQUn5GfURvI?=
 =?us-ascii?Q?basHMsMHL6X6yhsvwYH/lT2zRP28Ea66F2x0RnNp5Fs09uB4svf9cjjWjjA3?=
 =?us-ascii?Q?h7rE9gb0NuoTwqMWRQSMKWhTQw3AHtDys+e9rvoKj08NIukxsPtZE0ZbXhGS?=
 =?us-ascii?Q?RPtcEZXT5RAuiYC8OIdqh6qKL0C/n42/axtxVxcIUww4KvWSN6b7UoRl3xHF?=
 =?us-ascii?Q?99X9BeTuxgEboxar5W9caxvb2vgUbzv4uoGAUujOUGl6GiYVvgy4/3aJAqHw?=
 =?us-ascii?Q?vOrrA42Aqyj0OCZaUk5Npk38rAuZUGkinc/UEBy/a2S+3A3/8RYNhOK4HklD?=
 =?us-ascii?Q?tB11B+Bi/9HrXOhqZ5kqQtTlaLgCqcZsA8/hQOzmnKZ027K/DYIHC3PLTsqW?=
 =?us-ascii?Q?S4Mqa+h5fWIc6gQaEMzG9IrvDCnH8PhFiEihLayrxfpj3ILV0SIoQNvO9BY2?=
 =?us-ascii?Q?CG9V1650Nx9zfKFcrWR+tqIFEdCuixtamiQu7TkeE9c63MTRF2IfRpM5p/d+?=
 =?us-ascii?Q?WphKC/SZ0sdism2Cp3Bv0+H6exqVoUBKhcR6wMN5Ob3NC0ZpgM7Jul3GAGWp?=
 =?us-ascii?Q?VhG49HYlAwIab57Vgj3FbwKmtjgNKOxwNGT6DLXy1kEh8MwN42HTfUZKG6XG?=
 =?us-ascii?Q?tre2YwgNNuNX0tsU0slKtnjRVGioDgul+gvjBfwM+5Q0pO/WUtKC99x5gnOX?=
 =?us-ascii?Q?QA=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <91DA2B81512DC84DB1BC0296CACBC9D7@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4fe050eb-e1db-449a-d682-08dab826c90c
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Oct 2022 14:23:14.7051
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Rg3zw5kbHiNcV0wcOebKqruTOCKCuX5HN+x9KcYTZpPhqk2J8C9TjXGma9vxH4wJ2sc1kUhsgkp0hjQjI4Rvag==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR10MB5087
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-10-27_07,2022-10-27_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 adultscore=0
 mlxlogscore=999 malwarescore=0 suspectscore=0 phishscore=0 mlxscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2210170000 definitions=main-2210270078
X-Proofpoint-GUID: _p95on3-wb_EbAg0O2w2sP_KbUU-YDJz
X-Proofpoint-ORIG-GUID: _p95on3-wb_EbAg0O2w2sP_KbUU-YDJz
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Oct 24, 2022, at 6:09 PM, NeilBrown <neilb@suse.de> wrote:
>=20
> On Tue, 25 Oct 2022, Chuck Lever III wrote:
>>=20
>>> On Oct 23, 2022, at 6:11 PM, NeilBrown <neilb@suse.de> wrote:
>>>=20
>>> On Fri, 21 Oct 2022, David Disseldorp wrote:
>>>> expfs.c has a bunch of dprintk statements which are unusable due to:
>>>> #define dprintk(fmt, args...) do{}while(0)
>>>> Use pr_debug so that they can be enabled dynamically.
>>>> Also make some minor changes to the debug statements to fix some
>>>> incorrect types, and remove __func__ which can be handled by dynamic
>>>> debug separately.
>>>>=20
>>>> Signed-off-by: David Disseldorp <ddiss@suse.de>
>>>=20
>>> Reviewed-by: NeilBrown <neilb@suse.de>
>>>=20
>>> Thanks,
>>> NeilBrown
>>=20
>> I don't think we're the maintainers of expfs.c ?
>>=20
>> $ scripts/get_maintainer.pl fs/exportfs/expfs.c
>> Christian Brauner <brauner@kernel.org> (commit_signer:2/2=3D100%,authore=
d:1/2=3D50%,added_lines:3/6=3D50%,removed_lines:2/6=3D33%)
>> Al Viro <viro@zeniv.linux.org.uk> (commit_signer:1/2=3D50%,authored:1/2=
=3D50%,added_lines:3/6=3D50%,removed_lines:4/6=3D67%)
>> Miklos Szeredi <mszeredi@redhat.com> (commit_signer:1/2=3D50%)
>> Amir Goldstein <amir73il@gmail.com> (commit_signer:1/2=3D50%)
>> linux-kernel@vger.kernel.org (open list)
>>=20
>> But maybe MAINTAINERS needs to be fixed. There's no entry
>> there for fs/exportfs.
>=20
> Looking at recent commits, patches come in through multiple different
> trees.
> nfsd certainly has an interest in expfs.c.  The only other user is
> name_to_handle/open_by_handle API.
> I see it as primarily nfsd functionality which is useful enough to be
> exported directly to user-space.
> (It was created by me when I was nfsd maintainer - does that count?)

I can mechanically take the patch through nfsd if no-one objects.
My concern now is that it gets proper review. It's not an area
I'm especially familiar with.


> So I would support the suggestion of updating MAINTAINERS to include
> fs/exportfs/ in the NFSD section.

No problem with that, if my co-maintainer agrees.


> Having said that - given your apparent preference of tracepoints for
> tracing in nfsd - I suspect you might ask for a somewhat different patch
> :-)

I've certainly had that thought, but I don't think there currently is
a trace subsystem defined for exportfs.


> Thanks,
> NeilBrown
>=20
>=20
>>=20
>>=20
>>>> ---
>>>> fs/exportfs/expfs.c | 8 ++++----
>>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>>=20
>>>> diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
>>>> index c648a493faf23..3204bd33e4e8a 100644
>>>> --- a/fs/exportfs/expfs.c
>>>> +++ b/fs/exportfs/expfs.c
>>>> @@ -18,7 +18,7 @@
>>>> #include <linux/sched.h>
>>>> #include <linux/cred.h>
>>>>=20
>>>> -#define dprintk(fmt, args...) do{}while(0)
>>>> +#define dprintk(fmt, args...) pr_debug(fmt, ##args)
>>>>=20
>>>>=20
>>>> static int get_name(const struct path *path, char *name, struct dentry=
 *child);
>>>> @@ -132,8 +132,8 @@ static struct dentry *reconnect_one(struct vfsmoun=
t *mnt,
>>>> 	inode_unlock(dentry->d_inode);
>>>>=20
>>>> 	if (IS_ERR(parent)) {
>>>> -		dprintk("%s: get_parent of %ld failed, err %d\n",
>>>> -			__func__, dentry->d_inode->i_ino, PTR_ERR(parent));
>>>> +		dprintk("get_parent of %lu failed, err %ld\n",
>>>> +			dentry->d_inode->i_ino, PTR_ERR(parent));
>>>> 		return parent;
>>>> 	}
>>>>=20
>>>> @@ -147,7 +147,7 @@ static struct dentry *reconnect_one(struct vfsmoun=
t *mnt,
>>>> 	dprintk("%s: found name: %s\n", __func__, nbuf);
>>>> 	tmp =3D lookup_one_unlocked(mnt_user_ns(mnt), nbuf, parent, strlen(nb=
uf));
>>>> 	if (IS_ERR(tmp)) {
>>>> -		dprintk("%s: lookup failed: %d\n", __func__, PTR_ERR(tmp));
>>>> +		dprintk("lookup failed: %ld\n", PTR_ERR(tmp));
>>>> 		err =3D PTR_ERR(tmp);
>>>> 		goto out_err;
>>>> 	}
>>>> --=20
>>>> 2.35.3
>>>>=20
>>>>=20
>>=20
>> --
>> Chuck Lever

--
Chuck Lever