Re: [PATCH v2 3/5] proc/task_mmu: remove special-casing of smap_gather_stats() start parameter

[Suren Baghdasaryan]

On Thu, Sep 10, 2026 at 12:41 AM David Hildenbrand (Arm)
<david@xxxxxxxxxx> wrote:
>
> On 9/9/26 23:51, Suren Baghdasaryan wrote:
> > On Wed, Sep 9, 2026 at 12:16 PM David Hildenbrand (Arm)
> > <david@xxxxxxxxxx> wrote:
> >>
> >> On 9/9/26 20:28, Suren Baghdasaryan wrote:
> >>> On Wed, Sep 9, 2026 at 10:16 AM David Hildenbrand (Arm)
> >>> <david@xxxxxxxxxx> wrote:
> >>>
> >>> Hmm. So, are you saying that !is_cow always implies shared_or_ro? Or
> >>> maybe you are stating that vma_is_cow_mapping() was the actual intent
> >>> here?
> >>
> >> So the comment says:
> >>
> >> "For private writable mappings, we might have COW pages that  .."
> >>
> >> Which translates to:
> >>
> >>         private writable == vma_is_cow_mapping()
> >
> > Ok, just want to make sure I'm not missing something subtle.
> >
> >>
> >>>
> >>> shared_or_ro = VMA_SHARED_BIT || !VMA_WRITE_BIT
> >>>
> >>> is_cow = !VMA_SHARED_BIT && VMA_MAYWRITE_BIT
> >>> !is_cow = VMA_SHARED_BIT || !VMA_MAYWRITE_BIT
> >>>
> >>> so, !is_cow would impy shared_or_ro only if !VMA_MAYWRITE_BIT always
> >>> implies !VMA_WRITE_BIT. But I think it's possible to have a VMA that
> >>> has VMA_WRITE_BIT but not VMA_MAYWRITE_BIT, right?
> >>
> >> VMA_WRITE should imply VMA_MAYWRITE
> >
> > Ah, good to know.
> >
> >>
> >> (in sanitize_fault_flags() we even disallow write faults entirely if VM_MAYWRITE
> >> is missing)
> >
> > I see.
> >
> >>
> >> For example, a
> >>> driver can create such a VMA to allow writing to the VMA but to lock
> >>> its content once mprotect(PROT_READ) gets called.
> >>
> >> I don't think that would be valid for a driver to do. But it wouldn't matter
> >> here because
> >>
> >>         shmem_mapping(vma->vm_file->f_mapping)
> >
> > I was obviously overthinking this :)
> >
> >>
> >>
> >> I think we could simplify the comment as well to:
> >>
> >> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> >> index e671b4fd8dedd..4b7e7089cafa7 100644
> >> --- a/fs/proc/task_mmu.c
> >> +++ b/fs/proc/task_mmu.c
> >> @@ -1303,12 +1303,10 @@ static void smap_gather_stats(struct proc_maps_private
> >> *priv,if (is_partial || (shmem_swapped && is_cow))
> >>
> >>         if (vma->vm_file && shmem_mapping(vma->vm_file->f_mapping)) {
> >>                 /*
> >> -                * For shared or readonly shmem mappings we know that all
> >> -                * swapped out pages belong to the shmem object, and we can
> >> -                * obtain the swap value much more efficiently. For private
> >> -                * writable mappings, we might have COW pages that are
> >> -                * not affected by the parent swapped out pages of the shmem
> >> -                * object, so we have to distinguish them during the page walk.
> >> +                * In CoW mappings, we might have anon folios that are
> >> +                * independent of the shmem object. So fallback to the less
> >> +                * efficient mechanism in such mappings.
> >> +                *
> >
> > Sounds good. Will update.
>
> Likely worth putting that into a prior cleanup patch, so there is less noise in
> this patch.

Well, this is a cleanup specific for smap_gather_stats() funciton, so
I would prefer to keep all the pieces in one place.

>
> --
> Cheers,
>
> David