Re: [RFC PATCH] NFSD: Add a subsystem policy document

[Chuck Lever <cel@kernel.org>]

On 9/24/25 1:48 AM, NeilBrown wrote:
> On Tue, 23 Sep 2025, Chuck Lever wrote:
>> Hi Neil -
>>
>> On 9/21/25 9:25 PM, NeilBrown wrote:
>>>> +Patch preparation
>>>> +~~~~~~~~~~~~~~~~~
>>>> +Like all kernel submissions, please use tagging to identify all
>>>> +patch authors. Reviewers and testers can be added by replying to
>>>> +the email patch submission. Email is extensively used in order to
>>>> +publicly archive review and testing attributions, and will be
>>>> +automatically inserted into your patches when they are applied.
>>>> +
>>>> +The patch description must contain information that does not appear
>>>> +in the body of the diff. The code should be good enough to tell a
>>>> +story -- self-documenting -- but the patch description needs to
>>>> +provide rationale ("why does NFSD benefit from this change?") or
>>>> +a clear problem statement ("what is this patch trying to fix?").
>>
>>> These paras look to be completely generic - not at all nfsd-specific.
>>> Do they belong here?
>>
>> Can you clarify which paragraphs you mean, exactly? Maybe the whole
>> section?
> 
> I specifically meant the previous two paragraphs.
> 
> The "Describe your changes" section of submitting-patches.rst seems to
> cover the same ground.  It even says:
> 
>> Once the problem is established, describe what you are actually doing
>> about it in technical detail.  It's important to describe the change
>> in plain English for the reviewer to verify that the code is behaving
>> as you intend it to.
> 
> which is close enough to the addition that I suggested.

Hi Neil,

Based on your previous remarks, I've already restructured this section.
I'll post a refreshed version of this document and we can go from there.


>> For context:
>>
>> IMHO these comments aren't necessarily generic because I haven't seen
>> them in other documents, and we seem to get a lot of patches where the
>> description is just "Make this change".
>>
>> The comments about tagging: I think other subsystems might not mind
>> seeing Cc: stable in the initial submission. NFS maintainers (even on
>> the client side) like to add those themselves.
> 
> If you don't want "cc: stable" then certainly include that.
> submitting-patches.rst encourages it to be included - for "a severe
> bug"....  but it has been a long time since stable was for "severe" bugs
> only.

Right... I can't think of a reason to copy stable@ on a patch that is
first headed to Linus' kernel.

We could remind folks about stable@kernel.org, which is equivalent in
meaning to stable@vger.kernel.org but is a dead email address...?


>> I'd like to encourage contributors to get the Fixes: tag right before
>> submitting, too. It saves me a little incremental time per patch.
> 
> submitting-patches.rst encourages a Fixes: tag.

And still people forget. A little more encouragement can't hurt. Unless
you believe this addition is actively harmful, I'd like to keep the
gentle encouragement here.


-- 
Chuck Lever