Messages in this thread Patch in this message | | | Date | Thu, 16 Feb 2023 19:04:24 +0000 | From | Matthew Wilcox <> | Subject | Re: [PATCH] dm-crypt: allocate compound pages if possible |
| |
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)
| |