On 05/30/2018 01:49 PM, Oleksandr Andrushchenko wrote:
On 05/30/2018 06:20 PM, Boris Ostrovsky wrote:
On 05/30/2018 02:34 AM, Oleksandr Andrushchenko wrote:
On 05/29/2018 10:10 PM, Boris Ostrovsky wrote:
On 05/25/2018 11:33 AM, Oleksandr Andrushchenko wrote: +/**
- gnttab_dma_free_pages - free DMAable pages
- @args: arguments to the function
- */
+int gnttab_dma_free_pages(struct gnttab_dma_alloc_args *args) +{ + xen_pfn_t *frames; + size_t size; + int i, ret;
+ gnttab_pages_clear_private(args->nr_pages, args->pages);
+ frames = kcalloc(args->nr_pages, sizeof(*frames), GFP_KERNEL);
Any way you can do it without allocating memory? One possibility is to keep allocated frames from gnttab_dma_alloc_pages(). (Not sure I like that either but it's the only thing I can think of).
Yes, I was also thinking about storing the allocated frames array from gnttab_dma_alloc_pages(), but that seemed not to be clear enough as the caller of the gnttab_dma_alloc_pages will need to store those frames in some context, so we can pass them on free. But the caller doesn't really need the frames which might confuse, so I decided to make those allocations on the fly. But I can still rework that to store the frames if you insist: please let me know.
I would prefer not to allocate anything in the release path. Yes, I realize that dragging frames array around is not necessary but IMO it's better than potentially failing an allocation during a teardown. A comment in the struct definition could explain the reason for having this field.
Then I would suggest we have it this way: current API requires that struct page **pages are allocated from outside. So, let's allocate the frames from outside as well. This way the caller is responsible for both pages and frames arrays and API looks consistent.
Yes, that works too.
-boris