Re: [PATCH v2 2/6] drm/shmem_helper: use vmf_insert_pfn_mkwrite()

On Tue, 4 Aug 2026 16:34:10 +0200
Paolo Bonzini <[email protected]> wrote:

> On Tue, Aug 4, 2026 at 4:19 PM Boris Brezillon
> <[email protected]> wrote:
> > On Tue, 4 Aug 2026 16:15:49 +0200
> > Boris Brezillon <[email protected]> wrote:  
> > > Actually, if we're making the drm_gem_shmem_record_mkwrite() call
> > > unconditional (for PTE and PMD updates) in that path, can't we drop the
> > > drm_gem_shmem_pfn_mkwrite() call living in drm_gem_shmem_pfn_mkwrite()?
> > >
> > > Also, I'm not even sure we can end up with write=true for PTE updates,
> > > because our pfn_mkwrite implementation returns zero, not VM_FAULT_ERROR
> > > or VM_FAULT_NOPAGE. This means the default RO -> RW PTE upgrade
> > > implemented in finish_mkwrite_fault() [1] will take place. If we really
> > > want out try_insert_pfn() to be called for those RO -> RW updgrades, we
> > > need to call try_insert_pfn() from drm_gem_shmem_pfn_mkwrite().  
> >
> > Nevermind, it's all explained in the comment you've added. Sorry for
> > the noise. I keep wondering if we shouldn't call try_insert_pfn() from
> > pfn_mkwrite() though, like is done in other places.  
> 
> No, the .pfn_mkwrite() callback is invoked when a PTE already exists,
> and mm/ already takes care of making it writable. So there's nothing
> to insert, you just have to take note which you do with
> drm_gem_shmem_record_mkwrite().

Okay, I thought I'd ask to be sure, because of all the implementations
of pfn_mkwrite listed here [1], only drm_gem_shmem_helper.c and
kernel/events/core.c do that. The rest have their "generic" fault
handler (by generic I mean a fault handler helper that covers all the
order/WRITE_FLAG combinations) called from pfn_mkwrite(), and return a
non-zero vm_fault_t.

[1]https://elixir.bootlin.com/linux/v7.2-rc5/A/ident/pfn_mkwrite