Re: [PATCH v2] mm: Introduce new function vm_insert_kmem_page
From: Souptick Joarder
Date: Fri Oct 05 2018 - 06:02:00 EST
On Fri, Oct 5, 2018 at 2:22 PM Miguel Ojeda
<miguel.ojeda.sandonis@xxxxxxxxx> wrote:
>
> Hi Souptick,
>
> On Fri, Oct 5, 2018 at 7:51 AM Souptick Joarder <jrdr.linux@xxxxxxxxx> wrote:
> >
> > On Fri, Oct 5, 2018 at 1:16 AM Miguel Ojeda
> > <miguel.ojeda.sandonis@xxxxxxxxx> wrote:
> > >
> > >
> > > Also, not sure if you saw my comments/review: if the interface is not
> > > going to change, why the name change? Why can't we simply keep using
> > > vm_insert_page?
> >
> > yes, changing the name without changing the interface is a
> > bad approach and this can't be taken. As Matthew mentioned,
> > "vm_insert_range() which takes an array of struct page pointers.
> > That fits the majority of remaining users" would be a better approach
> > to fit this use case.
> >
> > But yes, we can't keep vm_insert_page and vmf_insert_page together
> > as it doesn't guarantee that future drivers will not use vm_insert_page
> > in #PF context ( which will generate new errno to VM_FAULT_CODE).
> >
>
> Maybe I am hard of thinking, but aren't you planning to remove
> vm_insert_page with these changes? If yes, why you can't use the keep
> vm_insert_page name? In other words, keep returning what the drivers
> expect?
The final goal is to remove vm_insert_page by converting it to
vmf_insert_page. But to do that we have to first introduce the
new API which is similar to vm_insert_page (for non #PF). I tried this by
introducing vm_insert_kmem_page ( * identical as vm_insert_page
except API name *) in this patch. But this looks like a bad approach.
The new proposal is to introduce vm_insert_range() ( * which might be
bit different from vm_insert_page but will serve all the non #PF use cases)