On Tue, Sep 23, 2025 at 8:37 PM Sheng Zhao <[email protected]> wrote:
>
>
>
> On 2025/9/17 16:16, Jason Wang wrote:
> > On Mon, Sep 15, 2025 at 3:34 PM <[email protected]> wrote:
> >>
> >> From: Sheng Zhao <[email protected]>
> >>
> >> 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 <[email protected]>
> >> ---
> >> 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?
Thanks
>
> Thanks
>
