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)

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Reply via email to