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

[David Hildenbrand <david@redhat.com>]

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.

-- 
Cheers,

David / dhildenb