Whamcloud - gitweb
EX-8270 ptlrpc: refactor pool growing code
authorPatrick Farrell <pfarrell@whamcloud.com>
Wed, 20 Sep 2023 21:26:31 +0000 (17:26 -0400)
committerAndreas Dilger <adilger@whamcloud.com>
Fri, 22 Sep 2023 23:54:55 +0000 (23:54 +0000)
This refactors the pool growing code, combining two
separate instances of it in to a single function.

Test-Parameters: trivial
Test-Parameters: kerberos=true testlist=sanity-krb5
Signed-off-by: Patrick Farrell <pfarrell@whamcloud.com>
Change-Id: I175abc7e61d55563e989f87207a8c59da852f5f9
Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/52443
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
lustre/ptlrpc/sec_bulk.c

index afffd2c..f70cfe6 100644 (file)
@@ -671,6 +671,8 @@ static inline void **page_from_bufarray(void *array, int index)
        return (void **)array;
 }
 
+static bool __grow_pool_try(int needed, struct ptlrpc_page_pool *pool);
+
 /*
  * we allocate the requested pages atomically.
  */
@@ -699,19 +701,17 @@ again:
                page_pool->ppp_st_missings++;
                page_pool->ppp_pages_short += count;
 
-               if (pool_should_grow(count, page_pool)) {
-                       page_pool->ppp_growing = 1;
-
-                       spin_unlock(&page_pool->ppp_lock);
-                       CDEBUG(D_SEC, "ppp_pages_short: %lu\n",
-                              page_pool->ppp_pages_short);
-                       pool_add_pages(8, page_pool);
-                       spin_lock(&page_pool->ppp_lock);
-
-                       page_pool->ppp_growing = 0;
-
-                       pool_wakeup(page_pool);
-               } else {
+               /* if we aren't able to add pages, check if someone else is
+                * growing the pool and sleep if so, otherwise we return
+                * ENOMEM because we can't sleep here waiting for other ops to
+                * complete (main user is ptlrpcd, which must not sleep waiting
+                * for other ops...  technically sleeping for pool growth is
+                * also questionable but it's very unlikely in practice to get
+                * stuck from this)
+                *
+                * if ENOMEM is returned here, the RPC will go back in the queue
+                */
+               if (!__grow_pool_try(count, page_pool)) {
                        if (page_pool->ppp_growing) {
                                if (++page_pool->ppp_waitqlen >
                                    page_pool->ppp_st_max_wqlen)
@@ -934,31 +934,69 @@ void sptlrpc_pool_put_pages(void *buf, unsigned int order)
 }
 EXPORT_SYMBOL(sptlrpc_pool_put_pages);
 
-
-/*
- * we don't do much stuff for add_user/del_user anymore, except adding some
- * initial pages in add_user() if current pools are empty, rest would be
- * handled by the pools's self-adaption.
- */
-void sptlrpc_pool_add_user(void)
+/* called with pool->ppp_lock held */
+static bool __grow_pool_try(int needed, struct ptlrpc_page_pool *pool)
 {
-       struct ptlrpc_page_pool *pool = page_pools[PAGES_POOL];
+       bool pool_grown = false;
+
+       assert_spin_locked(&pool->ppp_lock);
+
+       if (pool_should_grow(needed, pool)) {
+               unsigned int to_add;
+               int rc;
 
-       spin_lock(&pool->ppp_lock);
-       /* ask for 1 page - so if the pool is empty, it will grow
-        * (this might also grow an in-use pool if it's full, which is fine)
-        */
-       if (pool_should_grow(1, pool)) {
                pool->ppp_growing = 1;
+               /* the pool of single pages is grown a large amount on
+                * first use
+                */
+               if (pool->ppp_index == PAGES_POOL &&
+                   pool->ppp_total_pages == 0)
+                       to_add = PTLRPC_MAX_BRW_PAGES * 2;
+               else /* otherwise, we add requested or at least 8 items */
+                       to_add = max(needed, 8);
                spin_unlock(&pool->ppp_lock);
 
-               pool_add_pages(PTLRPC_MAX_BRW_PAGES * 2, pool);
+               CDEBUG(D_SEC,
+                      "pool %d is %lu elements (size %d bytes), growing by %d items\n",
+                       pool->ppp_index, pool->ppp_pages_short,
+                       ELEMENT_SIZE(pool->ppp_index), to_add);
+               /* we can't hold a spinlock over page allocation */
+               rc = pool_add_pages(to_add, pool);
+               if (rc == 0)
+                       pool_grown = true;
 
                spin_lock(&pool->ppp_lock);
                pool->ppp_growing = 0;
                pool_wakeup(pool);
        }
+
+       return pool_grown;
+}
+
+static bool grow_pool_try(int needed, struct ptlrpc_page_pool *pool)
+{
+       bool rc;
+
+       spin_lock(&pool->ppp_lock);
+       rc = __grow_pool_try(needed, pool);
        spin_unlock(&pool->ppp_lock);
+
+       return rc;
+}
+
+/*
+ * we don't do much stuff for add_user/del_user anymore, except adding some
+ * initial pages in add_user() if current pools are empty, rest would be
+ * handled by the pools's self-adaption.
+ */
+void sptlrpc_pool_add_user(void)
+{
+       struct ptlrpc_page_pool *pool = page_pools[PAGES_POOL];
+
+       /* since this is startup, no one is waiting for these pages, so we
+        * don't worry about sucess or failure here
+        */
+       grow_pool_try(1, pool);
 }
 EXPORT_SYMBOL(sptlrpc_pool_add_user);