On Wed, Jan 25, 2023 at 10:37 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > On Wed, Jan 25, 2023 at 08:49:50AM -0800, Suren Baghdasaryan wrote: > > On Wed, Jan 25, 2023 at 1:10 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > > > > + /* > > > > + * Flags, see mm.h. > > > > + * WARNING! Do not modify directly. > > > > + * Use {init|reset|set|clear|mod}_vm_flags() functions instead. > > > > + */ > > > > + unsigned long vm_flags; > > > > > > We have __private and ACCESS_PRIVATE() to help with enforcing this. > > > > Thanks for pointing this out, Peter! I guess for that I'll need to > > convert all read accesses and provide get_vm_flags() too? That will > > cause some additional churt (a quick search shows 801 hits over 248 > > files) but maybe it's worth it? I think Michal suggested that too in > > another patch. Should I do that while we are at it? > > Here's a trick I saw somewhere in the VFS: > > union { > const vm_flags_t vm_flags; > vm_flags_t __private __vm_flags; > }; > > Now it can be read by anybody but written only by those using > ACCESS_PRIVATE. Huh, this is quite nice! I think it does not save us from the cases when vma->vm_flags is passed by a reference and modified indirectly, like in ksm_madvise()? Though maybe such usecases are so rare (I found only 2 cases) that we can ignore this?