linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
To: Max Kellermann <max.kellermann@ionos.com>
Cc: akpm@linux-foundation.org, david@redhat.com,
	axelrasmussen@google.com, yuanchu@google.com,
	willy@infradead.org, hughd@google.com, mhocko@suse.com,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Liam.Howlett@oracle.com, vbabka@suse.cz, rppt@kernel.org,
	surenb@google.com, vishal.moola@gmail.com
Subject: Re: [PATCH v3 01/12] mm/shmem: add `const` to lots of pointer parameters
Date: Mon, 1 Sep 2025 09:52:26 +0100	[thread overview]
Message-ID: <843b5ccb-ef37-4398-b6e5-5f1d997acd77@lucifer.local> (raw)
In-Reply-To: <CAKPOu+-A6EoBnJhYkgX3Ktuivo2hpDZtbCKPfcmR_SNsvPQ02g@mail.gmail.com>

On Mon, Sep 01, 2025 at 10:01:16AM +0200, Max Kellermann wrote:
> On Mon, Sep 1, 2025 at 9:44 AM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> >
> > On Mon, Sep 01, 2025 at 08:12:12AM +0200, Max Kellermann wrote:
> > > For improved const-correctness.
> >
> > This is not an acceptable commit message, you need to explain what you're doing
> > here.
> >
> > I'm thinking that review will be the same for each...
> >
> > For instance, reference the fact you're starting with functions at the bottom of
> > the call graph,
>
> My 00/12 already describes that adding "const" to mm addresses the
> lowest level so higher levels (outside the scope of this patch set)
> are able to constify their APIs.

I actually found your cover letter lacking there also as reviewed.

It is simply not acceptable in the kernel to have a commit message like
this. If you want to argue, that's up to you, but your series won't be
merged.

So overall - we're fine with this kind of duplication even for a 'trivial'
series.

Describe why you're doing, it why these functions.

Something like 'In efforts to const-ify poitner parameters where
appropriate, we start by adjusting those which invoke no other function,
with the intent of working our way up gradually. Here we address functions
relating to shmem.'

etc.

As David said on the other thread, you'd use your energy more usefully
simply doing what's asked of you.

Review is a limited resource, please have some empathy for the human beings
behind the screen :)

>
> Other than that, there is exactly one dependency between the patches,
> and that is documented in the commit message of 06/12. The rest has no
> "bottom" or "top" that I could describe. All other patches are
> standalone.
>
> > and mention which functions you're changing.
>
> So you want to have a list of function names in the commit message?
> Maybe I'll write a Perl one-liner to extract that from the diff, but
> .... will that really be helpful? To me, it looks like noise in a
> patch set as trivial as this one.

By all means use a script to figure it out, but use full english sentences
to describe intent and what you're doing.

Thanks.


  reply	other threads:[~2025-09-01  8:52 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-01  6:12 [PATCH v3 00/12] mm: " Max Kellermann
2025-09-01  6:12 ` [PATCH v3 01/12] mm/shmem: " Max Kellermann
2025-09-01  7:44   ` Lorenzo Stoakes
2025-09-01  8:01     ` Max Kellermann
2025-09-01  8:52       ` Lorenzo Stoakes [this message]
2025-09-01  6:12 ` [PATCH v3 02/12] mm/pagemap: " Max Kellermann
2025-09-01  6:12 ` [PATCH v3 03/12] mm/mmzone: " Max Kellermann
2025-09-01  6:12 ` [PATCH v3 04/12] fs: add `const` to several " Max Kellermann
2025-09-01  6:12 ` [PATCH v3 05/12] mm/oom_kill: add `const` to pointer parameter Max Kellermann
2025-09-01  6:12 ` [PATCH v3 06/12] mm/util, s390: add `const` to several pointer parameters Max Kellermann
2025-09-01  6:12 ` [PATCH v3 07/12] parisc: add `const` to mmap_upper_limit() parameter Max Kellermann
2025-09-01  6:12 ` [PATCH v3 08/12] mm/util, s390, sparc, x86: add const to arch_pick_mmap_layout() parameter Max Kellermann
2025-09-01  6:12 ` [PATCH v3 09/12] mm/mm_types: add `const` to several pointer parameters Max Kellermann
2025-09-01  6:12 ` [PATCH v3 10/12] mm/mm_inline: add `const` to lots of " Max Kellermann
2025-09-01  6:12 ` [PATCH v3 11/12] mm: " Max Kellermann
2025-09-01  6:12 ` [PATCH v3 12/12] mm/highmem: " Max Kellermann
2025-09-01  7:39 ` [PATCH v3 00/12] mm: " Lorenzo Stoakes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=843b5ccb-ef37-4398-b6e5-5f1d997acd77@lucifer.local \
    --to=lorenzo.stoakes@oracle.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=david@redhat.com \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=max.kellermann@ionos.com \
    --cc=mhocko@suse.com \
    --cc=rppt@kernel.org \
    --cc=surenb@google.com \
    --cc=vbabka@suse.cz \
    --cc=vishal.moola@gmail.com \
    --cc=willy@infradead.org \
    --cc=yuanchu@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox