Re: [PATCH 1/2] xen: add helpers for caching grant mapping pages

[boris.ostrovsky@xxxxxxxxxx]

On 12/7/20 8:30 AM, Juergen Gross wrote:
> Instead of having similar helpers in multiple backend drivers use
> common helpers for caching pages allocated via gnttab_alloc_pages().
>
> Make use of those helpers in blkback and scsiback.
>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>


Reviewed-by: Boris Ostrovsky <boris.ostrovksy@xxxxxxxxxx>


> +
> +void gnttab_page_cache_shrink(struct gnttab_page_cache *cache, unsigned int 
> num)
> +{
> +     struct page *page[10];
> +     unsigned int i = 0;
> +     unsigned long flags;
> +
> +     spin_lock_irqsave(&cache->lock, flags);
> +
> +     while (cache->num_pages > num) {
> +             page[i] = list_first_entry(&cache->pages, struct page, lru);
> +             list_del(&page[i]->lru);
> +             cache->num_pages--;
> +             if (++i == ARRAY_SIZE(page)) {
> +                     spin_unlock_irqrestore(&cache->lock, flags);
> +                     gnttab_free_pages(i, page);
> +                     i = 0;
> +                     spin_lock_irqsave(&cache->lock, flags);
> +             }
> +     }
> +
> +     spin_unlock_irqrestore(&cache->lock, flags);
> +
> +     if (i != 0)
> +             gnttab_free_pages(i, page);
> +}


How about splitting cache->pages list into two lists (one @num long and the 
other holding the rest) and then batching gntab_free_pages() from that first 
list? Then you won't have to deal with locking.


In fact, I am not even sure batching gains us much, I'd consider going 
one-by-one.


-boris