On Wed, Jul 08, 2026 at 10:28:00PM -0400, Zi Yan wrote: > On Thu Jul 2, 2026 at 12:07 PM EDT, Lorenzo Stoakes wrote: > > On Thu, Jul 02, 2026 at 09:49:47PM +0800, Lance Yang wrote: > >> > >> On Mon, Jun 29, 2026 at 08:25:36PM +0100, Lorenzo Stoakes wrote: > >> >Replace use of the legacy vm_flags_t flags with vma_flags_t values > >> >throughout the mremap logic. > >> > > >> >Additionally update comments to reflect the changes to be consistent. > >> > > >> >No functional change intended. > >> > > >> >Signed-off-by: Lorenzo Stoakes <[email protected]> > >> >--- > >> > >> The vm_flags_set() cases below spell out vma_start_write(), but the > >> vm_flags_clear() cases don't? > > > > Yep as I said elsewhere, implicitly taking the lock is terrible and me doing > > this is completely on purpose to get rid of that :) > > > > But I haven't been clear enough clearly, so I should put the argument as to > > why > > that's ok in the commit message. > > > > Will do so on respin. > > How about also add a comment to vma_clear*() telling us a lock is not > needed and why like you explained a lock is needed for vma_set*()? > This asymmetry could confuse people.
Well you do absolutely have to hold the lock once the VMA is initialised when using any VMA flags API that manipulates flags (apart from the explicitly atomic case). The argument here is that the VMA was _already locked_ at the point of clearing :) > > This patch looks good to me. > > Reviewed-by: Zi Yan <[email protected]> Thanks! > > -- > Best Regards, > Yan, Zi > Cheers, Lorenzo
