Re: [PATCH v1] SUNRPC: Remove BUG_ON call sites

[Li Lingfeng <lilingfeng3@huawei.com>]

在 2024/8/24 3:59, Chuck Lever 写道:
> On Fri, Aug 23, 2024 at 03:15:04PM -0400, Chuck Lever wrote:
>> On Fri, Aug 23, 2024 at 11:35:28AM +0800, Li Lingfeng wrote:
>>
>> 	[ snipped ]
>>
>>> [   91.319328] Kernel panic - not syncing: Fatal exception
>>> [   91.320712] Kernel Offset: disabled
>>> [   91.321189] ---[ end Kernel panic - not syncing: Fatal exception ]---
>>>
>>> Both of them were introduced by commit 9f28a971ee9f ("nfsd: separate
>>> nfsd_last_thread() from nfsd_put()") since this patch changes the behavior
>>> of the error path.
>>>
>>> I confirmed this by fixing both issues with the following changes:
>>> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>>> index ee5713fca187..05d4b463c16b 100644
>>> --- a/fs/nfsd/nfssvc.c
>>> +++ b/fs/nfsd/nfssvc.c
>>> @@ -811,6 +811,8 @@ nfsd_svc(int nrservs, struct net *net, const struct cred
>>> *cred)
>>>          if (error < 0 && !nfsd_up_before)
>>>                  nfsd_shutdown_net(net);
>>>   out_put:
>>> +       if (error < 0)
>>> +               nfsd_last_thread(net);
>>>          /* Threads now hold service active */
>>>          if (xchg(&nn->keep_active, 0))
>>>                  svc_put(serv);
>>>
>>> They have been fixed by commit bf32075256e9 ("NFSD: simplify error paths in
>>> nfsd_svc()") in mainline.
>>>
>>> Maybe it would be a good idea to push it to the LTS branches.
>> To be clear, by "push it to LTS" I assume you mean apply bf32075?
>>
>> I have now applied commit bf32075256e9 ("NFSD: simplify error paths
>> in nfsd_svc()") to nfsd-6.6.y, nfsd-5.15.y, and nfsd-5.10.y in my
>> kernel.org git repo, for testing.
>>
>>     https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git
>>
>> I will run these three against the usual NFSD CI today, but feel
>> free to try them out yourself and report your results.
>>
>> Now unfortunately 6.1.y is still "special." It appears that commit
>> 9f28a971ee9f ("nfsd: separate nfsd_last_thread() from nfsd_put()")
>> was reverted in that kernel, and the fix you mention here does not
>> cleanly apply to v6.1.106. Based on some previous comments on this
>> list, I think I need to fix up v6.1 LTS to be like the other three
>> kernels, and then apply bf32075.
> OK, I've got a candidate for v6.1.y now, too. See the nfsd-6.1.y
> branch in the above git repo.

I'm getting a compile error when trying to compile nfsd-6.1.y.

The return value of fh_fill_pre_attrs is void, but nfsd_setattr attempts
to assign its return value to a __be32 variable.

I found out that the return type of fh_fill_pre_attrs was changed in
a332018a91c4 ("nfsd: handle failure to collect pre/post-op attrs more
sanely") which hasn't been added to nfsd-6.1.y yet.

Is nfsd-6.1.y not ready yet?

fs/nfsd/vfs.c: In function ‘nfsd_setattr’:
fs/nfsd/vfs.c:536:6: error: void value not ignored as it ought to be
   536 |  err = fh_fill_pre_attrs(fhp);
       |      ^
make[3]: *** [scripts/Makefile.build:250: fs/nfsd/vfs.o] Error 1
make[3]: *** Waiting for unfinished jobs....
   LD [M]  fs/ubifs/ubifs.o
make[2]: *** [scripts/Makefile.build:503: fs/nfsd] Error 2
make[1]: *** [scripts/Makefile.build:503: fs] Error 2
make: *** [Makefile:2009: .] Error 2
>