On 07/27/2018 05:27 PM, Tony Battersby wrote: > On 07/27/2018 03:38 PM, Tony Battersby wrote: >> But the bigger problem is that my first patch adds another list_head to >> the dma_page for the avail_page_link to make allocations faster. I >> suppose we could make the lists singly-linked instead of doubly-linked >> to save space. >> > I managed to redo my dma_pool_alloc() patch to make avail_page_list > singly-linked instead of doubly-linked. But the problem with making > either list singly-linked is that it would no longer be possible to call > pool_free_page() any time other than dma_pool_destroy() without scanning > the lists to remove the page from them, which would make pruning > arbitrary free pages slower (adding back a O(n^2)). But the current > code doesn't do that anyway, and in fact it has a comment in > dma_pool_free() to "resist the temptation" to prune free pages. And yet > it seems like it might be reasonable for someone to add such code in the > future if there are a whole lot of free pages, so I am hesitant to make > it more difficult. > > So my question is: when I post v2 of the patchset, should I send the > doubly-linked version or the singly-linked version, in anticipation that > someone else might want to take it further and move everything into > struct page as you suggest? > Over the weekend I came up with a better solution. Instead of having the page listed in two singly-linked lists at the same time, move the page between two doubly-linked lists. One list is dedicated for pages that have all blocks allocated, and one list is for pages that have some blocks free. Since the page is only in one list at a time, it only needs one set of list pointers. I also implemented the code to make the offset 16-bit, while ignoring the offset for cases where it is not needed (where it would overflow anyway). So now I have an implementation that eliminates struct dma_page. I will post it once it is ready for review.
