On Wed, Sep 24, 2025 at 12:05 PM Sheng Zhao <[email protected]> wrote: > > > > On 2025/9/24 08:57, Jason Wang wrote: > > 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? > > > > 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 > >> > > >
