Whamcloud - gitweb
LU-13805 llite: make page_list_{add,del} symmetric 57/52057/7
authorPatrick Farrell <pfarrell@whamcloud.com>
Sun, 17 Sep 2023 18:05:33 +0000 (14:05 -0400)
committerOleg Drokin <green@whamcloud.com>
Fri, 23 Feb 2024 07:00:07 +0000 (07:00 +0000)
An earlier patch created the slightly frightening situation
where we use cl_page_list_del to remove references which
were not taken by cl_page_list_add.

This assymetry is scary, so let's not do it.  Instead, DIO
now explicitly puts the only cl_page reference it takes.

Signed-off-by: Patrick Farrell <pfarrell@whamcloud.com>
Change-Id: I832d8ca7dc7f2f99dc30f972197bebc83b8b5977
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/52057
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Sebastien Buisson <sbuisson@ddn.com>
lustre/include/cl_object.h
lustre/llite/rw.c
lustre/llite/vvp_io.c
lustre/obdclass/cl_io.c
lustre/osc/osc_io.c

index 81a28e4..2af6dde 100644 (file)
@@ -2425,7 +2425,7 @@ static inline struct cl_page *cl_page_list_first(struct cl_page_list *plist)
 
 void cl_page_list_init(struct cl_page_list *plist);
 void cl_page_list_add(struct cl_page_list *plist, struct cl_page *page,
-                     bool get_ref);
+                     bool getref);
 void cl_page_list_move(struct cl_page_list *dst, struct cl_page_list *src,
                       struct cl_page *page);
 void cl_page_list_move_head(struct cl_page_list *dst, struct cl_page_list *src,
@@ -2433,7 +2433,8 @@ void cl_page_list_move_head(struct cl_page_list *dst, struct cl_page_list *src,
 void cl_page_list_splice(struct cl_page_list *list,
                         struct cl_page_list *head);
 void cl_page_list_del(const struct lu_env *env,
-                     struct cl_page_list *plist, struct cl_page *page);
+                     struct cl_page_list *plist, struct cl_page *page,
+                     bool putref);
 void cl_page_list_disown(const struct lu_env *env,
                         struct cl_page_list *plist);
 void cl_page_list_assume(const struct lu_env *env,
index c85b99b..49a46cf 100644 (file)
@@ -1774,7 +1774,7 @@ int ll_io_read_page(const struct lu_env *env, struct cl_io *io,
                rc = cl_sync_io_wait(env, anchor, 0);
 
                cl_page_assume(env, io, page);
-               cl_page_list_del(env, &queue->c2_qout, page);
+               cl_page_list_del(env, &queue->c2_qout, page, true);
 
                if (!PageUptodate(cl_page_vmpage(page))) {
                        /* Failed to read a mirror, discard this page so that
index 71c14ca..59e41da 100644 (file)
@@ -980,7 +980,7 @@ static int vvp_io_commit_sync(const struct lu_env *env, struct cl_io *io,
 
                while (plist->pl_nr > 0) {
                        page = cl_page_list_first(plist);
-                       cl_page_list_del(env, plist, page);
+                       cl_page_list_del(env, plist, page, true);
 
                        cl_page_clip(env, page, 0, PAGE_SIZE);
 
@@ -1249,7 +1249,7 @@ int vvp_io_write_commit(const struct lu_env *env, struct cl_io *io)
         * unless they were dirtied before. */
        while (queue->pl_nr > 0) {
                page = cl_page_list_first(queue);
-               cl_page_list_del(env, queue, page);
+               cl_page_list_del(env, queue, page, true);
 
                if (!PageDirty(cl_page_vmpage(page)))
                        cl_page_discard(env, io, page);
index 4035008..94e6bc8 100644 (file)
@@ -830,7 +830,7 @@ EXPORT_SYMBOL(cl_page_list_init);
  * Adds a page to a page list.
  */
 void cl_page_list_add(struct cl_page_list *plist, struct cl_page *page,
-                     bool get_ref)
+                     bool getref)
 {
        ENTRY;
        /* it would be better to check that page is owned by "current" io, but
@@ -841,7 +841,7 @@ void cl_page_list_add(struct cl_page_list *plist, struct cl_page *page,
        list_add_tail(&page->cp_batch, &plist->pl_pages);
        ++plist->pl_nr;
        lu_ref_add_at(&page->cp_reference, &page->cp_queue_ref, "queue", plist);
-       if (get_ref)
+       if (getref)
                cl_page_get(page);
        EXIT;
 }
@@ -851,7 +851,8 @@ EXPORT_SYMBOL(cl_page_list_add);
  * Removes a page from a page list.
  */
 void cl_page_list_del(const struct lu_env *env,
-                     struct cl_page_list *plist, struct cl_page *page)
+                     struct cl_page_list *plist, struct cl_page *page,
+                     bool putref)
 {
        LASSERT(plist->pl_nr > 0);
 
@@ -859,7 +860,8 @@ void cl_page_list_del(const struct lu_env *env,
        list_del_init(&page->cp_batch);
        --plist->pl_nr;
        lu_ref_del_at(&page->cp_reference, &page->cp_queue_ref, "queue", plist);
-       cl_page_put(env, page);
+       if (putref)
+               cl_page_put(env, page);
        EXIT;
 }
 EXPORT_SYMBOL(cl_page_list_del);
@@ -965,7 +967,7 @@ void cl_page_list_fini(const struct lu_env *env, struct cl_page_list *plist)
 
        ENTRY;
        cl_page_list_for_each_safe(page, temp, plist)
-               cl_page_list_del(env, plist, page);
+               cl_page_list_del(env, plist, page, true);
        LASSERT(plist->pl_nr == 0);
        EXIT;
 }
@@ -1204,7 +1206,8 @@ static void cl_sub_dio_end(const struct lu_env *env, struct cl_sync_io *anchor)
                struct cl_page *page = cl_page_list_first(&sdio->csd_pages);
 
                cl_page_delete(env, page);
-               cl_page_list_del(env, &sdio->csd_pages, page);
+               cl_page_list_del(env, &sdio->csd_pages, page, false);
+               cl_page_put(env, page);
        }
 
        if (sdio->csd_unaligned) {
index 816d743..7021e41 100644 (file)
@@ -208,7 +208,7 @@ int osc_io_submit(const struct lu_env *env, const struct cl_io_slice *ios,
                if (page->cp_sync_io != NULL)
                        cl_page_list_move(qout, qin, page);
                else /* async IO */
-                       cl_page_list_del(env, qin, page);
+                       cl_page_list_del(env, qin, page, true);
 
                queued++;
                if (queued == max_pages) {
@@ -356,7 +356,7 @@ int osc_io_commit_async(const struct lu_env *env,
                osc_page_touch_at(env, osc2cl(osc), osc_index(opg),
                                  page == last_page ? to : PAGE_SIZE);
 
-               cl_page_list_del(env, qin, page);
+               cl_page_list_del(env, qin, page, true);
 
                /* if there are no more slots, do the callback & reinit */
                if (pagevec_add(pvec, page->cp_vmpage) == 0) {