Re: [PATCH v5 03/12] mm: introduce AS_NO_DIRECT_MAP

[David Hildenbrand]

On 02.09.25 09:59, Fuad Tabba wrote:
> Hi Patrick,
>
> On Mon, 1 Sept 2025 at 15:56, Roy, Patrick <roypat@xxxxxxxxxxxx> wrote:
> > On Mon, 2025-09-01 at 14:54 +0100, "Roy, Patrick" wrote:
> > > Hi Fuad!
> > >
> > > On Thu, 2025-08-28 at 11:21 +0100, Fuad Tabba wrote:
> > > > Hi Patrick,
> > > >
> > > > On Thu, 28 Aug 2025 at 10:39, Roy, Patrick <roypat@xxxxxxxxxxxx> wrote:
> > > > > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> > > > > index 12a12dae727d..b52b28ae4636 100644
> > > > > --- a/include/linux/pagemap.h
> > > > > +++ b/include/linux/pagemap.h
> > > > > @@ -211,6 +211,7 @@ enum mapping_flags {
> > > > >                                     folio contents */
> > > > >          AS_INACCESSIBLE = 8,    /* Do not attempt direct R/W access to the mapping */
> > > > >          AS_WRITEBACK_MAY_DEADLOCK_ON_RECLAIM = 9,
> > > > > +       AS_NO_DIRECT_MAP = 10,  /* Folios in the mapping are not in the direct map */
> > > > >          /* Bits 16-25 are used for FOLIO_ORDER */
> > > > >          AS_FOLIO_ORDER_BITS = 5,
> > > > >          AS_FOLIO_ORDER_MIN = 16,
> > > > > @@ -346,6 +347,21 @@ static inline bool mapping_writeback_may_deadlock_on_reclaim(struct address_spac
> > > > >          return test_bit(AS_WRITEBACK_MAY_DEADLOCK_ON_RECLAIM, &mapping->flags);
> > > > >   }
> > > > >
> > > > > +static inline void mapping_set_no_direct_map(struct address_space *mapping)
> > > > > +{
> > > > > +       set_bit(AS_NO_DIRECT_MAP, &mapping->flags);
> > > > > +}
> > > > > +
> > > > > +static inline bool mapping_no_direct_map(struct address_space *mapping)
> > > > > +{
> > > > > +       return test_bit(AS_NO_DIRECT_MAP, &mapping->flags);
> > > > > +}
> > > > > +
> > > > > +static inline bool vma_is_no_direct_map(const struct vm_area_struct *vma)
> > > > > +{
> > > > > +       return vma->vm_file && mapping_no_direct_map(vma->vm_file->f_mapping);
> > > > > +}
> > > > > +
> > > > Any reason vma is const whereas mapping in the function that it calls
> > > > (defined above it) isn't?
> > >
> > > Ah, I cannot say that that was a conscious decision, but rather an artifact of
> > > the code that I looked at for reference when writing these two simply did it
> > > this way.  Are you saying both should be const, or neither (in my mind, both
> > > could be const, but the mapping_*() family of functions further up in this file
> > > dont take const arguments, so I'm a bit unsure now)?
> >
> > Hah, just saw
> > https://lore.kernel.org/linux-mm/20250901123028.3383461-3-max.kellermann@xxxxxxxxx/.
> > Guess that means "both should be const" then :D
>
> I don't have any strong preference regarding which way, as long as
> it's consistent. The thing that should be avoided is having one
> function with a parameter marked as const, pass that parameter (or
> something derived from it), to a non-const function. 

I think the compiler will tell you that that is not ok (and you'd have 
to force-cast the const it away).

Agreed that we should be using const * for these simple getter/test 
functions.

--
Cheers

David / dhildenb