From 7c98f16b91b5f34fcbdb98a37f0d8115e31a7297 Mon Sep 17 00:00:00 2001 From: Patrick Farrell Date: Sun, 17 Sep 2023 14:05:33 -0400 Subject: [PATCH] LU-13805 llite: make page_list_{add,del} symmetric 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 Change-Id: I832d8ca7dc7f2f99dc30f972197bebc83b8b5977 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/52057 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Oleg Drokin Reviewed-by: Andreas Dilger Reviewed-by: Sebastien Buisson --- lustre/include/cl_object.h | 5 +++-- lustre/llite/rw.c | 2 +- lustre/llite/vvp_io.c | 4 ++-- lustre/obdclass/cl_io.c | 15 +++++++++------ lustre/osc/osc_io.c | 4 ++-- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/lustre/include/cl_object.h b/lustre/include/cl_object.h index 81a28e4..2af6dde 100644 --- a/lustre/include/cl_object.h +++ b/lustre/include/cl_object.h @@ -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, diff --git a/lustre/llite/rw.c b/lustre/llite/rw.c index c85b99b..49a46cf 100644 --- a/lustre/llite/rw.c +++ b/lustre/llite/rw.c @@ -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 diff --git a/lustre/llite/vvp_io.c b/lustre/llite/vvp_io.c index 71c14ca..59e41da 100644 --- a/lustre/llite/vvp_io.c +++ b/lustre/llite/vvp_io.c @@ -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); diff --git a/lustre/obdclass/cl_io.c b/lustre/obdclass/cl_io.c index 4035008..94e6bc8 100644 --- a/lustre/obdclass/cl_io.c +++ b/lustre/obdclass/cl_io.c @@ -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) { diff --git a/lustre/osc/osc_io.c b/lustre/osc/osc_io.c index 816d743..7021e41 100644 --- a/lustre/osc/osc_io.c +++ b/lustre/osc/osc_io.c @@ -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) { -- 1.8.3.1