Re: [PATCH v2 bpf-next 2/3] mm, xen: Separate xen use cases from ioremap.

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Mike Rapoport <rppt@kernel.org>
Cc: bpf <bpf@vger.kernel.org>, Daniel Borkmann <daniel@iogearbox.net>,
	 Andrii Nakryiko <andrii@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	 Barret Rhoden <brho@google.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	 Lorenzo Stoakes <lstoakes@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	 Uladzislau Rezki <urezki@gmail.com>,
	Christoph Hellwig <hch@infradead.org>,
	 Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	sstabellini@kernel.org,  Juergen Gross <jgross@suse.com>,
	linux-mm <linux-mm@kvack.org>,
	xen-devel@lists.xenproject.org,  Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH v2 bpf-next 2/3] mm, xen: Separate xen use cases from ioremap.
Date: Mon, 4 Mar 2024 16:38:45 -0800	[thread overview]
Message-ID: <CAADnVQL4iEhc4nAoeE57Q3=X0_uh37ZDW_SOw1CicD1L7eTU6w@mail.gmail.com> (raw)
In-Reply-To: <ZeV-IE-65yiIwFSY@kernel.org>

On Sun, Mar 3, 2024 at 11:55 PM Mike Rapoport <rppt@kernel.org> wrote:
>
> On Fri, Feb 23, 2024 at 03:57:27PM -0800, Alexei Starovoitov wrote:
> > From: Alexei Starovoitov <ast@kernel.org>
> >
> > xen grant table and xenbus ring are not ioremap the way arch specific code is using it,
> > so let's add VM_XEN flag to separate them from VM_IOREMAP users.
> > xen will not and should not be calling ioremap_page_range() on that range.
> > /proc/vmallocinfo will print such region as "xen" instead of "ioremap" as well.
> >
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> >  arch/x86/xen/grant-table.c         | 2 +-
> >  drivers/xen/xenbus/xenbus_client.c | 2 +-
> >  include/linux/vmalloc.h            | 1 +
> >  mm/vmalloc.c                       | 7 +++++--
> >  4 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/xen/grant-table.c b/arch/x86/xen/grant-table.c
> > index 1e681bf62561..b816db0349c4 100644
> > --- a/arch/x86/xen/grant-table.c
> > +++ b/arch/x86/xen/grant-table.c
> > @@ -104,7 +104,7 @@ static int arch_gnttab_valloc(struct gnttab_vm_area *area, unsigned nr_frames)
> >       area->ptes = kmalloc_array(nr_frames, sizeof(*area->ptes), GFP_KERNEL);
> >       if (area->ptes == NULL)
> >               return -ENOMEM;
> > -     area->area = get_vm_area(PAGE_SIZE * nr_frames, VM_IOREMAP);
> > +     area->area = get_vm_area(PAGE_SIZE * nr_frames, VM_XEN);
> >       if (!area->area)
> >               goto out_free_ptes;
> >       if (apply_to_page_range(&init_mm, (unsigned long)area->area->addr,
> > diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
> > index 32835b4b9bc5..b9c81a2d578b 100644
> > --- a/drivers/xen/xenbus/xenbus_client.c
> > +++ b/drivers/xen/xenbus/xenbus_client.c
> > @@ -758,7 +758,7 @@ static int xenbus_map_ring_pv(struct xenbus_device *dev,
> >       bool leaked = false;
> >       int err = -ENOMEM;
> >
> > -     area = get_vm_area(XEN_PAGE_SIZE * nr_grefs, VM_IOREMAP);
> > +     area = get_vm_area(XEN_PAGE_SIZE * nr_grefs, VM_XEN);
> >       if (!area)
> >               return -ENOMEM;
> >       if (apply_to_page_range(&init_mm, (unsigned long)area->addr,
> > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> > index c720be70c8dd..223e51c243bc 100644
> > --- a/include/linux/vmalloc.h
> > +++ b/include/linux/vmalloc.h
> > @@ -28,6 +28,7 @@ struct iov_iter;            /* in uio.h */
> >  #define VM_FLUSH_RESET_PERMS 0x00000100      /* reset direct map and flush TLB on unmap, can't be freed in atomic context */
> >  #define VM_MAP_PUT_PAGES     0x00000200      /* put pages and free array in vfree */
> >  #define VM_ALLOW_HUGE_VMAP   0x00000400      /* Allow for huge pages on archs with HAVE_ARCH_HUGE_VMALLOC */
> > +#define VM_XEN                       0x00000800      /* xen use cases */
> >
> >  #if (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)) && \
> >       !defined(CONFIG_KASAN_VMALLOC)
>
> There's also VM_DEFER_KMEMLEAK a line below:

Ohh. Good catch. Will fix.

> I think it makes sense to use an enumeration for vm_flags, just like as
> Suren did for GFP
> (https://lore.kernel.org/linux-mm/20240224015800.2569851-1-surenb@google.com/)

Hmm. I'm pretty sure Christoph hates BIT macro obfuscation.
I'm not a fan of it either, though we use it in bpf in a few places.
If mm folks prefer that style they can do such conversion later.