On 2024/4/2 16:40, Chunhai Guo wrote:
Let's use alloc_pages_bulk_array() for simplicity and get rid of
unnecessary pagepool.

Signed-off-by: Chunhai Guo <guochun...@vivo.com>
---
  fs/erofs/zutil.c | 64 +++++++++++++++++++++++-------------------------
  1 file changed, 30 insertions(+), 34 deletions(-)

diff --git a/fs/erofs/zutil.c b/fs/erofs/zutil.c
index e13806681763..14440c0bf64e 100644
--- a/fs/erofs/zutil.c
+++ b/fs/erofs/zutil.c
@@ -60,61 +60,57 @@ void z_erofs_put_gbuf(void *ptr) __releases(gbuf->lock)
  int z_erofs_gbuf_growsize(unsigned int nrpages)
  {
        static DEFINE_MUTEX(gbuf_resize_mutex);
-       struct page *pagepool = NULL;
-       int delta, ret, i, j;
+       struct page **tmp_pages = NULL;
+       struct z_erofs_gbuf *gbuf;
+       void *ptr, *old_ptr;
+       int last, i, j;
+       int ret = 0;

no needed.

mutex_lock(&gbuf_resize_mutex);
-       delta = nrpages - z_erofs_gbuf_nrpages;
-       ret = 0;
        /* avoid shrinking gbufs, since no idea how many fses rely on */
-       if (delta <= 0)
+       if (nrpages <= z_erofs_gbuf_nrpages)
                goto out;

        if (nrpages <= z_erofs_gbuf_nrpages) {
                mutex_unlock(&gbuf_resize_mutex);
                return 0;
        }

since it's a fast side path, let's bail out this directly.

+ ret = -ENOMEM;

no needed.

        for (i = 0; i < z_erofs_gbuf_count; ++i) {
-               struct z_erofs_gbuf *gbuf = &z_erofs_gbufpool[i];
-               struct page **pages, **tmp_pages;
-               void *ptr, *old_ptr = NULL;
-
-               ret = -ENOMEM;
+               gbuf = &z_erofs_gbufpool[i];
                tmp_pages = kcalloc(nrpages, sizeof(*tmp_pages), GFP_KERNEL);
                if (!tmp_pages)
-                       break;
-               for (j = 0; j < nrpages; ++j) {
-                       tmp_pages[j] = erofs_allocpage(&pagepool, GFP_KERNEL);
-                       if (!tmp_pages[j])
-                               goto free_pagearray;
-               }
+                       goto out;
+
+               for (j = 0; j < gbuf->nrpages; ++j)
+                       tmp_pages[j] = gbuf->pages[j];
+               do {
+                       last = j;
+                       j = alloc_pages_bulk_array(GFP_KERNEL, nrpages,
+                                                  tmp_pages);
+                       if (last == j)
+                               goto out;
+               } while (j != nrpages);
+
                ptr = vmap(tmp_pages, nrpages, VM_MAP, PAGE_KERNEL);
                if (!ptr)
-                       goto free_pagearray;
+                       goto out;
- pages = tmp_pages;
                spin_lock(&gbuf->lock);
+               kfree(gbuf->pages);
+               gbuf->pages = tmp_pages;
                old_ptr = gbuf->ptr;
                gbuf->ptr = ptr;
-               tmp_pages = gbuf->pages;
-               gbuf->pages = pages;
-               j = gbuf->nrpages;
                gbuf->nrpages = nrpages;
                spin_unlock(&gbuf->lock);
-               ret = 0;
-               if (!tmp_pages) {
-                       DBG_BUGON(old_ptr);
-                       continue;
-               }
-
                if (old_ptr)
                        vunmap(old_ptr);
-free_pagearray:
-               while (j)
-                       erofs_pagepool_add(&pagepool, tmp_pages[--j]);
-               kfree(tmp_pages);
-               if (ret)
-                       break;
        }
+       ret = 0;
        z_erofs_gbuf_nrpages = nrpages;
-       erofs_release_pages(&pagepool);
  out:
+       if (ret && tmp_pages) {

        if (i < z_erofs_gbuf_count && tmp_pages) {

+               for (j = 0; j < nrpages; ++j)
+                       if (tmp_pages[j] && tmp_pages[j] != gbuf->pages[j])
+                               __free_page(tmp_pages[j]);
+               kfree(tmp_pages);
+       }
        mutex_unlock(&gbuf_resize_mutex);
        return ret;

        return i < z_erofs_gbuf_count ? -ENOMEM: 0;

Otherwise it looks good to me.

Thanks,
Gao Xiang

  }

Reply via email to