Re: [bug report] mm: replace vma->vm_flags direct modifications with modifier calls

[Suren Baghdasaryan <surenb@google.com>]

On Wed, Jul 12, 2023 at 8:55 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 12.07.23 17:52, Matthew Wilcox wrote:
> > On Wed, Jul 12, 2023 at 08:01:18AM -0700, Suren Baghdasaryan wrote:
> >> Are you suggesting to break remap_pfn_range() into two stages
> >> (remap_pfn_range_prepare() then remap_pfn_range())?
> >> If so, there are many places remap_pfn_range() is called and IIUC all
> >> of them would need to use that 2-stage approach (lots of code churn).
> >> In addition, this is an exported function, so many more drivers might
> >> expect the current behavior.
> >
> > You do not understand correctly.
> >
> > When somebody calls mmap, there are two reasonable implementations.
> > Here's one:
> >
> >          .mmap = snd_dma_iram_mmap,
> >
> > static int snd_dma_iram_mmap(struct snd_dma_buffer *dmab,
> >                               struct vm_area_struct *area)
> > {
> >          area->vm_page_prot = pgprot_writecombine(area->vm_page_prot);
> >          return remap_pfn_range(area, area->vm_start,
> >                                 dmab->addr >> PAGE_SHIFT,
> >                                 area->vm_end - area->vm_start,
> >                                 area->vm_page_prot);
> > }
> >
> > This is _fine_.  It is not called from the fault path, it is called in
> > process context.  Few locks are held (which ones aren't even
> > documented!)
> >
> > The other way is to set vma->vm_ops.  The fault handler in vm_ops
> > should not be calling remap_pfn_range().  It should be calling
> > set_ptes().  I almost have this driver fixed up, but I have another
> > meeting to go to now.
>
> Just a note that we still have to make sure that the VMA flags will be
> set properly -- I guess at mmap time is the right time as I suggested above.

Ah, ok, now I understand what you meant. Ok, let's wait for Matthew to
send the fix for the driver. Sounds like he has it almost ready.

>
> --
> Cheers,
>
> David / dhildenb
>