2024-02-03 12:20:12

by Christoph Hellwig

[permalink] [raw]
Subject: convert write_cache_pages() to an iterator v6

Hi all,

this is an evolution of the series Matthew Wilcox originally sent in June
2023, which has changed quite a bit since and now has a while based
iterator.

Note that in this version two patches are so different from the previous
version that I've not kept any Reviews or Acks for them, even if the
final result look almost the same as the previous patches with the
incremental patch on the list.

Changes since v5:
- completely reshuffle the series to directly prepare for the
writeback_iter() style.
- don't require *error to be initialized on first call
- improve various comments
- fix a bisection hazard where write_cache_pages don't return delayed
error for a few commits
- fix a whitespace error
- drop the iomap patch again for now as the iomap map multiple blocks
series isn't in mainline yet

Changes since v4:
- added back the (rebased) iomap conversion now that the conflict is in
mainline
- add a new patch to change the iterator

Changes since v3:
- various commit log spelling fixes
- remove a statement from a commit log that isn't true any more with the
changes in v3
- rename a function
- merge two helpers

Diffstat:
fs/iomap/buffered-io.c | 10 -
include/linux/pagevec.h | 18 ++
include/linux/writeback.h | 12 +
mm/page-writeback.c | 344 ++++++++++++++++++++++++++--------------------
4 files changed, 231 insertions(+), 153 deletions(-)


2024-02-03 12:20:18

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 01/13] writeback: remove a duplicate prototype for tag_pages_for_writeback

From: "Matthew Wilcox (Oracle)" <[email protected]>

Signed-off-by: Christoph Hellwig <[email protected]>
[hch: split from a larger patch]
Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
Reviewed-by: Brian Foster <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
Acked-by: Dave Chinner <[email protected]>
---
include/linux/writeback.h | 2 --
1 file changed, 2 deletions(-)

diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 453736fd1d23ce..4b8cf9e4810bad 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -363,8 +363,6 @@ bool wb_over_bg_thresh(struct bdi_writeback *wb);
typedef int (*writepage_t)(struct folio *folio, struct writeback_control *wbc,
void *data);

-void tag_pages_for_writeback(struct address_space *mapping,
- pgoff_t start, pgoff_t end);
int write_cache_pages(struct address_space *mapping,
struct writeback_control *wbc, writepage_t writepage,
void *data);
--
2.39.2


2024-02-03 13:12:20

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 02/13] writeback: fix done_index when hitting the wbc->nr_to_write

When write_cache_pages finishes writing out a folio, it fails to update
done_index to account for the number of pages in the folio just written.
That means when range_cyclic writeback is restarted, it will be
restarted at this folio instead of after it as it should. Fix that
by updating done_index before breaking out of the loop.

Signed-off-by: Christoph Hellwig <[email protected]>
Reviewed-by: Brian Foster <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
Acked-by: Dave Chinner <[email protected]>
---
mm/page-writeback.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 02147b61712bc9..b4d978f77b0b69 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2505,6 +2505,7 @@ int write_cache_pages(struct address_space *mapping,
* keep going until we have written all the pages
* we tagged for writeback prior to entering this loop.
*/
+ done_index = folio->index + nr;
wbc->nr_to_write -= nr;
if (wbc->nr_to_write <= 0 &&
wbc->sync_mode == WB_SYNC_NONE) {
--
2.39.2


2024-02-03 13:35:36

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 09/13] pagevec: Add ability to iterate a queue

From: "Matthew Wilcox (Oracle)" <[email protected]>

Add a loop counter inside the folio_batch to let us iterate from 0-nr
instead of decrementing nr and treating the batch as a stack. It would
generate some very weird and suboptimal I/O patterns for page writeback
to iterate over the batch as a stack.

Signed-off-by: Matthew Wilcox (Oracle) <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
Reviewed-by: Brian Foster <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
Acked-by: Dave Chinner <[email protected]>
---
include/linux/pagevec.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/include/linux/pagevec.h b/include/linux/pagevec.h
index 87cc678adc850b..fcc06c300a72c3 100644
--- a/include/linux/pagevec.h
+++ b/include/linux/pagevec.h
@@ -27,6 +27,7 @@ struct folio;
*/
struct folio_batch {
unsigned char nr;
+ unsigned char i;
bool percpu_pvec_drained;
struct folio *folios[PAGEVEC_SIZE];
};
@@ -40,12 +41,14 @@ struct folio_batch {
static inline void folio_batch_init(struct folio_batch *fbatch)
{
fbatch->nr = 0;
+ fbatch->i = 0;
fbatch->percpu_pvec_drained = false;
}

static inline void folio_batch_reinit(struct folio_batch *fbatch)
{
fbatch->nr = 0;
+ fbatch->i = 0;
}

static inline unsigned int folio_batch_count(struct folio_batch *fbatch)
@@ -75,6 +78,21 @@ static inline unsigned folio_batch_add(struct folio_batch *fbatch,
return folio_batch_space(fbatch);
}

+/**
+ * folio_batch_next - Return the next folio to process.
+ * @fbatch: The folio batch being processed.
+ *
+ * Use this function to implement a queue of folios.
+ *
+ * Return: The next folio in the queue, or NULL if the queue is empty.
+ */
+static inline struct folio *folio_batch_next(struct folio_batch *fbatch)
+{
+ if (fbatch->i == fbatch->nr)
+ return NULL;
+ return fbatch->folios[fbatch->i++];
+}
+
void __folio_batch_release(struct folio_batch *pvec);

static inline void folio_batch_release(struct folio_batch *fbatch)
--
2.39.2