Re: [PATCH v13 05/12] bcache: bch_nvmpg_free_pages() of the buddy allocator

yukuai (C)
2022-02-21T12:36:13Z

在 2021/12/13 1:05, Coly Li 写道:
> From: Jianpeng Ma <jianpeng.ma@intel.com>
>
> This patch implements the bch_nvmpg_free_pages() of the buddy allocator.
>
> The difference between this and page-buddy-free:
> it need owner_uuid to free owner allocated pages, and must
> persistent after free.
>
> Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
> Co-developed-by: Qiaowei Ren <qiaowei.ren@intel.com>
> Signed-off-by: Qiaowei Ren <qiaowei.ren@intel.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Jens Axboe <axboe@kernel.dk>
> ---
> drivers/md/bcache/nvmpg.c | 164 ++++++++++++++++++++++++++++++++++++--
> drivers/md/bcache/nvmpg.h | 3 +
> 2 files changed, 160 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/md/bcache/nvmpg.c b/drivers/md/bcache/nvmpg.c
> index a920779eb548..8ce0c4389b42 100644
> --- a/drivers/md/bcache/nvmpg.c
> +++ b/drivers/md/bcache/nvmpg.c
> @@ -248,6 +248,57 @@ static int init_nvmpg_set_header(struct bch_nvmpg_ns *ns)
> return rc;
> }
>
> +static void __free_space(struct bch_nvmpg_ns *ns, unsigned long nvmpg_offset,
> + int order)
> +{
> + unsigned long add_pages = (1L << order);
> + pgoff_t pgoff;
> + struct page *page;
> + void *va;
> +
> + if (nvmpg_offset == 0) {
> + pr_err("free pages on offset 0\n");
> + return;
> + }
> +
> + page = bch_nvmpg_va_to_pg(bch_nvmpg_offset_to_ptr(nvmpg_offset));
> + WARN_ON((!page) || (page->private != order));
> + pgoff = page->index;
> +
> + while (order < BCH_MAX_ORDER - 1) {
> + struct page *buddy_page;
> +
> + pgoff_t buddy_pgoff = pgoff ^ (1L << order);
> + pgoff_t parent_pgoff = pgoff & ~(1L << order);
> +
> + if ((parent_pgoff + (1L << (order + 1)) > ns->pages_total))
> + break;
> +
> + va = bch_nvmpg_pgoff_to_ptr(ns, buddy_pgoff);
> + buddy_page = bch_nvmpg_va_to_pg(va);
> + WARN_ON(!buddy_page);
> +
> + if (PageBuddy(buddy_page) && (buddy_page->private == order)) {
> + list_del((struct list_head *)&buddy_page->zone_device_data);
> + __ClearPageBuddy(buddy_page);
> + pgoff = parent_pgoff;
> + order++;
> + continue;
> + }
> + break;
> + }
> +
> + va = bch_nvmpg_pgoff_to_ptr(ns, pgoff);
> + page = bch_nvmpg_va_to_pg(va);
> + WARN_ON(!page);
> + list_add((struct list_head *)&page->zone_device_data,
> + &ns->free_area[order]);
> + page->index = pgoff;
> + set_page_private(page, order);
> + __SetPageBuddy(page);
> + ns->free += add_pages;
> +}
> +
> static void bch_nvmpg_init_free_space(struct bch_nvmpg_ns *ns)
> {
> unsigned int start, end, pages;
> @@ -261,21 +312,19 @@ static void bch_nvmpg_init_free_space(struct bch_nvmpg_ns *ns)
> pages = end - start;
>
> while (pages) {
> - void *addr;
> -
> for (i = BCH_MAX_ORDER - 1; i >= 0; i--) {
> if ((pgoff_start % (1L << i) == 0) &&
> (pages >= (1L << i)))
> break;
> }
>
> - addr = bch_nvmpg_pgoff_to_ptr(ns, pgoff_start);
> - page = bch_nvmpg_va_to_pg(addr);
> + page = bch_nvmpg_va_to_pg(
> + bch_nvmpg_pgoff_to_ptr(ns, pgoff_start));
> set_page_private(page, i);
> page->index = pgoff_start;
> - __SetPageBuddy(page);
> - list_add((struct list_head *)&page->zone_device_data,
> - &ns->free_area[i]);
> +
> + /* In order to update ns->free */
> + __free_space(ns, pgoff_start, i);

Hi,

While testing this patchset, we found that this is probably wrong,
pgoff_start represent page number here, however, __free_space expect
this to be page offset. Maybe this should be:

__free_space(ns, pgoff_start << PAGE_SHIFT, i);

Thanks,
Kuai