[PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters

Max Kellermann posted 12 patches 1 month ago
There is a newer version of this series
[PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by Max Kellermann 1 month ago
For improved const-correctness.

Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
Reviewed-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 include/linux/mm.h       | 8 ++++----
 include/linux/shmem_fs.h | 4 ++--
 mm/shmem.c               | 6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 00c8a54127d3..a40a3c42c904 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -979,11 +979,11 @@ static inline void vma_iter_set(struct vma_iterator *vmi, unsigned long addr)
  * The vma_is_shmem is not inline because it is used only by slow
  * paths in userfault.
  */
-bool vma_is_shmem(struct vm_area_struct *vma);
-bool vma_is_anon_shmem(struct vm_area_struct *vma);
+bool vma_is_shmem(const struct vm_area_struct *vma);
+bool vma_is_anon_shmem(const struct vm_area_struct *vma);
 #else
-static inline bool vma_is_shmem(struct vm_area_struct *vma) { return false; }
-static inline bool vma_is_anon_shmem(struct vm_area_struct *vma) { return false; }
+static inline bool vma_is_shmem(const struct vm_area_struct *vma) { return false; }
+static inline bool vma_is_anon_shmem(const struct vm_area_struct *vma) { return false; }
 #endif
 
 int vma_is_stack_for_current(struct vm_area_struct *vma);
diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h
index 6d0f9c599ff7..0e47465ef0fd 100644
--- a/include/linux/shmem_fs.h
+++ b/include/linux/shmem_fs.h
@@ -99,9 +99,9 @@ extern unsigned long shmem_get_unmapped_area(struct file *, unsigned long addr,
 		unsigned long len, unsigned long pgoff, unsigned long flags);
 extern int shmem_lock(struct file *file, int lock, struct ucounts *ucounts);
 #ifdef CONFIG_SHMEM
-bool shmem_mapping(struct address_space *mapping);
+bool shmem_mapping(const struct address_space *mapping);
 #else
-static inline bool shmem_mapping(struct address_space *mapping)
+static inline bool shmem_mapping(const struct address_space *mapping)
 {
 	return false;
 }
diff --git a/mm/shmem.c b/mm/shmem.c
index 13cc51df3893..2f765bbc20bc 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -275,18 +275,18 @@ static const struct vm_operations_struct shmem_vm_ops;
 static const struct vm_operations_struct shmem_anon_vm_ops;
 static struct file_system_type shmem_fs_type;
 
-bool shmem_mapping(struct address_space *mapping)
+bool shmem_mapping(const struct address_space *const mapping)
 {
 	return mapping->a_ops == &shmem_aops;
 }
 EXPORT_SYMBOL_GPL(shmem_mapping);
 
-bool vma_is_anon_shmem(struct vm_area_struct *vma)
+bool vma_is_anon_shmem(const struct vm_area_struct *const vma)
 {
 	return vma->vm_ops == &shmem_anon_vm_ops;
 }
 
-bool vma_is_shmem(struct vm_area_struct *vma)
+bool vma_is_shmem(const struct vm_area_struct *const vma)
 {
 	return vma_is_anon_shmem(vma) || vma->vm_ops == &shmem_vm_ops;
 }
-- 
2.47.2
Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by Kiryl Shutsemau 1 month ago
On Sun, Aug 31, 2025 at 11:39:07AM +0200, Max Kellermann wrote:
> For improved const-correctness.

It is not a proper commit message.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by Max Kellermann 1 month ago
On Mon, Sep 1, 2025 at 9:33 AM Kiryl Shutsemau <kirill@shutemov.name> wrote:
>
> On Sun, Aug 31, 2025 at 11:39:07AM +0200, Max Kellermann wrote:
> > For improved const-correctness.
>
> It is not a proper commit message.

I believe it is proper for something as trivial as this. I think
adding more text would just be noise, only wasting the time of people
reading it. But that is a matter of perspective: I expect every
competent C developer to know the concept of const-correctness.

Do you believe the commit message of 29cfe7556bfd ("mm: constify more
page/folio tests") is "proper"?
Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by Mike Rapoport 1 month ago
On Mon, Sep 01, 2025 at 10:05:53AM +0200, Max Kellermann wrote:
> On Mon, Sep 1, 2025 at 9:33 AM Kiryl Shutsemau <kirill@shutemov.name> wrote:
> >
> > On Sun, Aug 31, 2025 at 11:39:07AM +0200, Max Kellermann wrote:
> > > For improved const-correctness.
> >
> > It is not a proper commit message.
> 
> I believe it is proper for something as trivial as this. I think
> adding more text would just be noise, only wasting the time of people
> reading it. But that is a matter of perspective: I expect every
> competent C developer to know the concept of const-correctness.

True, but having a brief explanation why it's ok to constify these
parameters helps. Especially in longer patches.
 
> Do you believe the commit message of 29cfe7556bfd ("mm: constify more
> page/folio tests") is "proper"?

-- 
Sincerely yours,
Mike.
Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by David Hildenbrand 1 month ago
On 01.09.25 10:05, Max Kellermann wrote:
> On Mon, Sep 1, 2025 at 9:33 AM Kiryl Shutsemau <kirill@shutemov.name> wrote:
>>
>> On Sun, Aug 31, 2025 at 11:39:07AM +0200, Max Kellermann wrote:
>>> For improved const-correctness.
>>
>> It is not a proper commit message.
> 
> I believe it is proper for something as trivial as this. I think
> adding more text would just be noise, only wasting the time of people
> reading it. But that is a matter of perspective: I expect every
> competent C developer to know the concept of const-correctness.
> 
> Do you believe the commit message of 29cfe7556bfd ("mm: constify more
> page/folio tests") is "proper"?
> 

"Constify shmem related test functions for improved const-correctness."

-- 
Cheers

David / dhildenb

Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by Max Kellermann 1 month ago
On Mon, Sep 1, 2025 at 10:20 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 01.09.25 10:05, Max Kellermann wrote:
> > On Mon, Sep 1, 2025 at 9:33 AM Kiryl Shutsemau <kirill@shutemov.name> wrote:
> >>
> >> On Sun, Aug 31, 2025 at 11:39:07AM +0200, Max Kellermann wrote:
> >>> For improved const-correctness.
> >>
> >> It is not a proper commit message.
> >
> > I believe it is proper for something as trivial as this. I think
> > adding more text would just be noise, only wasting the time of people
> > reading it. But that is a matter of perspective: I expect every
> > competent C developer to know the concept of const-correctness.
> >
> > Do you believe the commit message of 29cfe7556bfd ("mm: constify more
> > page/folio tests") is "proper"?
> >
>
> "Constify shmem related test functions for improved const-correctness."

Mentioning "shmem" adds no information because that is already
mentioned in the subject. "Constify" is just as redundant, it's the
same as "adding const".

The only new piece of information here is "test". If you want, I can
change the subject to "mm/shmem: add `const` to pointer parameters of
test functions" and leave the body. Would that make the commit message
"proper", or do you insist on having redundant information in the
body?
Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by David Hildenbrand 1 month ago
On 01.09.25 10:26, Max Kellermann wrote:
> On Mon, Sep 1, 2025 at 10:20 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 01.09.25 10:05, Max Kellermann wrote:
>>> On Mon, Sep 1, 2025 at 9:33 AM Kiryl Shutsemau <kirill@shutemov.name> wrote:
>>>>
>>>> On Sun, Aug 31, 2025 at 11:39:07AM +0200, Max Kellermann wrote:
>>>>> For improved const-correctness.
>>>>
>>>> It is not a proper commit message.
>>>
>>> I believe it is proper for something as trivial as this. I think
>>> adding more text would just be noise, only wasting the time of people
>>> reading it. But that is a matter of perspective: I expect every
>>> competent C developer to know the concept of const-correctness.
>>>
>>> Do you believe the commit message of 29cfe7556bfd ("mm: constify more
>>> page/folio tests") is "proper"?
>>>
>>
>> "Constify shmem related test functions for improved const-correctness."
> 
> Mentioning "shmem" adds no information because that is already
> mentioned in the subject. "Constify" is just as redundant, it's the
> same as "adding const".
> 
> The only new piece of information here is "test". If you want, I can
> change the subject to "mm/shmem: add `const` to pointer parameters of
> test functions" and leave the body. Would that make the commit message
> "proper", or do you insist on having redundant information in the
> body?

We usually write complete sentences, and there is nothing wrong with 
repeating what the subject says.

All the time it takes you to argue here would be better used improving 
your patch descriptions.

-- 
Cheers

David / dhildenb

Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by Max Kellermann 1 month ago
On Mon, Sep 1, 2025 at 10:35 AM David Hildenbrand <david@redhat.com> wrote:
> We usually write complete sentences, and there is nothing wrong with
> repeating what the subject says.
>
> All the time it takes you to argue here would be better used improving
> your patch descriptions.

Sure, but first I need to know what is really needed. Reviews on LKML
are often contradictory, and it's easy to get pushed around from one
corner to the next.

I just posted v4 with longer commit messages. I think that's a lot of
unnecessary noise that takes a lot of time to read, but oh well, if
that's what you guys really want...

(In the days of LLMs, writing is almost free, but reading all that
redundant or generated garbage text becomes impossible. I think it is
harmful to have so much redundant text because time spent reading it
is time wasted. But that's just my opinion.)
Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by David Hildenbrand 1 month ago
On 01.09.25 11:26, Max Kellermann wrote:
> On Mon, Sep 1, 2025 at 10:35 AM David Hildenbrand <david@redhat.com> wrote:
>> We usually write complete sentences, and there is nothing wrong with
>> repeating what the subject says.
>>
>> All the time it takes you to argue here would be better used improving
>> your patch descriptions.
> 
> Sure, but first I need to know what is really needed. Reviews on LKML
> are often contradictory, and it's easy to get pushed around from one
> corner to the next.

Yeah, and that sucks and I'm sorry if that happened to you in the past.

If you ever get pushback on (a) splitting patches into reasonable 
logical chunks or (b) writing a short yet concise patch description that 
explains what the patch does and why, feel free to CC me.

> 
> I just posted v4 with longer commit messages. I think that's a lot of
> unnecessary noise that takes a lot of time to read, but oh well, if
> that's what you guys really want...
> 
> (In the days of LLMs, writing is almost free, but reading all that
> redundant or generated garbage text becomes impossible. I think it is
> harmful to have so much redundant text because time spent reading it
> is time wasted. But that's just my opinion.)

There is an important distinction between garbage and a reasonable patch 
description.

-- 
Cheers

David / dhildenb

Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by Max Kellermann 1 month ago
On Mon, Sep 1, 2025 at 11:41 AM David Hildenbrand <david@redhat.com> wrote:
> There is an important distinction between garbage and a reasonable patch
> description.

It's still not clear to me where you draw the line (it's opinion, no
objective truth; to me, every redundant piece of text is effectively
garbage because it steals my time) - but you asked me to get busy
instead of arguing with you.
Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by David Hildenbrand 1 month ago
On 01.09.25 11:48, Max Kellermann wrote:
> On Mon, Sep 1, 2025 at 11:41 AM David Hildenbrand <david@redhat.com> wrote:
>> There is an important distinction between garbage and a reasonable patch
>> description.
> 
> It's still not clear to me where you draw the line (it's opinion, no
> objective truth; to me, every redundant piece of text is effectively
> garbage because it steals my time) - but you asked me to get busy
> instead of arguing with you.

I'm sorry, I have no time to argue about the basics of writing a patch 
description. I even proposed a simple example of what we (multiple 
reviewers) would expect as a bare minimum.

If that's not good enough, I don't know what would be.

-- 
Cheers

David / dhildenb

Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by Max Kellermann 1 month ago
On Mon, Sep 1, 2025 at 11:53 AM David Hildenbrand <david@redhat.com> wrote:
> I'm sorry, I have no time to argue about the basics of writing a patch
> description. I even proposed a simple example of what we (multiple
> reviewers) would expect as a bare minimum.

But Lorenzo Stoakes and Mike Rappoport wanted much more than that.

> If you feel like you need other rules than everybody else here

What other rules? I get confused by different requirements by different people.

Quite contrary - I want the same rules as everybody else. For example,
the same rules as Matthew Wilcox who already submitted similar patches
(with similar commit messages) that were merged without pushing him
around.
Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by David Hildenbrand 1 month ago
On 01.09.25 12:00, Max Kellermann wrote:
> On Mon, Sep 1, 2025 at 11:53 AM David Hildenbrand <david@redhat.com> wrote:
>> I'm sorry, I have no time to argue about the basics of writing a patch
>> description. I even proposed a simple example of what we (multiple
>> reviewers) would expect as a bare minimum.
> 
> But Lorenzo Stoakes and Mike Rappoport wanted much more than that.

Sure, if it's not a simple "test" function as the one I commented on, it 
might make sense to explain more why it is okay.

> 
>> If you feel like you need other rules than everybody else here
> 
> What other rules? I get confused by different requirements by different people.
> 
> Quite contrary - I want the same rules as everybody else. For example,
> the same rules as Matthew Wilcox who already submitted similar patches
> (with similar commit messages) that were merged without pushing him
> around.

In my example I literally took the beginning of Willy's patch 
description and modified it, extending it by the const-correctnes.

And before I get seriously annoyed and behave differently than I usually 
would here on this list, like ever,

this is my suggestion for that one patch:

"Constify shmem related test functions for improved const-correctness."

and this is the beginning of Willy's patch

"Constify the flag tests ..."

And now I am wondering why on earth I am wasting my time here on 
something that trivial.

-- 
Cheers

David / dhildenb

Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by Max Kellermann 1 month ago
On Mon, Sep 1, 2025 at 12:07 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 01.09.25 12:00, Max Kellermann wrote:
> > On Mon, Sep 1, 2025 at 11:53 AM David Hildenbrand <david@redhat.com> wrote:
> >> I'm sorry, I have no time to argue about the basics of writing a patch
> >> description. I even proposed a simple example of what we (multiple
> >> reviewers) would expect as a bare minimum.
> >
> > But Lorenzo Stoakes and Mike Rappoport wanted much more than that.
>
> Sure, if it's not a simple "test" function as the one I commented on, it
> might make sense to explain more why it is okay.

Lorenzo and Mike commented on the very same patch as you (i.e. 01/12).

I remember that you provided an example, and implementing that would
have been easy - but it would not have been enough.
Matthew's patches would not have been enough, had they been judged by
the same rules.
Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by David Hildenbrand 1 month ago
On 01.09.25 12:36, Max Kellermann wrote:
> On Mon, Sep 1, 2025 at 12:07 PM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 01.09.25 12:00, Max Kellermann wrote:
>>> On Mon, Sep 1, 2025 at 11:53 AM David Hildenbrand <david@redhat.com> wrote:
>>>> I'm sorry, I have no time to argue about the basics of writing a patch
>>>> description. I even proposed a simple example of what we (multiple
>>>> reviewers) would expect as a bare minimum.
>>>
>>> But Lorenzo Stoakes and Mike Rappoport wanted much more than that.
>>
>> Sure, if it's not a simple "test" function as the one I commented on, it
>> might make sense to explain more why it is okay.
> 
> Lorenzo and Mike commented on the very same patch as you (i.e. 01/12).
> 
> I remember that you provided an example, and implementing that would
> have been easy - but it would not have been enough.

It would have been :)

See, Willy's patch made it clear that these are "test" functions. I 
incorporated that in my suggestion by using the term "test function".

For a "test" function (or a getter), it's trivial to see why we would 
want to have it const.

For other functions it's less clear, and might contradict to some plans 
we have (e.g., currently does not modify it but might in the future).

-- 
Cheers

David / dhildenb

Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by Vlastimil Babka 1 month ago
On 9/1/25 11:26, Max Kellermann wrote:
> On Mon, Sep 1, 2025 at 10:35 AM David Hildenbrand <david@redhat.com> wrote:
>> We usually write complete sentences, and there is nothing wrong with
>> repeating what the subject says.
>>
>> All the time it takes you to argue here would be better used improving
>> your patch descriptions.
> 
> Sure, but first I need to know what is really needed. Reviews on LKML
> are often contradictory, and it's easy to get pushed around from one
> corner to the next.
> 
> I just posted v4 with longer commit messages. I think that's a lot of
> unnecessary noise that takes a lot of time to read, but oh well, if
> that's what you guys really want...

No I don't think we want a passive-aggressive malicious compliance.
> (In the days of LLMs, writing is almost free, but reading all that
> redundant or generated garbage text becomes impossible. I think it is
> harmful to have so much redundant text because time spent reading it
> is time wasted. But that's just my opinion.)

Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by Max Kellermann 1 month ago
On Mon, Sep 1, 2025 at 11:32 AM Vlastimil Babka <vbabka@suse.cz> wrote:
> > Sure, but first I need to know what is really needed. Reviews on LKML
> > are often contradictory, and it's easy to get pushed around from one
> > corner to the next.
> >
> > I just posted v4 with longer commit messages. I think that's a lot of
> > unnecessary noise that takes a lot of time to read, but oh well, if
> > that's what you guys really want...
>
> No I don't think we want a passive-aggressive malicious compliance.

What the .....
Your response is similar to what I mean with "getting pushed around".

You guys insist on verbose commit messages, or else it won't get
merged, you say. I don't agree, but when I yield to your requirements,
take the time to implement them, I get accused of being
"passive-aggressive" and "malicious".

Please be more respectful when handling code submissions.
Re: [PATCH v2 01/12] mm/shmem: add `const` to lots of pointer parameters
Posted by David Hildenbrand 1 month ago
On 01.09.25 11:45, Max Kellermann wrote:
> On Mon, Sep 1, 2025 at 11:32 AM Vlastimil Babka <vbabka@suse.cz> wrote:
>>> Sure, but first I need to know what is really needed. Reviews on LKML
>>> are often contradictory, and it's easy to get pushed around from one
>>> corner to the next.
>>>
>>> I just posted v4 with longer commit messages. I think that's a lot of
>>> unnecessary noise that takes a lot of time to read, but oh well, if
>>> that's what you guys really want...
>>
>> No I don't think we want a passive-aggressive malicious compliance.
> 
> What the .....
> Your response is similar to what I mean with "getting pushed around".
> 
> You guys insist on verbose commit messages, or else it won't get
> merged, you say. I don't agree, but when I yield to your requirements,
> take the time to implement them, I get accused of being
> "passive-aggressive" and "malicious".
> 
> Please be more respectful when handling code submissions.

It's rare that we have such conflicts/drama around here.

If you feel like you need other rules than everybody else here, I 
suggest you focus your effort on other parts of the kernel.

-- 
Cheers

David / dhildenb