linux-kernel - Re: Re: [PATCH] vduse: Use fixed 4KB bounce pages for arm64 64KB page size

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACGkMEv4TZOAyxaTkCvZ4rgyVsPet+r3pNSauHaGOYHim3Loag@mail.gmail.com>
Date: Wed, 24 Sep 2025 12:15:57 +0800
From: Jason Wang <jasowang@...hat.com>
To: Sheng Zhao <sheng.zhao@...edance.com>
Cc: mst@...hat.com, xuanzhuo@...ux.alibaba.com, eperezma@...hat.com, 
	virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	xieyongji@...edance.com
Subject: Re: Re: [PATCH] vduse: Use fixed 4KB bounce pages for arm64 64KB page size

On Wed, Sep 24, 2025 at 12:05 PM Sheng Zhao <sheng.zhao@...edance.com> wrote:
>
>
>
> On 2025/9/24 08:57, Jason Wang wrote:
> > On Tue, Sep 23, 2025 at 8:37 PM Sheng Zhao <sheng.zhao@...edance.com> wrote:
> >>
> >>
> >>
> >> On 2025/9/17 16:16, Jason Wang wrote:
> >>> On Mon, Sep 15, 2025 at 3:34 PM <sheng.zhao@...edance.com> wrote:
> >>>>
> >>>> From: Sheng Zhao <sheng.zhao@...edance.com>
> >>>>
> >>>> The allocation granularity of bounce pages is PAGE_SIZE. This may cause
> >>>> even small IO requests to occupy an entire bounce page exclusively. The
> >>>> kind of memory waste will be more significant on arm64 with 64KB pages.
> >>>
> >>> Let's tweak the title as there are archs that are using non 4KB pages
> >>> other than arm.
> >>>
> >>
> >> Got it. I will modify this in v2.
> >>
> >>>>
> >>>> So, optimize it by using fixed 4KB bounce pages.
> >>>>
> >>>> Signed-off-by: Sheng Zhao <sheng.zhao@...edance.com>
> >>>> ---
> >>>>    drivers/vdpa/vdpa_user/iova_domain.c | 120 +++++++++++++++++----------
> >>>>    drivers/vdpa/vdpa_user/iova_domain.h |   5 ++
> >>>>    2 files changed, 83 insertions(+), 42 deletions(-)
> >>>>
> >>>> diff --git a/drivers/vdpa/vdpa_user/iova_domain.c b/drivers/vdpa/vdpa_user/iova_domain.c
> >>>> index 58116f89d8da..768313c80b62 100644
> >>>> --- a/drivers/vdpa/vdpa_user/iova_domain.c
> >>>> +++ b/drivers/vdpa/vdpa_user/iova_domain.c
> >>>> @@ -103,19 +103,26 @@ void vduse_domain_clear_map(struct vduse_iova_domain *domain,
> >>>>    static int vduse_domain_map_bounce_page(struct vduse_iova_domain *domain,
> >>>>                                            u64 iova, u64 size, u64 paddr)
> >>>>    {
> >>>> -       struct vduse_bounce_map *map;
> >>>> +       struct vduse_bounce_map *map, *head_map;
> >>>> +       struct page *tmp_page;
> >>>>           u64 last = iova + size - 1;
> >>>>
> >>>>           while (iova <= last) {
> >>>> -               map = &domain->bounce_maps[iova >> PAGE_SHIFT];
> >>>> +               map = &domain->bounce_maps[iova >> BOUNCE_PAGE_SHIFT];
> >>>
> >>> BOUNCE_PAGE_SIZE is kind of confusing as it's not the size of any page
> >>> at all when PAGE_SIZE is not 4K.
> >>>
> >>
> >> How about BOUNCE_MAP_SIZE?
> >
> > Fine with me.
> >
> >>
> >>>>                   if (!map->bounce_page) {
> >>>> -                       map->bounce_page = alloc_page(GFP_ATOMIC);
> >>>> -                       if (!map->bounce_page)
> >>>> -                               return -ENOMEM;
> >>>> +                       head_map = &domain->bounce_maps[(iova & PAGE_MASK) >> BOUNCE_PAGE_SHIFT];
> >>>> +                       if (!head_map->bounce_page) {
> >>>> +                               tmp_page = alloc_page(GFP_ATOMIC);
> >>>> +                               if (!tmp_page)
> >>>> +                                       return -ENOMEM;
> >>>> +                               if (cmpxchg(&head_map->bounce_page, NULL, tmp_page))
> >>>> +                                       __free_page(tmp_page);
> >>>
> >>> I don't understand why we need cmpxchg() logic.
> >>>
> >>> Btw, it looks like you want to make multiple bounce_map to point to
> >>> the same 64KB page? I wonder what's the advantages of doing this. Can
> >>> we simply keep the 64KB page in bounce_map?
> >>>
> >>> Thanks
> >>>
> >>
> >> That's correct. We use fixed 4KB-sized bounce pages, and there will be a
> >> many-to-one relationship between these 4KB bounce pages and the 64KB
> >> memory pages.
> >>
> >> Bounce pages are allocated on demand. As a result, it may occur that
> >> multiple bounce pages corresponding to the same 64KB memory page attempt
> >> to allocate memory simultaneously, so we use cmpxchg to handle this
> >> concurrency.
> >>
> >> In the current implementation, the bounce_map structure requires no
> >> modification. However, if we keep the 64KB page into a single bounce_map
> >> while still wanting to implement a similar logic, we may need an
> >> additional array to store multiple orig_phys values in order to
> >> accommodate the many-to-one relationship.
> >
> > Or simply having a bitmap is sufficient per bounce_map?
> >
>
> Yes, using a bitmap can mark the usage status of each 4KB, but it may
> not simplify things overall.
>
> - we will inevitably need to add an additional array per bounce_map to
> store the orig_phys corresponding to each 4KB for subsequent copying
> (vduse_domain_bounce).

I may miss something, the PAGE_SIZE is 64KB in this case, why do we
need to store per 4KB orig_phys?

>
> - compared to the current commit, this modification may only be a
> structural change and fail to reduce the amount of changes to the code
> logic. For instance, cmpxchg is still required.

Thanks

>
>
> Thanks
>
> > Thanks
> >
> >>
> >> Thanks
> >>
> >
>