On Thu, Feb 16, 2023 at 12:47:08PM -0500, Mikulas Patocka wrote:
> + while (order > 0) {
> + page = alloc_pages(gfp_mask
> + | __GFP_NOMEMALLOC | __GFP_NORETRY | __GFP_NOWARN, order);
... | __GFP_COMP
> page = mempool_alloc(&cc->page_pool, gfp_mask);
> if (!page) {
> crypt_free_buffer_pages(cc, clone);
> bio_put(clone);
> gfp_mask |= __GFP_DIRECT_RECLAIM;
> + order = 0;
> goto retry;
> }
>
> - len = (remaining_size > PAGE_SIZE) ? PAGE_SIZE : remaining_size;
> -
> - bio_add_page(clone, page, len, 0);
> +have_pages:
> + page->compound_order = order;
No. You'll corrupt the next page if page is order-0, which it is if it
came from the mempool. Also we've deleted page->compound_order in -next
so you can't make this mistake. Using __GFP_COMP will set this field
for you, so you can just drop this line.
> - remaining_size -= len;
> + for (o = 0; o < 1U << order; o++) {
> + unsigned len = min((unsigned)PAGE_SIZE, remaining_size);
> + bio_add_page(clone, page, len, 0);
> + remaining_size -= len;
> + page++;
You can add multiple pages at once, whether they're compound or not. So
replace this entire loop with:
bio_add_page(clone, page, remaining_size, 0);
> @@ -1711,10 +1732,23 @@ static void crypt_free_buffer_pages(stru
> {
> struct bio_vec *bv;
> struct bvec_iter_all iter_all;
> + unsigned skip_entries = 0;
>
> bio_for_each_segment_all(bv, clone, iter_all) {
> - BUG_ON(!bv->bv_page);
> - mempool_free(bv->bv_page, &cc->page_pool);
> + unsigned order;
> + struct page *page = bv->bv_page;
> + BUG_ON(!page);
> + if (skip_entries) {
> + skip_entries--;
> + continue;
> + }
> + order = page->compound_order;
> + if (order) {
> + __free_pages(page, order);
> + skip_entries = (1U << order) - 1;
> + } else {
> + mempool_free(page, &cc->page_pool);
> + }
You can simplify this by using the folio code.
struct folio_iter fi;
bio_for_each_folio_all(fi, bio) {
if (folio_test_large(folio))
folio_put(folio);
else
mempool_free(&folio->page, &cc->page_pool);
}
(further work would actually convert this driver to use folios instead
of pages)
