[PATCH 00/15] ubifs folio conversion

[PATCH 00/15] ubifs folio conversion

[Matthew Wilcox (Oracle)]

This patchset converts ubifs to use folios throughout.  I made some
attempt to support large folios, but more work definitely needs to be
done before it can be enabled.  It's not clear to me whether it's worth
doing, so I've left that for you ;-)

The first patch fixes a misplaced call to SetPageUptodate which has been
with us since the initial merge of ubifs.  It looks to be a pretty hard
race to hit, which is why it probably hasn't affected anyone.

With my usual test config, this saves about 2kB of kernel text (almost
1%).  Some functions shrink more than others, eg ubifs_write_begin
reduces by almost a third.  YMMV with different configs.

I've done a few other updates while I'm in here; eg setting/clearing
PageError is pointless because the VFS doesn't use it and ubifs doesn't
test it.  And we're phasing out use of PagePrivate / folio_test_private()
in favour of just testing whether folio->private is set.

I ran this through xfstests and it didn't seem any worse.

Matthew Wilcox (Oracle) (15):
  ubifs: Set page uptodate in the correct place
  ubifs: Convert from writepage to writepages
  ubifs: Convert ubifs_writepage to use a folio
  ubifs: Use a folio in do_truncation()
  ubifs: Convert do_writepage() to take a folio
  ubifs: Convert ubifs_vm_page_mkwrite() to use a folio
  ubifs: Convert write_begin_slow() to use a folio
  ubifs: Convert ubifs_write_begin() to use a folio
  ubifs: Convert ubifs_write_end() to use a folio
  ubifs: Convert do_readpage() to take a folio
  ubifs: Convert allocate_budget() to work on a folio
  ubifs: Convert cancel_budget() to take a folio
  ubifs: Pass a folio into ubifs_bulk_read() and ubifs_do_bulk_read()
  ubifs: Use a folio in ubifs_do_bulk_read()
  ubifs: Convert populate_page() to take a folio

 Documentation/mm/page_cache.rst |  10 +
 fs/ubifs/file.c                 | 443 ++++++++++++++++----------------
 2 files changed, 232 insertions(+), 221 deletions(-)

-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 01/15] ubifs: Set page uptodate in the correct place

[Matthew Wilcox (Oracle)]

Page cache reads are lockless, so setting the freshly allocated page
uptodate before we've overwritten it with the data it's supposed to have
in it will allow a simultaneous reader to see old data.  Move the call
to SetPageUptodate into ubifs_write_end(), which is after we copied the
new data into the page.

Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
Cc: stable@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 5029eb3390a5..40a9b03ef821 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -463,9 +463,6 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 				return err;
 			}
 		}
-
-		SetPageUptodate(page);
-		ClearPageError(page);
 	}
 
 	err = allocate_budget(c, page, ui, appending);
@@ -569,6 +566,9 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
 		goto out;
 	}
 
+	if (len == PAGE_SIZE)
+		SetPageUptodate(page);
+
 	if (!PagePrivate(page)) {
 		attach_page_private(page, (void *)1);
 		atomic_long_inc(&c->dirty_pg_cnt);
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 01/15] ubifs: Set page uptodate in the correct place

[Matthew Wilcox (Oracle)]

Page cache reads are lockless, so setting the freshly allocated page
uptodate before we've overwritten it with the data it's supposed to have
in it will allow a simultaneous reader to see old data.  Move the call
to SetPageUptodate into ubifs_write_end(), which is after we copied the
new data into the page.

Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
Cc: stable@vger.kernel.org
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 5029eb3390a5..40a9b03ef821 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -463,9 +463,6 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 				return err;
 			}
 		}
-
-		SetPageUptodate(page);
-		ClearPageError(page);
 	}
 
 	err = allocate_budget(c, page, ui, appending);
@@ -569,6 +566,9 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
 		goto out;
 	}
 
+	if (len == PAGE_SIZE)
+		SetPageUptodate(page);
+
 	if (!PagePrivate(page)) {
 		attach_page_private(page, (void *)1);
 		atomic_long_inc(&c->dirty_pg_cnt);
-- 
2.43.0

[PATCH 02/15] ubifs: Convert from writepage to writepages

[Matthew Wilcox (Oracle)]

This is a simplistic conversion to separate out any effects of
no longer having a writepage method.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 40a9b03ef821..007a73052561 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1006,8 +1006,10 @@ static int do_writepage(struct page *page, int len)
  * on the page lock and it would not write the truncated inode node to the
  * journal before we have finished.
  */
-static int ubifs_writepage(struct page *page, struct writeback_control *wbc)
+static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
+		void *data)
 {
+	struct page *page = &folio->page;
 	struct inode *inode = page->mapping->host;
 	struct ubifs_info *c = inode->i_sb->s_fs_info;
 	struct ubifs_inode *ui = ubifs_inode(inode);
@@ -1079,6 +1081,12 @@ static int ubifs_writepage(struct page *page, struct writeback_control *wbc)
 	return err;
 }
 
+static int ubifs_writepages(struct address_space *mapping,
+		struct writeback_control *wbc)
+{
+	return write_cache_pages(mapping, wbc, ubifs_writepage, NULL);
+}
+
 /**
  * do_attr_changes - change inode attributes.
  * @inode: inode to change attributes for
@@ -1648,7 +1656,7 @@ static int ubifs_symlink_getattr(struct mnt_idmap *idmap,
 
 const struct address_space_operations ubifs_file_address_operations = {
 	.read_folio     = ubifs_read_folio,
-	.writepage      = ubifs_writepage,
+	.writepages     = ubifs_writepages,
 	.write_begin    = ubifs_write_begin,
 	.write_end      = ubifs_write_end,
 	.invalidate_folio = ubifs_invalidate_folio,
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 03/15] ubifs: Convert ubifs_writepage to use a folio

[Matthew Wilcox (Oracle)]

We still pass the page down to do_writepage(), but ubifs_writepage()
itself is now large folio safe.  It also contains far fewer hidden calls
to compound_head().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 007a73052561..7d0967daded3 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1009,21 +1009,18 @@ static int do_writepage(struct page *page, int len)
 static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
 		void *data)
 {
-	struct page *page = &folio->page;
-	struct inode *inode = page->mapping->host;
+	struct inode *inode = folio->mapping->host;
 	struct ubifs_info *c = inode->i_sb->s_fs_info;
 	struct ubifs_inode *ui = ubifs_inode(inode);
 	loff_t i_size =  i_size_read(inode), synced_i_size;
-	pgoff_t end_index = i_size >> PAGE_SHIFT;
-	int err, len = i_size & (PAGE_SIZE - 1);
-	void *kaddr;
+	int err, len = folio_size(folio);
 
 	dbg_gen("ino %lu, pg %lu, pg flags %#lx",
-		inode->i_ino, page->index, page->flags);
-	ubifs_assert(c, PagePrivate(page));
+		inode->i_ino, folio->index, folio->flags);
+	ubifs_assert(c, folio->private != NULL);
 
-	/* Is the page fully outside @i_size? (truncate in progress) */
-	if (page->index > end_index || (page->index == end_index && !len)) {
+	/* Is the folio fully outside @i_size? (truncate in progress) */
+	if (folio_pos(folio) >= i_size) {
 		err = 0;
 		goto out_unlock;
 	}
@@ -1032,9 +1029,9 @@ static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
 	synced_i_size = ui->synced_i_size;
 	spin_unlock(&ui->ui_lock);
 
-	/* Is the page fully inside @i_size? */
-	if (page->index < end_index) {
-		if (page->index >= synced_i_size >> PAGE_SHIFT) {
+	/* Is the folio fully inside i_size? */
+	if (folio_pos(folio) + len <= i_size) {
+		if (folio_pos(folio) >= synced_i_size) {
 			err = inode->i_sb->s_op->write_inode(inode, NULL);
 			if (err)
 				goto out_redirty;
@@ -1047,20 +1044,18 @@ static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
 			 * with this.
 			 */
 		}
-		return do_writepage(page, PAGE_SIZE);
+		return do_writepage(&folio->page, len);
 	}
 
 	/*
-	 * The page straddles @i_size. It must be zeroed out on each and every
+	 * The folio straddles @i_size. It must be zeroed out on each and every
 	 * writepage invocation because it may be mmapped. "A file is mapped
 	 * in multiples of the page size. For a file that is not a multiple of
 	 * the page size, the remaining memory is zeroed when mapped, and
 	 * writes to that region are not written out to the file."
 	 */
-	kaddr = kmap_atomic(page);
-	memset(kaddr + len, 0, PAGE_SIZE - len);
-	flush_dcache_page(page);
-	kunmap_atomic(kaddr);
+	len = i_size - folio_pos(folio);
+	folio_zero_segment(folio, len, folio_size(folio));
 
 	if (i_size > synced_i_size) {
 		err = inode->i_sb->s_op->write_inode(inode, NULL);
@@ -1068,16 +1063,16 @@ static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
 			goto out_redirty;
 	}
 
-	return do_writepage(page, len);
+	return do_writepage(&folio->page, len);
 out_redirty:
 	/*
-	 * redirty_page_for_writepage() won't call ubifs_dirty_inode() because
+	 * folio_redirty_for_writepage() won't call ubifs_dirty_inode() because
 	 * it passes I_DIRTY_PAGES flag while calling __mark_inode_dirty(), so
 	 * there is no need to do space budget for dirty inode.
 	 */
-	redirty_page_for_writepage(wbc, page);
+	folio_redirty_for_writepage(wbc, folio);
 out_unlock:
-	unlock_page(page);
+	folio_unlock(folio);
 	return err;
 }
 
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 04/15] ubifs: Use a folio in do_truncation()

[Matthew Wilcox (Oracle)]

Convert from the old page APIs to the new folio APIs which saves
a few hidden calls to compound_head().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 7d0967daded3..b297582c9b75 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1158,11 +1158,11 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
 
 	if (offset) {
 		pgoff_t index = new_size >> PAGE_SHIFT;
-		struct page *page;
+		struct folio *folio;
 
-		page = find_lock_page(inode->i_mapping, index);
-		if (page) {
-			if (PageDirty(page)) {
+		folio = filemap_lock_folio(inode->i_mapping, index);
+		if (!IS_ERR(folio)) {
+			if (folio_test_dirty(folio)) {
 				/*
 				 * 'ubifs_jnl_truncate()' will try to truncate
 				 * the last data node, but it contains
@@ -1171,14 +1171,14 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
 				 * 'ubifs_jnl_truncate()' will see an already
 				 * truncated (and up to date) data node.
 				 */
-				ubifs_assert(c, PagePrivate(page));
+				ubifs_assert(c, folio->private != NULL);
 
-				clear_page_dirty_for_io(page);
+				folio_clear_dirty_for_io(folio);
 				if (UBIFS_BLOCKS_PER_PAGE_SHIFT)
-					offset = new_size &
-						 (PAGE_SIZE - 1);
-				err = do_writepage(page, offset);
-				put_page(page);
+					offset = offset_in_folio(folio,
+							new_size);
+				err = do_writepage(&folio->page, offset);
+				folio_put(folio);
 				if (err)
 					goto out_budg;
 				/*
@@ -1191,8 +1191,8 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
 				 * to 'ubifs_jnl_truncate()' to save it from
 				 * having to read it.
 				 */
-				unlock_page(page);
-				put_page(page);
+				folio_unlock(folio);
+				folio_put(folio);
 			}
 		}
 	}
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 05/15] ubifs: Convert do_writepage() to take a folio

[Matthew Wilcox (Oracle)]

Replace the call to SetPageError() with a call to mapping_set_error().
Support large folios by using kmap_local_folio() and remapping each time
we cross a page boundary.  Saves a lot of hidden calls to compound_head().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 56 ++++++++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 26 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index b297582c9b75..e755d0a11146 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -903,60 +903,64 @@ static int ubifs_read_folio(struct file *file, struct folio *folio)
 	return 0;
 }
 
-static int do_writepage(struct page *page, int len)
+static int do_writepage(struct folio *folio, size_t len)
 {
-	int err = 0, i, blen;
+	int err = 0, blen;
 	unsigned int block;
 	void *addr;
+	size_t offset = 0;
 	union ubifs_key key;
-	struct inode *inode = page->mapping->host;
+	struct inode *inode = folio->mapping->host;
 	struct ubifs_info *c = inode->i_sb->s_fs_info;
 
 #ifdef UBIFS_DEBUG
 	struct ubifs_inode *ui = ubifs_inode(inode);
 	spin_lock(&ui->ui_lock);
-	ubifs_assert(c, page->index <= ui->synced_i_size >> PAGE_SHIFT);
+	ubifs_assert(c, folio->index <= ui->synced_i_size >> PAGE_SHIFT);
 	spin_unlock(&ui->ui_lock);
 #endif
 
-	/* Update radix tree tags */
-	set_page_writeback(page);
+	folio_start_writeback(folio);
 
-	addr = kmap(page);
-	block = page->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
-	i = 0;
-	while (len) {
-		blen = min_t(int, len, UBIFS_BLOCK_SIZE);
+	addr = kmap_local_folio(folio, offset);
+	block = folio->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
+	for (;;) {
+		blen = min_t(size_t, len, UBIFS_BLOCK_SIZE);
 		data_key_init(c, &key, inode->i_ino, block);
 		err = ubifs_jnl_write_data(c, inode, &key, addr, blen);
 		if (err)
 			break;
-		if (++i >= UBIFS_BLOCKS_PER_PAGE)
+		len -= blen;
+		if (!len)
 			break;
 		block += 1;
 		addr += blen;
-		len -= blen;
+		if (folio_test_highmem(folio) && !offset_in_page(addr)) {
+			kunmap_local(addr - blen);
+			offset += PAGE_SIZE;
+			addr = kmap_local_folio(folio, offset);
+		}
 	}
+	kunmap_local(addr);
 	if (err) {
-		SetPageError(page);
-		ubifs_err(c, "cannot write page %lu of inode %lu, error %d",
-			  page->index, inode->i_ino, err);
+		mapping_set_error(folio->mapping, err);
+		ubifs_err(c, "cannot write folio %lu of inode %lu, error %d",
+			  folio->index, inode->i_ino, err);
 		ubifs_ro_mode(c, err);
 	}
 
-	ubifs_assert(c, PagePrivate(page));
-	if (PageChecked(page))
+	ubifs_assert(c, folio->private != NULL);
+	if (folio_test_checked(folio))
 		release_new_page_budget(c);
 	else
 		release_existing_page_budget(c);
 
 	atomic_long_dec(&c->dirty_pg_cnt);
-	detach_page_private(page);
-	ClearPageChecked(page);
+	folio_detach_private(folio);
+	folio_clear_checked(folio);
 
-	kunmap(page);
-	unlock_page(page);
-	end_page_writeback(page);
+	folio_unlock(folio);
+	folio_end_writeback(folio);
 	return err;
 }
 
@@ -1044,7 +1048,7 @@ static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
 			 * with this.
 			 */
 		}
-		return do_writepage(&folio->page, len);
+		return do_writepage(folio, len);
 	}
 
 	/*
@@ -1063,7 +1067,7 @@ static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
 			goto out_redirty;
 	}
 
-	return do_writepage(&folio->page, len);
+	return do_writepage(folio, len);
 out_redirty:
 	/*
 	 * folio_redirty_for_writepage() won't call ubifs_dirty_inode() because
@@ -1177,7 +1181,7 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
 				if (UBIFS_BLOCKS_PER_PAGE_SHIFT)
 					offset = offset_in_folio(folio,
 							new_size);
-				err = do_writepage(&folio->page, offset);
+				err = do_writepage(folio, offset);
 				folio_put(folio);
 				if (err)
 					goto out_budg;
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 06/15] ubifs: Convert ubifs_vm_page_mkwrite() to use a folio

[Matthew Wilcox (Oracle)]

Replace six implicit calls to compound_head() with one.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 Documentation/mm/page_cache.rst | 10 +++++++++
 fs/ubifs/file.c                 | 36 ++++++++++++++++-----------------
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/Documentation/mm/page_cache.rst b/Documentation/mm/page_cache.rst
index 75eba7c431b2..138d61f869df 100644
--- a/Documentation/mm/page_cache.rst
+++ b/Documentation/mm/page_cache.rst
@@ -3,3 +3,13 @@
 ==========
 Page Cache
 ==========
+
+The page cache is the primary way that the user and the rest of the kernel
+interact with filesystems.  It can be bypassed (e.g. with O_DIRECT),
+but normal reads, writes and mmaps go through the page cache.
+
+Folios
+======
+
+The folio is the unit of memory management within the page cache.
+Operations 
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index e755d0a11146..9dfedd91b576 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1519,14 +1519,14 @@ static bool ubifs_release_folio(struct folio *folio, gfp_t unused_gfp_flags)
  */
 static vm_fault_t ubifs_vm_page_mkwrite(struct vm_fault *vmf)
 {
-	struct page *page = vmf->page;
+	struct folio *folio = page_folio(vmf->page);
 	struct inode *inode = file_inode(vmf->vma->vm_file);
 	struct ubifs_info *c = inode->i_sb->s_fs_info;
 	struct timespec64 now = current_time(inode);
 	struct ubifs_budget_req req = { .new_page = 1 };
 	int err, update_time;
 
-	dbg_gen("ino %lu, pg %lu, i_size %lld",	inode->i_ino, page->index,
+	dbg_gen("ino %lu, pg %lu, i_size %lld",	inode->i_ino, folio->index,
 		i_size_read(inode));
 	ubifs_assert(c, !c->ro_media && !c->ro_mount);
 
@@ -1534,17 +1534,17 @@ static vm_fault_t ubifs_vm_page_mkwrite(struct vm_fault *vmf)
 		return VM_FAULT_SIGBUS; /* -EROFS */
 
 	/*
-	 * We have not locked @page so far so we may budget for changing the
-	 * page. Note, we cannot do this after we locked the page, because
+	 * We have not locked @folio so far so we may budget for changing the
+	 * folio. Note, we cannot do this after we locked the folio, because
 	 * budgeting may cause write-back which would cause deadlock.
 	 *
-	 * At the moment we do not know whether the page is dirty or not, so we
-	 * assume that it is not and budget for a new page. We could look at
+	 * At the moment we do not know whether the folio is dirty or not, so we
+	 * assume that it is not and budget for a new folio. We could look at
 	 * the @PG_private flag and figure this out, but we may race with write
-	 * back and the page state may change by the time we lock it, so this
+	 * back and the folio state may change by the time we lock it, so this
 	 * would need additional care. We do not bother with this at the
 	 * moment, although it might be good idea to do. Instead, we allocate
-	 * budget for a new page and amend it later on if the page was in fact
+	 * budget for a new folio and amend it later on if the folio was in fact
 	 * dirty.
 	 *
 	 * The budgeting-related logic of this function is similar to what we
@@ -1567,21 +1567,21 @@ static vm_fault_t ubifs_vm_page_mkwrite(struct vm_fault *vmf)
 		return VM_FAULT_SIGBUS;
 	}
 
-	lock_page(page);
-	if (unlikely(page->mapping != inode->i_mapping ||
-		     page_offset(page) > i_size_read(inode))) {
-		/* Page got truncated out from underneath us */
+	folio_lock(folio);
+	if (unlikely(folio->mapping != inode->i_mapping ||
+		     folio_pos(folio) >= i_size_read(inode))) {
+		/* Folio got truncated out from underneath us */
 		goto sigbus;
 	}
 
-	if (PagePrivate(page))
+	if (folio->private)
 		release_new_page_budget(c);
 	else {
-		if (!PageChecked(page))
+		if (!folio_test_checked(folio))
 			ubifs_convert_page_budget(c);
-		attach_page_private(page, (void *)1);
+		folio_attach_private(folio, (void *)1);
 		atomic_long_inc(&c->dirty_pg_cnt);
-		__set_page_dirty_nobuffers(page);
+		filemap_dirty_folio(folio->mapping, folio);
 	}
 
 	if (update_time) {
@@ -1597,11 +1597,11 @@ static vm_fault_t ubifs_vm_page_mkwrite(struct vm_fault *vmf)
 			ubifs_release_dirty_inode_budget(c, ui);
 	}
 
-	wait_for_stable_page(page);
+	folio_wait_stable(folio);
 	return VM_FAULT_LOCKED;
 
 sigbus:
-	unlock_page(page);
+	folio_unlock(folio);
 	ubifs_release_budget(c, &req);
 	return VM_FAULT_SIGBUS;
 }
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 07/15] ubifs: Convert write_begin_slow() to use a folio

[Matthew Wilcox (Oracle)]

Update to new APIs, removing several calls to compound_head() and
including support for large folios.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 48 ++++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 9dfedd91b576..6302ce65e0b3 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -222,16 +222,16 @@ static int write_begin_slow(struct address_space *mapping,
 	pgoff_t index = pos >> PAGE_SHIFT;
 	struct ubifs_budget_req req = { .new_page = 1 };
 	int err, appending = !!(pos + len > inode->i_size);
-	struct page *page;
+	struct folio *folio;
 
 	dbg_gen("ino %lu, pos %llu, len %u, i_size %lld",
 		inode->i_ino, pos, len, inode->i_size);
 
 	/*
-	 * At the slow path we have to budget before locking the page, because
-	 * budgeting may force write-back, which would wait on locked pages and
-	 * deadlock if we had the page locked. At this point we do not know
-	 * anything about the page, so assume that this is a new page which is
+	 * At the slow path we have to budget before locking the folio, because
+	 * budgeting may force write-back, which would wait on locked folios and
+	 * deadlock if we had the folio locked. At this point we do not know
+	 * anything about the folio, so assume that this is a new folio which is
 	 * written to a hole. This corresponds to largest budget. Later the
 	 * budget will be amended if this is not true.
 	 */
@@ -243,45 +243,45 @@ static int write_begin_slow(struct address_space *mapping,
 	if (unlikely(err))
 		return err;
 
-	page = grab_cache_page_write_begin(mapping, index);
-	if (unlikely(!page)) {
+	folio = __filemap_get_folio(mapping, index, FGP_WRITEBEGIN,
+			mapping_gfp_mask(mapping));
+	if (IS_ERR(folio)) {
 		ubifs_release_budget(c, &req);
-		return -ENOMEM;
+		return PTR_ERR(folio);
 	}
 
-	if (!PageUptodate(page)) {
-		if (!(pos & ~PAGE_MASK) && len == PAGE_SIZE)
-			SetPageChecked(page);
+	if (!folio_test_uptodate(folio)) {
+		if (pos == folio_pos(folio) && len >= folio_size(folio))
+			folio_set_checked(folio);
 		else {
-			err = do_readpage(page);
+			err = do_readpage(&folio->page);
 			if (err) {
-				unlock_page(page);
-				put_page(page);
+				folio_unlock(folio);
+				folio_put(folio);
 				ubifs_release_budget(c, &req);
 				return err;
 			}
 		}
 
-		SetPageUptodate(page);
-		ClearPageError(page);
+		folio_mark_uptodate(folio);
 	}
 
-	if (PagePrivate(page))
+	if (folio->private)
 		/*
-		 * The page is dirty, which means it was budgeted twice:
+		 * The folio is dirty, which means it was budgeted twice:
 		 *   o first time the budget was allocated by the task which
-		 *     made the page dirty and set the PG_private flag;
+		 *     made the folio dirty and set the private field;
 		 *   o and then we budgeted for it for the second time at the
 		 *     very beginning of this function.
 		 *
-		 * So what we have to do is to release the page budget we
+		 * So what we have to do is to release the folio budget we
 		 * allocated.
 		 */
 		release_new_page_budget(c);
-	else if (!PageChecked(page))
+	else if (!folio_test_checked(folio))
 		/*
-		 * We are changing a page which already exists on the media.
-		 * This means that changing the page does not make the amount
+		 * We are changing a folio which already exists on the media.
+		 * This means that changing the folio does not make the amount
 		 * of indexing information larger, and this part of the budget
 		 * which we have already acquired may be released.
 		 */
@@ -304,7 +304,7 @@ static int write_begin_slow(struct address_space *mapping,
 			ubifs_release_dirty_inode_budget(c, ui);
 	}
 
-	*pagep = page;
+	*pagep = &folio->page;
 	return 0;
 }
 
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 08/15] ubifs: Convert ubifs_write_begin() to use a folio

[Matthew Wilcox (Oracle)]

Save eight calls to compound_head() by using the new folio API.
Remove a few assumptions that would break with large folios.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 6302ce65e0b3..ef262499f228 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -428,7 +428,7 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 	pgoff_t index = pos >> PAGE_SHIFT;
 	int err, appending = !!(pos + len > inode->i_size);
 	int skipped_read = 0;
-	struct page *page;
+	struct folio *folio;
 
 	ubifs_assert(c, ubifs_inode(inode)->ui_size == inode->i_size);
 	ubifs_assert(c, !c->ro_media && !c->ro_mount);
@@ -437,13 +437,14 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 		return -EROFS;
 
 	/* Try out the fast-path part first */
-	page = grab_cache_page_write_begin(mapping, index);
-	if (unlikely(!page))
-		return -ENOMEM;
+	folio = __filemap_get_folio(mapping, index, FGP_WRITEBEGIN,
+			mapping_gfp_mask(mapping));
+	if (IS_ERR(folio))
+		return PTR_ERR(folio);
 
-	if (!PageUptodate(page)) {
+	if (!folio_test_uptodate(folio)) {
 		/* The page is not loaded from the flash */
-		if (!(pos & ~PAGE_MASK) && len == PAGE_SIZE) {
+		if (pos == folio_pos(folio) && len >= folio_size(folio)) {
 			/*
 			 * We change whole page so no need to load it. But we
 			 * do not know whether this page exists on the media or
@@ -453,29 +454,27 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 			 * media. Thus, we are setting the @PG_checked flag
 			 * here.
 			 */
-			SetPageChecked(page);
+			folio_set_checked(folio);
 			skipped_read = 1;
 		} else {
-			err = do_readpage(page);
+			err = do_readpage(&folio->page);
 			if (err) {
-				unlock_page(page);
-				put_page(page);
+				folio_unlock(folio);
+				folio_put(folio);
 				return err;
 			}
 		}
 	}
 
-	err = allocate_budget(c, page, ui, appending);
+	err = allocate_budget(c, &folio->page, ui, appending);
 	if (unlikely(err)) {
 		ubifs_assert(c, err == -ENOSPC);
 		/*
 		 * If we skipped reading the page because we were going to
 		 * write all of it, then it is not up to date.
 		 */
-		if (skipped_read) {
-			ClearPageChecked(page);
-			ClearPageUptodate(page);
-		}
+		if (skipped_read)
+			folio_clear_checked(folio);
 		/*
 		 * Budgeting failed which means it would have to force
 		 * write-back but didn't, because we set the @fast flag in the
@@ -487,8 +486,8 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 			ubifs_assert(c, mutex_is_locked(&ui->ui_mutex));
 			mutex_unlock(&ui->ui_mutex);
 		}
-		unlock_page(page);
-		put_page(page);
+		folio_unlock(folio);
+		folio_put(folio);
 
 		return write_begin_slow(mapping, pos, len, pagep);
 	}
@@ -499,9 +498,8 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 	 * with @ui->ui_mutex locked if we are appending pages, and unlocked
 	 * otherwise. This is an optimization (slightly hacky though).
 	 */
-	*pagep = page;
+	*pagep = &folio->page;
 	return 0;
-
 }
 
 /**
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 09/15] ubifs: Convert ubifs_write_end() to use a folio

[Matthew Wilcox (Oracle)]

Convert the incoming page pointer to a folio and use it throughout,
saving several calls to compound_head().  Also remove some PAGE_SIZE
assumptions.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index ef262499f228..52027b4feebf 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -532,6 +532,7 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
 			   loff_t pos, unsigned len, unsigned copied,
 			   struct page *page, void *fsdata)
 {
+	struct folio *folio = page_folio(page);
 	struct inode *inode = mapping->host;
 	struct ubifs_inode *ui = ubifs_inode(inode);
 	struct ubifs_info *c = inode->i_sb->s_fs_info;
@@ -539,47 +540,47 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
 	int appending = !!(end_pos > inode->i_size);
 
 	dbg_gen("ino %lu, pos %llu, pg %lu, len %u, copied %d, i_size %lld",
-		inode->i_ino, pos, page->index, len, copied, inode->i_size);
+		inode->i_ino, pos, folio->index, len, copied, inode->i_size);
 
-	if (unlikely(copied < len && len == PAGE_SIZE)) {
+	if (unlikely(copied < len && !folio_test_uptodate(folio))) {
 		/*
-		 * VFS copied less data to the page that it intended and
+		 * VFS copied less data to the folio than it intended and
 		 * declared in its '->write_begin()' call via the @len
-		 * argument. If the page was not up-to-date, and @len was
-		 * @PAGE_SIZE, the 'ubifs_write_begin()' function did
+		 * argument. If the folio was not up-to-date,
+		 * the 'ubifs_write_begin()' function did
 		 * not load it from the media (for optimization reasons). This
-		 * means that part of the page contains garbage. So read the
-		 * page now.
+		 * means that part of the folio contains garbage. So read the
+		 * folio now.
 		 */
 		dbg_gen("copied %d instead of %d, read page and repeat",
 			copied, len);
-		cancel_budget(c, page, ui, appending);
-		ClearPageChecked(page);
+		cancel_budget(c, &folio->page, ui, appending);
+		folio_clear_checked(folio);
 
 		/*
 		 * Return 0 to force VFS to repeat the whole operation, or the
 		 * error code if 'do_readpage()' fails.
 		 */
-		copied = do_readpage(page);
+		copied = do_readpage(&folio->page);
 		goto out;
 	}
 
-	if (len == PAGE_SIZE)
-		SetPageUptodate(page);
+	if (len >= folio_size(folio))
+		folio_mark_uptodate(folio);
 
-	if (!PagePrivate(page)) {
-		attach_page_private(page, (void *)1);
+	if (!folio->private) {
+		folio_attach_private(folio, (void *)1);
 		atomic_long_inc(&c->dirty_pg_cnt);
-		__set_page_dirty_nobuffers(page);
+		filemap_dirty_folio(mapping, folio);
 	}
 
 	if (appending) {
 		i_size_write(inode, end_pos);
 		ui->ui_size = end_pos;
 		/*
-		 * Note, we do not set @I_DIRTY_PAGES (which means that the
-		 * inode has dirty pages), this has been done in
-		 * '__set_page_dirty_nobuffers()'.
+		 * We do not set @I_DIRTY_PAGES (which means that
+		 * the inode has dirty pages), this was done in
+		 * filemap_dirty_folio().
 		 */
 		__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
 		ubifs_assert(c, mutex_is_locked(&ui->ui_mutex));
@@ -587,8 +588,8 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
 	}
 
 out:
-	unlock_page(page);
-	put_page(page);
+	folio_unlock(folio);
+	folio_put(folio);
 	return copied;
 }
 
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 10/15] ubifs: Convert do_readpage() to take a folio

[Matthew Wilcox (Oracle)]

All the callers now have a folio, so pass it in, and convert do_readpage()
to us folios directly.  Includes unifying the exit paths from this
function and using kmap_local instead of plain kmap.  This function
should now work with large folios, but this is not tested.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 64 +++++++++++++++++++++++--------------------------
 1 file changed, 30 insertions(+), 34 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 52027b4feebf..cc450afdcc47 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -96,36 +96,36 @@ static int read_block(struct inode *inode, void *addr, unsigned int block,
 	return -EINVAL;
 }
 
-static int do_readpage(struct page *page)
+static int do_readpage(struct folio *folio)
 {
 	void *addr;
 	int err = 0, i;
 	unsigned int block, beyond;
-	struct ubifs_data_node *dn;
-	struct inode *inode = page->mapping->host;
+	struct ubifs_data_node *dn = NULL;
+	struct inode *inode = folio->mapping->host;
 	struct ubifs_info *c = inode->i_sb->s_fs_info;
 	loff_t i_size = i_size_read(inode);
 
 	dbg_gen("ino %lu, pg %lu, i_size %lld, flags %#lx",
-		inode->i_ino, page->index, i_size, page->flags);
-	ubifs_assert(c, !PageChecked(page));
-	ubifs_assert(c, !PagePrivate(page));
+		inode->i_ino, folio->index, i_size, folio->flags);
+	ubifs_assert(c, !folio_test_checked(folio));
+	ubifs_assert(c, !folio->private);
 
-	addr = kmap(page);
+	addr = kmap_local_folio(folio, 0);
 
-	block = page->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
+	block = folio->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
 	beyond = (i_size + UBIFS_BLOCK_SIZE - 1) >> UBIFS_BLOCK_SHIFT;
 	if (block >= beyond) {
 		/* Reading beyond inode */
-		SetPageChecked(page);
-		memset(addr, 0, PAGE_SIZE);
+		folio_set_checked(folio);
+		addr = folio_zero_tail(folio, 0, addr);
 		goto out;
 	}
 
 	dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS);
 	if (!dn) {
 		err = -ENOMEM;
-		goto error;
+		goto out;
 	}
 
 	i = 0;
@@ -150,39 +150,35 @@ static int do_readpage(struct page *page)
 					memset(addr + ilen, 0, dlen - ilen);
 			}
 		}
-		if (++i >= UBIFS_BLOCKS_PER_PAGE)
+		if (++i >= (UBIFS_BLOCKS_PER_PAGE << folio_order(folio)))
 			break;
 		block += 1;
 		addr += UBIFS_BLOCK_SIZE;
+		if (folio_test_highmem(folio) && (offset_in_page(addr) == 0)) {
+			kunmap_local(addr - UBIFS_BLOCK_SIZE);
+			addr = kmap_local_folio(folio, i * UBIFS_BLOCK_SIZE);
+		}
 	}
+
 	if (err) {
 		struct ubifs_info *c = inode->i_sb->s_fs_info;
 		if (err == -ENOENT) {
 			/* Not found, so it must be a hole */
-			SetPageChecked(page);
+			folio_set_checked(folio);
 			dbg_gen("hole");
-			goto out_free;
+			err = 0;
+		} else {
+			ubifs_err(c, "cannot read page %lu of inode %lu, error %d",
+				  folio->index, inode->i_ino, err);
 		}
-		ubifs_err(c, "cannot read page %lu of inode %lu, error %d",
-			  page->index, inode->i_ino, err);
-		goto error;
 	}
 
-out_free:
-	kfree(dn);
 out:
-	SetPageUptodate(page);
-	ClearPageError(page);
-	flush_dcache_page(page);
-	kunmap(page);
-	return 0;
-
-error:
 	kfree(dn);
-	ClearPageUptodate(page);
-	SetPageError(page);
-	flush_dcache_page(page);
-	kunmap(page);
+	if (!err)
+		folio_mark_uptodate(folio);
+	flush_dcache_folio(folio);
+	kunmap_local(addr);
 	return err;
 }
 
@@ -254,7 +250,7 @@ static int write_begin_slow(struct address_space *mapping,
 		if (pos == folio_pos(folio) && len >= folio_size(folio))
 			folio_set_checked(folio);
 		else {
-			err = do_readpage(&folio->page);
+			err = do_readpage(folio);
 			if (err) {
 				folio_unlock(folio);
 				folio_put(folio);
@@ -457,7 +453,7 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 			folio_set_checked(folio);
 			skipped_read = 1;
 		} else {
-			err = do_readpage(&folio->page);
+			err = do_readpage(folio);
 			if (err) {
 				folio_unlock(folio);
 				folio_put(folio);
@@ -561,7 +557,7 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
 		 * Return 0 to force VFS to repeat the whole operation, or the
 		 * error code if 'do_readpage()' fails.
 		 */
-		copied = do_readpage(&folio->page);
+		copied = do_readpage(folio);
 		goto out;
 	}
 
@@ -897,7 +893,7 @@ static int ubifs_read_folio(struct file *file, struct folio *folio)
 
 	if (ubifs_bulk_read(page))
 		return 0;
-	do_readpage(page);
+	do_readpage(folio);
 	folio_unlock(folio);
 	return 0;
 }
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 11/15] ubifs: Convert allocate_budget() to work on a folio

[Matthew Wilcox (Oracle)]

The one caller has a folio, so pass it in instead of the page.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index cc450afdcc47..6bc7965ce918 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -307,7 +307,7 @@ static int write_begin_slow(struct address_space *mapping,
 /**
  * allocate_budget - allocate budget for 'ubifs_write_begin()'.
  * @c: UBIFS file-system description object
- * @page: page to allocate budget for
+ * @folio: folio to allocate budget for
  * @ui: UBIFS inode object the page belongs to
  * @appending: non-zero if the page is appended
  *
@@ -318,15 +318,15 @@ static int write_begin_slow(struct address_space *mapping,
  *
  * Returns: %0 in case of success and %-ENOSPC in case of failure.
  */
-static int allocate_budget(struct ubifs_info *c, struct page *page,
+static int allocate_budget(struct ubifs_info *c, struct folio *folio,
 			   struct ubifs_inode *ui, int appending)
 {
 	struct ubifs_budget_req req = { .fast = 1 };
 
-	if (PagePrivate(page)) {
+	if (folio->private) {
 		if (!appending)
 			/*
-			 * The page is dirty and we are not appending, which
+			 * The folio is dirty and we are not appending, which
 			 * means no budget is needed at all.
 			 */
 			return 0;
@@ -350,11 +350,11 @@ static int allocate_budget(struct ubifs_info *c, struct page *page,
 		 */
 		req.dirtied_ino = 1;
 	} else {
-		if (PageChecked(page))
+		if (folio_test_checked(folio))
 			/*
 			 * The page corresponds to a hole and does not
 			 * exist on the media. So changing it makes
-			 * make the amount of indexing information
+			 * the amount of indexing information
 			 * larger, and we have to budget for a new
 			 * page.
 			 */
@@ -462,7 +462,7 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 		}
 	}
 
-	err = allocate_budget(c, &folio->page, ui, appending);
+	err = allocate_budget(c, folio, ui, appending);
 	if (unlikely(err)) {
 		ubifs_assert(c, err == -ENOSPC);
 		/*
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 12/15] ubifs: Convert cancel_budget() to take a folio

[Matthew Wilcox (Oracle)]

The one caller already has a folio, so pass it in instead of the page.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 6bc7965ce918..f34371436a84 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -501,14 +501,14 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
 /**
  * cancel_budget - cancel budget.
  * @c: UBIFS file-system description object
- * @page: page to cancel budget for
+ * @folio: folio to cancel budget for
  * @ui: UBIFS inode object the page belongs to
  * @appending: non-zero if the page is appended
  *
  * This is a helper function for a page write operation. It unlocks the
  * @ui->ui_mutex in case of appending.
  */
-static void cancel_budget(struct ubifs_info *c, struct page *page,
+static void cancel_budget(struct ubifs_info *c, struct folio *folio,
 			  struct ubifs_inode *ui, int appending)
 {
 	if (appending) {
@@ -516,8 +516,8 @@ static void cancel_budget(struct ubifs_info *c, struct page *page,
 			ubifs_release_dirty_inode_budget(c, ui);
 		mutex_unlock(&ui->ui_mutex);
 	}
-	if (!PagePrivate(page)) {
-		if (PageChecked(page))
+	if (!folio->private) {
+		if (folio_test_checked(folio))
 			release_new_page_budget(c);
 		else
 			release_existing_page_budget(c);
@@ -550,7 +550,7 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
 		 */
 		dbg_gen("copied %d instead of %d, read page and repeat",
 			copied, len);
-		cancel_budget(c, &folio->page, ui, appending);
+		cancel_budget(c, folio, ui, appending);
 		folio_clear_checked(folio);
 
 		/*
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 13/15] ubifs: Pass a folio into ubifs_bulk_read() and ubifs_do_bulk_read()

[Matthew Wilcox (Oracle)]

This saves a single call to compound_head().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index f34371436a84..8cad1fe9c50f 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -705,15 +705,15 @@ static int populate_page(struct ubifs_info *c, struct page *page,
  * ubifs_do_bulk_read - do bulk-read.
  * @c: UBIFS file-system description object
  * @bu: bulk-read information
- * @page1: first page to read
+ * @folio1: first folio to read
  *
  * Returns: %1 if the bulk-read is done, otherwise %0 is returned.
  */
 static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
-			      struct page *page1)
+			      struct folio *folio1)
 {
-	pgoff_t offset = page1->index, end_index;
-	struct address_space *mapping = page1->mapping;
+	pgoff_t offset = folio1->index, end_index;
+	struct address_space *mapping = folio1->mapping;
 	struct inode *inode = mapping->host;
 	struct ubifs_inode *ui = ubifs_inode(inode);
 	int err, page_idx, page_cnt, ret = 0, n = 0;
@@ -763,11 +763,11 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
 			goto out_warn;
 	}
 
-	err = populate_page(c, page1, bu, &n);
+	err = populate_page(c, &folio1->page, bu, &n);
 	if (err)
 		goto out_warn;
 
-	unlock_page(page1);
+	folio_unlock(folio1);
 	ret = 1;
 
 	isize = i_size_read(inode);
@@ -812,7 +812,7 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
 
 /**
  * ubifs_bulk_read - determine whether to bulk-read and, if so, do it.
- * @page: page from which to start bulk-read.
+ * @folio: folio from which to start bulk-read.
  *
  * Some flash media are capable of reading sequentially at faster rates. UBIFS
  * bulk-read facility is designed to take advantage of that, by reading in one
@@ -821,12 +821,12 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
  *
  * Returns: %1 if a bulk-read is done and %0 otherwise.
  */
-static int ubifs_bulk_read(struct page *page)
+static int ubifs_bulk_read(struct folio *folio)
 {
-	struct inode *inode = page->mapping->host;
+	struct inode *inode = folio->mapping->host;
 	struct ubifs_info *c = inode->i_sb->s_fs_info;
 	struct ubifs_inode *ui = ubifs_inode(inode);
-	pgoff_t index = page->index, last_page_read = ui->last_page_read;
+	pgoff_t index = folio->index, last_page_read = ui->last_page_read;
 	struct bu_info *bu;
 	int err = 0, allocated = 0;
 
@@ -874,8 +874,8 @@ static int ubifs_bulk_read(struct page *page)
 
 	bu->buf_len = c->max_bu_buf_len;
 	data_key_init(c, &bu->key, inode->i_ino,
-		      page->index << UBIFS_BLOCKS_PER_PAGE_SHIFT);
-	err = ubifs_do_bulk_read(c, bu, page);
+		      folio->index << UBIFS_BLOCKS_PER_PAGE_SHIFT);
+	err = ubifs_do_bulk_read(c, bu, folio);
 
 	if (!allocated)
 		mutex_unlock(&c->bu_mutex);
@@ -889,9 +889,7 @@ static int ubifs_bulk_read(struct page *page)
 
 static int ubifs_read_folio(struct file *file, struct folio *folio)
 {
-	struct page *page = &folio->page;
-
-	if (ubifs_bulk_read(page))
+	if (ubifs_bulk_read(folio))
 		return 0;
 	do_readpage(folio);
 	folio_unlock(folio);
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 14/15] ubifs: Use a folio in ubifs_do_bulk_read()

[Matthew Wilcox (Oracle)]

When looking in the page cache, retrieve a folio instead of a page.
This would need some work to make it safe for large folios.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 8cad1fe9c50f..dec5258b4f38 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -777,19 +777,19 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
 
 	for (page_idx = 1; page_idx < page_cnt; page_idx++) {
 		pgoff_t page_offset = offset + page_idx;
-		struct page *page;
+		struct folio *folio;
 
 		if (page_offset > end_index)
 			break;
-		page = pagecache_get_page(mapping, page_offset,
+		folio = __filemap_get_folio(mapping, page_offset,
 				 FGP_LOCK|FGP_ACCESSED|FGP_CREAT|FGP_NOWAIT,
 				 ra_gfp_mask);
-		if (!page)
+		if (IS_ERR(folio))
 			break;
-		if (!PageUptodate(page))
-			err = populate_page(c, page, bu, &n);
-		unlock_page(page);
-		put_page(page);
+		if (!folio_test_uptodate(folio))
+			err = populate_page(c, &folio->page, bu, &n);
+		folio_unlock(folio);
+		folio_put(folio);
 		if (err)
 			break;
 	}
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 15/15] ubifs: Convert populate_page() to take a folio

[Matthew Wilcox (Oracle)]

Both callers now have a folio, so pass it in.  This function contains
several assumptions that folios are not large.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ubifs/file.c | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index dec5258b4f38..fbbd07390959 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -592,35 +592,35 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
 /**
  * populate_page - copy data nodes into a page for bulk-read.
  * @c: UBIFS file-system description object
- * @page: page
+ * @folio: folio
  * @bu: bulk-read information
  * @n: next zbranch slot
  *
  * Returns: %0 on success and a negative error code on failure.
  */
-static int populate_page(struct ubifs_info *c, struct page *page,
+static int populate_page(struct ubifs_info *c, struct folio *folio,
 			 struct bu_info *bu, int *n)
 {
 	int i = 0, nn = *n, offs = bu->zbranch[0].offs, hole = 0, read = 0;
-	struct inode *inode = page->mapping->host;
+	struct inode *inode = folio->mapping->host;
 	loff_t i_size = i_size_read(inode);
 	unsigned int page_block;
 	void *addr, *zaddr;
 	pgoff_t end_index;
 
 	dbg_gen("ino %lu, pg %lu, i_size %lld, flags %#lx",
-		inode->i_ino, page->index, i_size, page->flags);
+		inode->i_ino, folio->index, i_size, folio->flags);
 
-	addr = zaddr = kmap(page);
+	addr = zaddr = kmap_local_folio(folio, 0);
 
 	end_index = (i_size - 1) >> PAGE_SHIFT;
-	if (!i_size || page->index > end_index) {
+	if (!i_size || folio->index > end_index) {
 		hole = 1;
-		memset(addr, 0, PAGE_SIZE);
+		addr = folio_zero_tail(folio, 0, addr);
 		goto out_hole;
 	}
 
-	page_block = page->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
+	page_block = folio->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
 	while (1) {
 		int err, len, out_len, dlen;
 
@@ -669,9 +669,13 @@ static int populate_page(struct ubifs_info *c, struct page *page,
 			break;
 		addr += UBIFS_BLOCK_SIZE;
 		page_block += 1;
+		if (folio_test_highmem(folio) && (offset_in_page(addr) == 0)) {
+			kunmap_local(addr - UBIFS_BLOCK_SIZE);
+			addr = kmap_local_folio(folio, i * UBIFS_BLOCK_SIZE);
+		}
 	}
 
-	if (end_index == page->index) {
+	if (end_index == folio->index) {
 		int len = i_size & (PAGE_SIZE - 1);
 
 		if (len && len < read)
@@ -680,22 +684,19 @@ static int populate_page(struct ubifs_info *c, struct page *page,
 
 out_hole:
 	if (hole) {
-		SetPageChecked(page);
+		folio_set_checked(folio);
 		dbg_gen("hole");
 	}
 
-	SetPageUptodate(page);
-	ClearPageError(page);
-	flush_dcache_page(page);
-	kunmap(page);
+	folio_mark_uptodate(folio);
+	flush_dcache_folio(folio);
+	kunmap_local(addr);
 	*n = nn;
 	return 0;
 
 out_err:
-	ClearPageUptodate(page);
-	SetPageError(page);
-	flush_dcache_page(page);
-	kunmap(page);
+	flush_dcache_folio(folio);
+	kunmap_local(addr);
 	ubifs_err(c, "bad data node (block %u, inode %lu)",
 		  page_block, inode->i_ino);
 	return -EINVAL;
@@ -763,7 +764,7 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
 			goto out_warn;
 	}
 
-	err = populate_page(c, &folio1->page, bu, &n);
+	err = populate_page(c, folio1, bu, &n);
 	if (err)
 		goto out_warn;
 
@@ -787,7 +788,7 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
 		if (IS_ERR(folio))
 			break;
 		if (!folio_test_uptodate(folio))
-			err = populate_page(c, &folio->page, bu, &n);
+			err = populate_page(c, folio, bu, &n);
 		folio_unlock(folio);
 		folio_put(folio);
 		if (err)
-- 
2.43.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 01/15] ubifs: Set page uptodate in the correct place

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> Page cache reads are lockless, so setting the freshly allocated page
> uptodate before we've overwritten it with the data it's supposed to have
> in it will allow a simultaneous reader to see old data.  Move the call
> to SetPageUptodate into ubifs_write_end(), which is after we copied the
> new data into the page.
> 
> Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
> Cc: stable@vger.kernel.org
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 5029eb3390a5..40a9b03ef821 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -463,9 +463,6 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   				return err;
>   			}
>   		}
> -
> -		SetPageUptodate(page);
> -		ClearPageError(page);
>   	}

This solution looks good to me, and I think 'SetPageUptodate' should be 
removed from write_begin_slow(slow path) too.

>   
>   	err = allocate_budget(c, page, ui, appending);
> @@ -569,6 +566,9 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
>   		goto out;
>   	}
>   
> +	if (len == PAGE_SIZE)
> +		SetPageUptodate(page);
> +
>   	if (!PagePrivate(page)) {
>   		attach_page_private(page, (void *)1);
>   		atomic_long_inc(&c->dirty_pg_cnt);
> 

Re: [PATCH 01/15] ubifs: Set page uptodate in the correct place

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> Page cache reads are lockless, so setting the freshly allocated page
> uptodate before we've overwritten it with the data it's supposed to have
> in it will allow a simultaneous reader to see old data.  Move the call
> to SetPageUptodate into ubifs_write_end(), which is after we copied the
> new data into the page.
> 
> Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
> Cc: stable@vger.kernel.org
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 5029eb3390a5..40a9b03ef821 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -463,9 +463,6 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   				return err;
>   			}
>   		}
> -
> -		SetPageUptodate(page);
> -		ClearPageError(page);
>   	}

This solution looks good to me, and I think 'SetPageUptodate' should be 
removed from write_begin_slow(slow path) too.

>   
>   	err = allocate_budget(c, page, ui, appending);
> @@ -569,6 +566,9 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
>   		goto out;
>   	}
>   
> +	if (len == PAGE_SIZE)
> +		SetPageUptodate(page);
> +
>   	if (!PagePrivate(page)) {
>   		attach_page_private(page, (void *)1);
>   		atomic_long_inc(&c->dirty_pg_cnt);
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 03/15] ubifs: Convert ubifs_writepage to use a folio

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> We still pass the page down to do_writepage(), but ubifs_writepage()
> itself is now large folio safe.  It also contains far fewer hidden calls
> to compound_head().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 39 +++++++++++++++++----------------------
>   1 file changed, 17 insertions(+), 22 deletions(-)
> 

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 007a73052561..7d0967daded3 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -1009,21 +1009,18 @@ static int do_writepage(struct page *page, int len)
>   static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
>   		void *data)
>   {
> -	struct page *page = &folio->page;
> -	struct inode *inode = page->mapping->host;
> +	struct inode *inode = folio->mapping->host;
>   	struct ubifs_info *c = inode->i_sb->s_fs_info;
>   	struct ubifs_inode *ui = ubifs_inode(inode);
>   	loff_t i_size =  i_size_read(inode), synced_i_size;
> -	pgoff_t end_index = i_size >> PAGE_SHIFT;
> -	int err, len = i_size & (PAGE_SIZE - 1);
> -	void *kaddr;
> +	int err, len = folio_size(folio);
>   
>   	dbg_gen("ino %lu, pg %lu, pg flags %#lx",
> -		inode->i_ino, page->index, page->flags);
> -	ubifs_assert(c, PagePrivate(page));
> +		inode->i_ino, folio->index, folio->flags);
> +	ubifs_assert(c, folio->private != NULL);
>   
> -	/* Is the page fully outside @i_size? (truncate in progress) */
> -	if (page->index > end_index || (page->index == end_index && !len)) {
> +	/* Is the folio fully outside @i_size? (truncate in progress) */
> +	if (folio_pos(folio) >= i_size) {
>   		err = 0;
>   		goto out_unlock;
>   	}
> @@ -1032,9 +1029,9 @@ static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
>   	synced_i_size = ui->synced_i_size;
>   	spin_unlock(&ui->ui_lock);
>   
> -	/* Is the page fully inside @i_size? */
> -	if (page->index < end_index) {
> -		if (page->index >= synced_i_size >> PAGE_SHIFT) {
> +	/* Is the folio fully inside i_size? */
> +	if (folio_pos(folio) + len <= i_size) {
> +		if (folio_pos(folio) >= synced_i_size) {
>   			err = inode->i_sb->s_op->write_inode(inode, NULL);
>   			if (err)
>   				goto out_redirty;
> @@ -1047,20 +1044,18 @@ static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
>   			 * with this.
>   			 */
>   		}
> -		return do_writepage(page, PAGE_SIZE);
> +		return do_writepage(&folio->page, len);
>   	}
>   
>   	/*
> -	 * The page straddles @i_size. It must be zeroed out on each and every
> +	 * The folio straddles @i_size. It must be zeroed out on each and every
>   	 * writepage invocation because it may be mmapped. "A file is mapped
>   	 * in multiples of the page size. For a file that is not a multiple of
>   	 * the page size, the remaining memory is zeroed when mapped, and
>   	 * writes to that region are not written out to the file."
>   	 */
> -	kaddr = kmap_atomic(page);
> -	memset(kaddr + len, 0, PAGE_SIZE - len);
> -	flush_dcache_page(page);
> -	kunmap_atomic(kaddr);
> +	len = i_size - folio_pos(folio);
> +	folio_zero_segment(folio, len, folio_size(folio));
>   
>   	if (i_size > synced_i_size) {
>   		err = inode->i_sb->s_op->write_inode(inode, NULL);
> @@ -1068,16 +1063,16 @@ static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
>   			goto out_redirty;
>   	}
>   
> -	return do_writepage(page, len);
> +	return do_writepage(&folio->page, len);
>   out_redirty:
>   	/*
> -	 * redirty_page_for_writepage() won't call ubifs_dirty_inode() because
> +	 * folio_redirty_for_writepage() won't call ubifs_dirty_inode() because
>   	 * it passes I_DIRTY_PAGES flag while calling __mark_inode_dirty(), so
>   	 * there is no need to do space budget for dirty inode.
>   	 */
> -	redirty_page_for_writepage(wbc, page);
> +	folio_redirty_for_writepage(wbc, folio);
>   out_unlock:
> -	unlock_page(page);
> +	folio_unlock(folio);
>   	return err;
>   }
>   
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 04/15] ubifs: Use a folio in do_truncation()

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> Convert from the old page APIs to the new folio APIs which saves
> a few hidden calls to compound_head().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
> 

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 7d0967daded3..b297582c9b75 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -1158,11 +1158,11 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
>   
>   	if (offset) {
>   		pgoff_t index = new_size >> PAGE_SHIFT;
> -		struct page *page;
> +		struct folio *folio;
>   
> -		page = find_lock_page(inode->i_mapping, index);
> -		if (page) {
> -			if (PageDirty(page)) {
> +		folio = filemap_lock_folio(inode->i_mapping, index);
> +		if (!IS_ERR(folio)) {
> +			if (folio_test_dirty(folio)) {
>   				/*
>   				 * 'ubifs_jnl_truncate()' will try to truncate
>   				 * the last data node, but it contains
> @@ -1171,14 +1171,14 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
>   				 * 'ubifs_jnl_truncate()' will see an already
>   				 * truncated (and up to date) data node.
>   				 */
> -				ubifs_assert(c, PagePrivate(page));
> +				ubifs_assert(c, folio->private != NULL);
>   
> -				clear_page_dirty_for_io(page);
> +				folio_clear_dirty_for_io(folio);
>   				if (UBIFS_BLOCKS_PER_PAGE_SHIFT)
> -					offset = new_size &
> -						 (PAGE_SIZE - 1);
> -				err = do_writepage(page, offset);
> -				put_page(page);
> +					offset = offset_in_folio(folio,
> +							new_size);
> +				err = do_writepage(&folio->page, offset);
> +				folio_put(folio);
>   				if (err)
>   					goto out_budg;
>   				/*
> @@ -1191,8 +1191,8 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
>   				 * to 'ubifs_jnl_truncate()' to save it from
>   				 * having to read it.
>   				 */
> -				unlock_page(page);
> -				put_page(page);
> +				folio_unlock(folio);
> +				folio_put(folio);
>   			}
>   		}
>   	}
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 05/15] ubifs: Convert do_writepage() to take a folio

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> Replace the call to SetPageError() with a call to mapping_set_error().
> Support large folios by using kmap_local_folio() and remapping each time
> we cross a page boundary.  Saves a lot of hidden calls to compound_head().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 56 ++++++++++++++++++++++++++-----------------------
>   1 file changed, 30 insertions(+), 26 deletions(-)
> 

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index b297582c9b75..e755d0a11146 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -903,60 +903,64 @@ static int ubifs_read_folio(struct file *file, struct folio *folio)
>   	return 0;
>   }
>   
> -static int do_writepage(struct page *page, int len)
> +static int do_writepage(struct folio *folio, size_t len)
>   {
> -	int err = 0, i, blen;
> +	int err = 0, blen;
>   	unsigned int block;
>   	void *addr;
> +	size_t offset = 0;
>   	union ubifs_key key;
> -	struct inode *inode = page->mapping->host;
> +	struct inode *inode = folio->mapping->host;
>   	struct ubifs_info *c = inode->i_sb->s_fs_info;
>   
>   #ifdef UBIFS_DEBUG
>   	struct ubifs_inode *ui = ubifs_inode(inode);
>   	spin_lock(&ui->ui_lock);
> -	ubifs_assert(c, page->index <= ui->synced_i_size >> PAGE_SHIFT);
> +	ubifs_assert(c, folio->index <= ui->synced_i_size >> PAGE_SHIFT);
>   	spin_unlock(&ui->ui_lock);
>   #endif
>   
> -	/* Update radix tree tags */
> -	set_page_writeback(page);
> +	folio_start_writeback(folio);
>   
> -	addr = kmap(page);
> -	block = page->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
> -	i = 0;
> -	while (len) {
> -		blen = min_t(int, len, UBIFS_BLOCK_SIZE);
> +	addr = kmap_local_folio(folio, offset);
> +	block = folio->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
> +	for (;;) {
> +		blen = min_t(size_t, len, UBIFS_BLOCK_SIZE);
>   		data_key_init(c, &key, inode->i_ino, block);
>   		err = ubifs_jnl_write_data(c, inode, &key, addr, blen);
>   		if (err)
>   			break;
> -		if (++i >= UBIFS_BLOCKS_PER_PAGE)
> +		len -= blen;
> +		if (!len)
>   			break;
>   		block += 1;
>   		addr += blen;
> -		len -= blen;
> +		if (folio_test_highmem(folio) && !offset_in_page(addr)) {
> +			kunmap_local(addr - blen);
> +			offset += PAGE_SIZE;
> +			addr = kmap_local_folio(folio, offset);
> +		}
>   	}
> +	kunmap_local(addr);
>   	if (err) {
> -		SetPageError(page);
> -		ubifs_err(c, "cannot write page %lu of inode %lu, error %d",
> -			  page->index, inode->i_ino, err);
> +		mapping_set_error(folio->mapping, err);
> +		ubifs_err(c, "cannot write folio %lu of inode %lu, error %d",
> +			  folio->index, inode->i_ino, err);
>   		ubifs_ro_mode(c, err);
>   	}
>   
> -	ubifs_assert(c, PagePrivate(page));
> -	if (PageChecked(page))
> +	ubifs_assert(c, folio->private != NULL);
> +	if (folio_test_checked(folio))
>   		release_new_page_budget(c);
>   	else
>   		release_existing_page_budget(c);
>   
>   	atomic_long_dec(&c->dirty_pg_cnt);
> -	detach_page_private(page);
> -	ClearPageChecked(page);
> +	folio_detach_private(folio);
> +	folio_clear_checked(folio);
>   
> -	kunmap(page);
> -	unlock_page(page);
> -	end_page_writeback(page);
> +	folio_unlock(folio);
> +	folio_end_writeback(folio);
>   	return err;
>   }
>   
> @@ -1044,7 +1048,7 @@ static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
>   			 * with this.
>   			 */
>   		}
> -		return do_writepage(&folio->page, len);
> +		return do_writepage(folio, len);
>   	}
>   
>   	/*
> @@ -1063,7 +1067,7 @@ static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
>   			goto out_redirty;
>   	}
>   
> -	return do_writepage(&folio->page, len);
> +	return do_writepage(folio, len);
>   out_redirty:
>   	/*
>   	 * folio_redirty_for_writepage() won't call ubifs_dirty_inode() because
> @@ -1177,7 +1181,7 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
>   				if (UBIFS_BLOCKS_PER_PAGE_SHIFT)
>   					offset = offset_in_folio(folio,
>   							new_size);
> -				err = do_writepage(&folio->page, offset);
> +				err = do_writepage(folio, offset);
>   				folio_put(folio);
>   				if (err)
>   					goto out_budg;
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 02/15] ubifs: Convert from writepage to writepages

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> This is a simplistic conversion to separate out any effects of
> no longer having a writepage method.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 40a9b03ef821..007a73052561 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -1006,8 +1006,10 @@ static int do_writepage(struct page *page, int len)
>    * on the page lock and it would not write the truncated inode node to the
>    * journal before we have finished.
>    */
> -static int ubifs_writepage(struct page *page, struct writeback_control *wbc)
> +static int ubifs_writepage(struct folio *folio, struct writeback_control *wbc,
> +		void *data)
>   {
> +	struct page *page = &folio->page;
>   	struct inode *inode = page->mapping->host;
>   	struct ubifs_info *c = inode->i_sb->s_fs_info;
>   	struct ubifs_inode *ui = ubifs_inode(inode);
> @@ -1079,6 +1081,12 @@ static int ubifs_writepage(struct page *page, struct writeback_control *wbc)
>   	return err;
>   }
>   
> +static int ubifs_writepages(struct address_space *mapping,
> +		struct writeback_control *wbc)
> +{
> +	return write_cache_pages(mapping, wbc, ubifs_writepage, NULL);
> +}
> +
>   /**
>    * do_attr_changes - change inode attributes.
>    * @inode: inode to change attributes for
> @@ -1648,7 +1656,7 @@ static int ubifs_symlink_getattr(struct mnt_idmap *idmap,
>   
>   const struct address_space_operations ubifs_file_address_operations = {
>   	.read_folio     = ubifs_read_folio,
> -	.writepage      = ubifs_writepage,
> +	.writepages     = ubifs_writepages,
>   	.write_begin    = ubifs_write_begin,
>   	.write_end      = ubifs_write_end,
>   	.invalidate_folio = ubifs_invalidate_folio,
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 06/15] ubifs: Convert ubifs_vm_page_mkwrite() to use a folio

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> Replace six implicit calls to compound_head() with one.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   Documentation/mm/page_cache.rst | 10 +++++++++
>   fs/ubifs/file.c                 | 36 ++++++++++++++++-----------------
>   2 files changed, 28 insertions(+), 18 deletions(-)

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> 
> diff --git a/Documentation/mm/page_cache.rst b/Documentation/mm/page_cache.rst
> index 75eba7c431b2..138d61f869df 100644
> --- a/Documentation/mm/page_cache.rst
> +++ b/Documentation/mm/page_cache.rst
> @@ -3,3 +3,13 @@
>   ==========
>   Page Cache
>   ==========
> +
> +The page cache is the primary way that the user and the rest of the kernel
> +interact with filesystems.  It can be bypassed (e.g. with O_DIRECT),
> +but normal reads, writes and mmaps go through the page cache.
> +
> +Folios
> +======
> +
> +The folio is the unit of memory management within the page cache.
> +Operations
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index e755d0a11146..9dfedd91b576 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -1519,14 +1519,14 @@ static bool ubifs_release_folio(struct folio *folio, gfp_t unused_gfp_flags)
>    */
>   static vm_fault_t ubifs_vm_page_mkwrite(struct vm_fault *vmf)
>   {
> -	struct page *page = vmf->page;
> +	struct folio *folio = page_folio(vmf->page);
>   	struct inode *inode = file_inode(vmf->vma->vm_file);
>   	struct ubifs_info *c = inode->i_sb->s_fs_info;
>   	struct timespec64 now = current_time(inode);
>   	struct ubifs_budget_req req = { .new_page = 1 };
>   	int err, update_time;
>   
> -	dbg_gen("ino %lu, pg %lu, i_size %lld",	inode->i_ino, page->index,
> +	dbg_gen("ino %lu, pg %lu, i_size %lld",	inode->i_ino, folio->index,
>   		i_size_read(inode));
>   	ubifs_assert(c, !c->ro_media && !c->ro_mount);
>   
> @@ -1534,17 +1534,17 @@ static vm_fault_t ubifs_vm_page_mkwrite(struct vm_fault *vmf)
>   		return VM_FAULT_SIGBUS; /* -EROFS */
>   
>   	/*
> -	 * We have not locked @page so far so we may budget for changing the
> -	 * page. Note, we cannot do this after we locked the page, because
> +	 * We have not locked @folio so far so we may budget for changing the
> +	 * folio. Note, we cannot do this after we locked the folio, because
>   	 * budgeting may cause write-back which would cause deadlock.
>   	 *
> -	 * At the moment we do not know whether the page is dirty or not, so we
> -	 * assume that it is not and budget for a new page. We could look at
> +	 * At the moment we do not know whether the folio is dirty or not, so we
> +	 * assume that it is not and budget for a new folio. We could look at
>   	 * the @PG_private flag and figure this out, but we may race with write
> -	 * back and the page state may change by the time we lock it, so this
> +	 * back and the folio state may change by the time we lock it, so this
>   	 * would need additional care. We do not bother with this at the
>   	 * moment, although it might be good idea to do. Instead, we allocate
> -	 * budget for a new page and amend it later on if the page was in fact
> +	 * budget for a new folio and amend it later on if the folio was in fact
>   	 * dirty.
>   	 *
>   	 * The budgeting-related logic of this function is similar to what we
> @@ -1567,21 +1567,21 @@ static vm_fault_t ubifs_vm_page_mkwrite(struct vm_fault *vmf)
>   		return VM_FAULT_SIGBUS;
>   	}
>   
> -	lock_page(page);
> -	if (unlikely(page->mapping != inode->i_mapping ||
> -		     page_offset(page) > i_size_read(inode))) {
> -		/* Page got truncated out from underneath us */
> +	folio_lock(folio);
> +	if (unlikely(folio->mapping != inode->i_mapping ||
> +		     folio_pos(folio) >= i_size_read(inode))) {
> +		/* Folio got truncated out from underneath us */
>   		goto sigbus;
>   	}
>   
> -	if (PagePrivate(page))
> +	if (folio->private)
>   		release_new_page_budget(c);
>   	else {
> -		if (!PageChecked(page))
> +		if (!folio_test_checked(folio))
>   			ubifs_convert_page_budget(c);
> -		attach_page_private(page, (void *)1);
> +		folio_attach_private(folio, (void *)1);
>   		atomic_long_inc(&c->dirty_pg_cnt);
> -		__set_page_dirty_nobuffers(page);
> +		filemap_dirty_folio(folio->mapping, folio);
>   	}
>   
>   	if (update_time) {
> @@ -1597,11 +1597,11 @@ static vm_fault_t ubifs_vm_page_mkwrite(struct vm_fault *vmf)
>   			ubifs_release_dirty_inode_budget(c, ui);
>   	}
>   
> -	wait_for_stable_page(page);
> +	folio_wait_stable(folio);
>   	return VM_FAULT_LOCKED;
>   
>   sigbus:
> -	unlock_page(page);
> +	folio_unlock(folio);
>   	ubifs_release_budget(c, &req);
>   	return VM_FAULT_SIGBUS;
>   }
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 08/15] ubifs: Convert ubifs_write_begin() to use a folio

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> Save eight calls to compound_head() by using the new folio API.
> Remove a few assumptions that would break with large folios.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 36 +++++++++++++++++-------------------
>   1 file changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 6302ce65e0b3..ef262499f228 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -428,7 +428,7 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   	pgoff_t index = pos >> PAGE_SHIFT;
>   	int err, appending = !!(pos + len > inode->i_size);
>   	int skipped_read = 0;
> -	struct page *page;
> +	struct folio *folio;
>   
>   	ubifs_assert(c, ubifs_inode(inode)->ui_size == inode->i_size);
>   	ubifs_assert(c, !c->ro_media && !c->ro_mount);
> @@ -437,13 +437,14 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   		return -EROFS;
>   
>   	/* Try out the fast-path part first */
> -	page = grab_cache_page_write_begin(mapping, index);
> -	if (unlikely(!page))
> -		return -ENOMEM;
> +	folio = __filemap_get_folio(mapping, index, FGP_WRITEBEGIN,
> +			mapping_gfp_mask(mapping));
> +	if (IS_ERR(folio))
> +		return PTR_ERR(folio);
>   
> -	if (!PageUptodate(page)) {
> +	if (!folio_test_uptodate(folio)) {
>   		/* The page is not loaded from the flash */
> -		if (!(pos & ~PAGE_MASK) && len == PAGE_SIZE) {
> +		if (pos == folio_pos(folio) && len >= folio_size(folio)) {

Why not len == folio_size(folio)? Although 'len >= folio_size(folio)' is 
not wrong.

>   			/*
>   			 * We change whole page so no need to load it. But we
>   			 * do not know whether this page exists on the media or
> @@ -453,29 +454,27 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   			 * media. Thus, we are setting the @PG_checked flag
>   			 * here.
>   			 */
> -			SetPageChecked(page);
> +			folio_set_checked(folio);
>   			skipped_read = 1;
>   		} else {
> -			err = do_readpage(page);
> +			err = do_readpage(&folio->page);
>   			if (err) {
> -				unlock_page(page);
> -				put_page(page);
> +				folio_unlock(folio);
> +				folio_put(folio);
>   				return err;
>   			}
>   		}
>   	}
>   
> -	err = allocate_budget(c, page, ui, appending);
> +	err = allocate_budget(c, &folio->page, ui, appending);
>   	if (unlikely(err)) {
>   		ubifs_assert(c, err == -ENOSPC);
>   		/*
>   		 * If we skipped reading the page because we were going to
>   		 * write all of it, then it is not up to date.
>   		 */
> -		if (skipped_read) {
> -			ClearPageChecked(page);
> -			ClearPageUptodate(page);
> -		}
> +		if (skipped_read)
> +			folio_clear_checked(folio);
>   		/*
>   		 * Budgeting failed which means it would have to force
>   		 * write-back but didn't, because we set the @fast flag in the
> @@ -487,8 +486,8 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   			ubifs_assert(c, mutex_is_locked(&ui->ui_mutex));
>   			mutex_unlock(&ui->ui_mutex);
>   		}
> -		unlock_page(page);
> -		put_page(page);
> +		folio_unlock(folio);
> +		folio_put(folio);
>   
>   		return write_begin_slow(mapping, pos, len, pagep);
>   	}
> @@ -499,9 +498,8 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   	 * with @ui->ui_mutex locked if we are appending pages, and unlocked
>   	 * otherwise. This is an optimization (slightly hacky though).
>   	 */
> -	*pagep = page;
> +	*pagep = &folio->page;
>   	return 0;
> -
>   }
>   
>   /**
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 11/15] ubifs: Convert allocate_budget() to work on a folio

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> The one caller has a folio, so pass it in instead of the page.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index cc450afdcc47..6bc7965ce918 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -307,7 +307,7 @@ static int write_begin_slow(struct address_space *mapping,
>   /**
>    * allocate_budget - allocate budget for 'ubifs_write_begin()'.
>    * @c: UBIFS file-system description object
> - * @page: page to allocate budget for
> + * @folio: folio to allocate budget for
>    * @ui: UBIFS inode object the page belongs to
>    * @appending: non-zero if the page is appended
>    *
> @@ -318,15 +318,15 @@ static int write_begin_slow(struct address_space *mapping,
>    *
>    * Returns: %0 in case of success and %-ENOSPC in case of failure.
>    */
> -static int allocate_budget(struct ubifs_info *c, struct page *page,
> +static int allocate_budget(struct ubifs_info *c, struct folio *folio,
>   			   struct ubifs_inode *ui, int appending)
>   {
>   	struct ubifs_budget_req req = { .fast = 1 };
>   
> -	if (PagePrivate(page)) {
> +	if (folio->private) {
>   		if (!appending)
>   			/*
> -			 * The page is dirty and we are not appending, which
> +			 * The folio is dirty and we are not appending, which
>   			 * means no budget is needed at all.
>   			 */
>   			return 0;
> @@ -350,11 +350,11 @@ static int allocate_budget(struct ubifs_info *c, struct page *page,
>   		 */
>   		req.dirtied_ino = 1;
>   	} else {
> -		if (PageChecked(page))
> +		if (folio_test_checked(folio))
>   			/*
>   			 * The page corresponds to a hole and does not
>   			 * exist on the media. So changing it makes
> -			 * make the amount of indexing information
> +			 * the amount of indexing information
>   			 * larger, and we have to budget for a new
>   			 * page.
>   			 */
> @@ -462,7 +462,7 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   		}
>   	}
>   
> -	err = allocate_budget(c, &folio->page, ui, appending);
> +	err = allocate_budget(c, folio, ui, appending);
>   	if (unlikely(err)) {
>   		ubifs_assert(c, err == -ENOSPC);
>   		/*
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 10/15] ubifs: Convert do_readpage() to take a folio

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> All the callers now have a folio, so pass it in, and convert do_readpage()
> to us folios directly.  Includes unifying the exit paths from this
> function and using kmap_local instead of plain kmap.  This function
> should now work with large folios, but this is not tested.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 64 +++++++++++++++++++++++--------------------------
>   1 file changed, 30 insertions(+), 34 deletions(-)

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> 
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 52027b4feebf..cc450afdcc47 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -96,36 +96,36 @@ static int read_block(struct inode *inode, void *addr, unsigned int block,
>   	return -EINVAL;
>   }
>   
> -static int do_readpage(struct page *page)
> +static int do_readpage(struct folio *folio)
>   {
>   	void *addr;
>   	int err = 0, i;
>   	unsigned int block, beyond;
> -	struct ubifs_data_node *dn;
> -	struct inode *inode = page->mapping->host;
> +	struct ubifs_data_node *dn = NULL;
> +	struct inode *inode = folio->mapping->host;
>   	struct ubifs_info *c = inode->i_sb->s_fs_info;
>   	loff_t i_size = i_size_read(inode);
>   
>   	dbg_gen("ino %lu, pg %lu, i_size %lld, flags %#lx",
> -		inode->i_ino, page->index, i_size, page->flags);
> -	ubifs_assert(c, !PageChecked(page));
> -	ubifs_assert(c, !PagePrivate(page));
> +		inode->i_ino, folio->index, i_size, folio->flags);
> +	ubifs_assert(c, !folio_test_checked(folio));
> +	ubifs_assert(c, !folio->private);
>   
> -	addr = kmap(page);
> +	addr = kmap_local_folio(folio, 0);
>   
> -	block = page->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
> +	block = folio->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
>   	beyond = (i_size + UBIFS_BLOCK_SIZE - 1) >> UBIFS_BLOCK_SHIFT;
>   	if (block >= beyond) {
>   		/* Reading beyond inode */
> -		SetPageChecked(page);
> -		memset(addr, 0, PAGE_SIZE);
> +		folio_set_checked(folio);
> +		addr = folio_zero_tail(folio, 0, addr);
>   		goto out;
>   	}
>   
>   	dn = kmalloc(UBIFS_MAX_DATA_NODE_SZ, GFP_NOFS);
>   	if (!dn) {
>   		err = -ENOMEM;
> -		goto error;
> +		goto out;
>   	}
>   
>   	i = 0;
> @@ -150,39 +150,35 @@ static int do_readpage(struct page *page)
>   					memset(addr + ilen, 0, dlen - ilen);
>   			}
>   		}
> -		if (++i >= UBIFS_BLOCKS_PER_PAGE)
> +		if (++i >= (UBIFS_BLOCKS_PER_PAGE << folio_order(folio)))
>   			break;
>   		block += 1;
>   		addr += UBIFS_BLOCK_SIZE;
> +		if (folio_test_highmem(folio) && (offset_in_page(addr) == 0)) {
> +			kunmap_local(addr - UBIFS_BLOCK_SIZE);
> +			addr = kmap_local_folio(folio, i * UBIFS_BLOCK_SIZE);
> +		}
>   	}
> +
>   	if (err) {
>   		struct ubifs_info *c = inode->i_sb->s_fs_info;
>   		if (err == -ENOENT) {
>   			/* Not found, so it must be a hole */
> -			SetPageChecked(page);
> +			folio_set_checked(folio);
>   			dbg_gen("hole");
> -			goto out_free;
> +			err = 0;
> +		} else {
> +			ubifs_err(c, "cannot read page %lu of inode %lu, error %d",
> +				  folio->index, inode->i_ino, err);
>   		}
> -		ubifs_err(c, "cannot read page %lu of inode %lu, error %d",
> -			  page->index, inode->i_ino, err);
> -		goto error;
>   	}
>   
> -out_free:
> -	kfree(dn);
>   out:
> -	SetPageUptodate(page);
> -	ClearPageError(page);
> -	flush_dcache_page(page);
> -	kunmap(page);
> -	return 0;
> -
> -error:
>   	kfree(dn);
> -	ClearPageUptodate(page);
> -	SetPageError(page);
> -	flush_dcache_page(page);
> -	kunmap(page);
> +	if (!err)
> +		folio_mark_uptodate(folio);
> +	flush_dcache_folio(folio);
> +	kunmap_local(addr);
>   	return err;
>   }
>   
> @@ -254,7 +250,7 @@ static int write_begin_slow(struct address_space *mapping,
>   		if (pos == folio_pos(folio) && len >= folio_size(folio))
>   			folio_set_checked(folio);
>   		else {
> -			err = do_readpage(&folio->page);
> +			err = do_readpage(folio);
>   			if (err) {
>   				folio_unlock(folio);
>   				folio_put(folio);
> @@ -457,7 +453,7 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   			folio_set_checked(folio);
>   			skipped_read = 1;
>   		} else {
> -			err = do_readpage(&folio->page);
> +			err = do_readpage(folio);
>   			if (err) {
>   				folio_unlock(folio);
>   				folio_put(folio);
> @@ -561,7 +557,7 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
>   		 * Return 0 to force VFS to repeat the whole operation, or the
>   		 * error code if 'do_readpage()' fails.
>   		 */
> -		copied = do_readpage(&folio->page);
> +		copied = do_readpage(folio);
>   		goto out;
>   	}
>   
> @@ -897,7 +893,7 @@ static int ubifs_read_folio(struct file *file, struct folio *folio)
>   
>   	if (ubifs_bulk_read(page))
>   		return 0;
> -	do_readpage(page);
> +	do_readpage(folio);
>   	folio_unlock(folio);
>   	return 0;
>   }
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 12/15] ubifs: Convert cancel_budget() to take a folio

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> The one caller already has a folio, so pass it in instead of the page.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> 
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 6bc7965ce918..f34371436a84 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -501,14 +501,14 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   /**
>    * cancel_budget - cancel budget.
>    * @c: UBIFS file-system description object
> - * @page: page to cancel budget for
> + * @folio: folio to cancel budget for
>    * @ui: UBIFS inode object the page belongs to
>    * @appending: non-zero if the page is appended
>    *
>    * This is a helper function for a page write operation. It unlocks the
>    * @ui->ui_mutex in case of appending.
>    */
> -static void cancel_budget(struct ubifs_info *c, struct page *page,
> +static void cancel_budget(struct ubifs_info *c, struct folio *folio,
>   			  struct ubifs_inode *ui, int appending)
>   {
>   	if (appending) {
> @@ -516,8 +516,8 @@ static void cancel_budget(struct ubifs_info *c, struct page *page,
>   			ubifs_release_dirty_inode_budget(c, ui);
>   		mutex_unlock(&ui->ui_mutex);
>   	}
> -	if (!PagePrivate(page)) {
> -		if (PageChecked(page))
> +	if (!folio->private) {
> +		if (folio_test_checked(folio))
>   			release_new_page_budget(c);
>   		else
>   			release_existing_page_budget(c);
> @@ -550,7 +550,7 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
>   		 */
>   		dbg_gen("copied %d instead of %d, read page and repeat",
>   			copied, len);
> -		cancel_budget(c, &folio->page, ui, appending);
> +		cancel_budget(c, folio, ui, appending);
>   		folio_clear_checked(folio);
>   
>   		/*
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 13/15] ubifs: Pass a folio into ubifs_bulk_read() and ubifs_do_bulk_read()

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> This saves a single call to compound_head().
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 28 +++++++++++++---------------
>   1 file changed, 13 insertions(+), 15 deletions(-)
> 

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index f34371436a84..8cad1fe9c50f 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -705,15 +705,15 @@ static int populate_page(struct ubifs_info *c, struct page *page,
>    * ubifs_do_bulk_read - do bulk-read.
>    * @c: UBIFS file-system description object
>    * @bu: bulk-read information
> - * @page1: first page to read
> + * @folio1: first folio to read
>    *
>    * Returns: %1 if the bulk-read is done, otherwise %0 is returned.
>    */
>   static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
> -			      struct page *page1)
> +			      struct folio *folio1)
>   {
> -	pgoff_t offset = page1->index, end_index;
> -	struct address_space *mapping = page1->mapping;
> +	pgoff_t offset = folio1->index, end_index;
> +	struct address_space *mapping = folio1->mapping;
>   	struct inode *inode = mapping->host;
>   	struct ubifs_inode *ui = ubifs_inode(inode);
>   	int err, page_idx, page_cnt, ret = 0, n = 0;
> @@ -763,11 +763,11 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
>   			goto out_warn;
>   	}
>   
> -	err = populate_page(c, page1, bu, &n);
> +	err = populate_page(c, &folio1->page, bu, &n);
>   	if (err)
>   		goto out_warn;
>   
> -	unlock_page(page1);
> +	folio_unlock(folio1);
>   	ret = 1;
>   
>   	isize = i_size_read(inode);
> @@ -812,7 +812,7 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
>   
>   /**
>    * ubifs_bulk_read - determine whether to bulk-read and, if so, do it.
> - * @page: page from which to start bulk-read.
> + * @folio: folio from which to start bulk-read.
>    *
>    * Some flash media are capable of reading sequentially at faster rates. UBIFS
>    * bulk-read facility is designed to take advantage of that, by reading in one
> @@ -821,12 +821,12 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
>    *
>    * Returns: %1 if a bulk-read is done and %0 otherwise.
>    */
> -static int ubifs_bulk_read(struct page *page)
> +static int ubifs_bulk_read(struct folio *folio)
>   {
> -	struct inode *inode = page->mapping->host;
> +	struct inode *inode = folio->mapping->host;
>   	struct ubifs_info *c = inode->i_sb->s_fs_info;
>   	struct ubifs_inode *ui = ubifs_inode(inode);
> -	pgoff_t index = page->index, last_page_read = ui->last_page_read;
> +	pgoff_t index = folio->index, last_page_read = ui->last_page_read;
>   	struct bu_info *bu;
>   	int err = 0, allocated = 0;
>   
> @@ -874,8 +874,8 @@ static int ubifs_bulk_read(struct page *page)
>   
>   	bu->buf_len = c->max_bu_buf_len;
>   	data_key_init(c, &bu->key, inode->i_ino,
> -		      page->index << UBIFS_BLOCKS_PER_PAGE_SHIFT);
> -	err = ubifs_do_bulk_read(c, bu, page);
> +		      folio->index << UBIFS_BLOCKS_PER_PAGE_SHIFT);
> +	err = ubifs_do_bulk_read(c, bu, folio);
>   
>   	if (!allocated)
>   		mutex_unlock(&c->bu_mutex);
> @@ -889,9 +889,7 @@ static int ubifs_bulk_read(struct page *page)
>   
>   static int ubifs_read_folio(struct file *file, struct folio *folio)
>   {
> -	struct page *page = &folio->page;
> -
> -	if (ubifs_bulk_read(page))
> +	if (ubifs_bulk_read(folio))
>   		return 0;
>   	do_readpage(folio);
>   	folio_unlock(folio);
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 14/15] ubifs: Use a folio in ubifs_do_bulk_read()

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> When looking in the page cache, retrieve a folio instead of a page.
> This would need some work to make it safe for large folios.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> 
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 8cad1fe9c50f..dec5258b4f38 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -777,19 +777,19 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
>   
>   	for (page_idx = 1; page_idx < page_cnt; page_idx++) {
>   		pgoff_t page_offset = offset + page_idx;
> -		struct page *page;
> +		struct folio *folio;
>   
>   		if (page_offset > end_index)
>   			break;
> -		page = pagecache_get_page(mapping, page_offset,
> +		folio = __filemap_get_folio(mapping, page_offset,
>   				 FGP_LOCK|FGP_ACCESSED|FGP_CREAT|FGP_NOWAIT,
>   				 ra_gfp_mask);
> -		if (!page)
> +		if (IS_ERR(folio))
>   			break;
> -		if (!PageUptodate(page))
> -			err = populate_page(c, page, bu, &n);
> -		unlock_page(page);
> -		put_page(page);
> +		if (!folio_test_uptodate(folio))
> +			err = populate_page(c, &folio->page, bu, &n);
> +		folio_unlock(folio);
> +		folio_put(folio);
>   		if (err)
>   			break;
>   	}
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 15/15] ubifs: Convert populate_page() to take a folio

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> Both callers now have a folio, so pass it in.  This function contains
> several assumptions that folios are not large.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 41 +++++++++++++++++++++--------------------
>   1 file changed, 21 insertions(+), 20 deletions(-)
> 

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index dec5258b4f38..fbbd07390959 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -592,35 +592,35 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
>   /**
>    * populate_page - copy data nodes into a page for bulk-read.
>    * @c: UBIFS file-system description object
> - * @page: page
> + * @folio: folio
>    * @bu: bulk-read information
>    * @n: next zbranch slot
>    *
>    * Returns: %0 on success and a negative error code on failure.
>    */
> -static int populate_page(struct ubifs_info *c, struct page *page,
> +static int populate_page(struct ubifs_info *c, struct folio *folio,
>   			 struct bu_info *bu, int *n)
>   {
>   	int i = 0, nn = *n, offs = bu->zbranch[0].offs, hole = 0, read = 0;
> -	struct inode *inode = page->mapping->host;
> +	struct inode *inode = folio->mapping->host;
>   	loff_t i_size = i_size_read(inode);
>   	unsigned int page_block;
>   	void *addr, *zaddr;
>   	pgoff_t end_index;
>   
>   	dbg_gen("ino %lu, pg %lu, i_size %lld, flags %#lx",
> -		inode->i_ino, page->index, i_size, page->flags);
> +		inode->i_ino, folio->index, i_size, folio->flags);
>   
> -	addr = zaddr = kmap(page);
> +	addr = zaddr = kmap_local_folio(folio, 0);
>   
>   	end_index = (i_size - 1) >> PAGE_SHIFT;
> -	if (!i_size || page->index > end_index) {
> +	if (!i_size || folio->index > end_index) {
>   		hole = 1;
> -		memset(addr, 0, PAGE_SIZE);
> +		addr = folio_zero_tail(folio, 0, addr);
>   		goto out_hole;
>   	}
>   
> -	page_block = page->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
> +	page_block = folio->index << UBIFS_BLOCKS_PER_PAGE_SHIFT;
>   	while (1) {
>   		int err, len, out_len, dlen;
>   
> @@ -669,9 +669,13 @@ static int populate_page(struct ubifs_info *c, struct page *page,
>   			break;
>   		addr += UBIFS_BLOCK_SIZE;
>   		page_block += 1;
> +		if (folio_test_highmem(folio) && (offset_in_page(addr) == 0)) {
> +			kunmap_local(addr - UBIFS_BLOCK_SIZE);
> +			addr = kmap_local_folio(folio, i * UBIFS_BLOCK_SIZE);
> +		}
>   	}
>   
> -	if (end_index == page->index) {
> +	if (end_index == folio->index) {
>   		int len = i_size & (PAGE_SIZE - 1);
>   
>   		if (len && len < read)
> @@ -680,22 +684,19 @@ static int populate_page(struct ubifs_info *c, struct page *page,
>   
>   out_hole:
>   	if (hole) {
> -		SetPageChecked(page);
> +		folio_set_checked(folio);
>   		dbg_gen("hole");
>   	}
>   
> -	SetPageUptodate(page);
> -	ClearPageError(page);
> -	flush_dcache_page(page);
> -	kunmap(page);
> +	folio_mark_uptodate(folio);
> +	flush_dcache_folio(folio);
> +	kunmap_local(addr);
>   	*n = nn;
>   	return 0;
>   
>   out_err:
> -	ClearPageUptodate(page);
> -	SetPageError(page);
> -	flush_dcache_page(page);
> -	kunmap(page);
> +	flush_dcache_folio(folio);
> +	kunmap_local(addr);
>   	ubifs_err(c, "bad data node (block %u, inode %lu)",
>   		  page_block, inode->i_ino);
>   	return -EINVAL;
> @@ -763,7 +764,7 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
>   			goto out_warn;
>   	}
>   
> -	err = populate_page(c, &folio1->page, bu, &n);
> +	err = populate_page(c, folio1, bu, &n);
>   	if (err)
>   		goto out_warn;
>   
> @@ -787,7 +788,7 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
>   		if (IS_ERR(folio))
>   			break;
>   		if (!folio_test_uptodate(folio))
> -			err = populate_page(c, &folio->page, bu, &n);
> +			err = populate_page(c, folio, bu, &n);
>   		folio_unlock(folio);
>   		folio_put(folio);
>   		if (err)
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 01/15] ubifs: Set page uptodate in the correct place

[Matthew Wilcox]

On Mon, Jan 22, 2024 at 03:22:45PM +0800, Zhihao Cheng wrote:
> 在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> > Page cache reads are lockless, so setting the freshly allocated page
> > uptodate before we've overwritten it with the data it's supposed to have
> > in it will allow a simultaneous reader to see old data.  Move the call
> > to SetPageUptodate into ubifs_write_end(), which is after we copied the
> > new data into the page.
> 
> This solution looks good to me, and I think 'SetPageUptodate' should be
> removed from write_begin_slow(slow path) too.

I didn't bother because we have just read into the page so it is
uptodate.  A racing read will see the data from before the write, but
that's an acceptable ordering of events.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 01/15] ubifs: Set page uptodate in the correct place

[Matthew Wilcox]

On Mon, Jan 22, 2024 at 03:22:45PM +0800, Zhihao Cheng wrote:
> 在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> > Page cache reads are lockless, so setting the freshly allocated page
> > uptodate before we've overwritten it with the data it's supposed to have
> > in it will allow a simultaneous reader to see old data.  Move the call
> > to SetPageUptodate into ubifs_write_end(), which is after we copied the
> > new data into the page.
> 
> This solution looks good to me, and I think 'SetPageUptodate' should be
> removed from write_begin_slow(slow path) too.

I didn't bother because we have just read into the page so it is
uptodate.  A racing read will see the data from before the write, but
that's an acceptable ordering of events.

Re: [PATCH 08/15] ubifs: Convert ubifs_write_begin() to use a folio

[Matthew Wilcox]

On Mon, Jan 22, 2024 at 07:51:03PM +0800, Zhihao Cheng wrote:
> > @@ -437,13 +437,14 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
> >   		return -EROFS;
> >   	/* Try out the fast-path part first */
> > -	page = grab_cache_page_write_begin(mapping, index);
> > -	if (unlikely(!page))
> > -		return -ENOMEM;
> > +	folio = __filemap_get_folio(mapping, index, FGP_WRITEBEGIN,
> > +			mapping_gfp_mask(mapping));
> > +	if (IS_ERR(folio))
> > +		return PTR_ERR(folio);
> > -	if (!PageUptodate(page)) {
> > +	if (!folio_test_uptodate(folio)) {
> >   		/* The page is not loaded from the flash */
> > -		if (!(pos & ~PAGE_MASK) && len == PAGE_SIZE) {
> > +		if (pos == folio_pos(folio) && len >= folio_size(folio)) {
> 
> Why not len == folio_size(folio)? Although 'len >= folio_size(folio)' is not
> wrong.

This is based on my experience with iomap.  Today, the caller passes in
min(length-of-write, PAGE_SIZE).  In order to be able to create large
folios in the write path, we want to change the caller of write_begin
to pass in the length of the write, so we can see a len which is larger
than the size of the folio we found.


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 08/15] ubifs: Convert ubifs_write_begin() to use a folio

[Zhihao Cheng]

在 2024/1/22 22:43, Matthew Wilcox 写道:
> On Mon, Jan 22, 2024 at 07:51:03PM +0800, Zhihao Cheng wrote:
>>> @@ -437,13 +437,14 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>>>    		return -EROFS;
>>>    	/* Try out the fast-path part first */
>>> -	page = grab_cache_page_write_begin(mapping, index);
>>> -	if (unlikely(!page))
>>> -		return -ENOMEM;
>>> +	folio = __filemap_get_folio(mapping, index, FGP_WRITEBEGIN,
>>> +			mapping_gfp_mask(mapping));
>>> +	if (IS_ERR(folio))
>>> +		return PTR_ERR(folio);
>>> -	if (!PageUptodate(page)) {
>>> +	if (!folio_test_uptodate(folio)) {
>>>    		/* The page is not loaded from the flash */
>>> -		if (!(pos & ~PAGE_MASK) && len == PAGE_SIZE) {
>>> +		if (pos == folio_pos(folio) && len >= folio_size(folio)) {
>>
>> Why not len == folio_size(folio)? Although 'len >= folio_size(folio)' is not
>> wrong.
> 
> This is based on my experience with iomap.  Today, the caller passes in
> min(length-of-write, PAGE_SIZE).  In order to be able to create large
> folios in the write path, we want to change the caller of write_begin
> to pass in the length of the write, so we can see a len which is larger
> than the size of the folio we found.
> 
> .
> 

I guess you are preparing for adding large folio support for 
generic_perform_write path, may I have a look for the demo code? I'm a 
little confused for this words "we want to change the caller of 
write_begin to pass in the length of the write". According to the 
implementation of iomap_write_iter(), I find the write length in each 
iteration is min(write_bytes, folio_size), so I guess the write length 
passed in ubifs_write_begin won't exceed folio_size.


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 01/15] ubifs: Set page uptodate in the correct place

[Zhihao Cheng]

在 2024/1/22 22:40, Matthew Wilcox 写道:
> On Mon, Jan 22, 2024 at 03:22:45PM +0800, Zhihao Cheng wrote:
>> 在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
>>> Page cache reads are lockless, so setting the freshly allocated page
>>> uptodate before we've overwritten it with the data it's supposed to have
>>> in it will allow a simultaneous reader to see old data.  Move the call
>>> to SetPageUptodate into ubifs_write_end(), which is after we copied the
>>> new data into the page.
>>
>> This solution looks good to me, and I think 'SetPageUptodate' should be
>> removed from write_begin_slow(slow path) too.
> 
> I didn't bother because we have just read into the page so it is
> uptodate.  A racing read will see the data from before the write, but
> that's an acceptable ordering of events.
> .
> 

I can't find where the page is read and set uptodate. I think the 
uninitialized data can be found in following path:

       writer               reader
ubifs_write_begin
  page1 = grab_cache_page_write_begin
  err = allocate_budget // ENOSPC
  unlock_page(page1)
  put_page(page1)
  write_begin_slow
   page2 = grab_cache_page_write_begin
   SetPageChecked(page2)
   SetPageUptodate(page2)
                 generic_file_read_iter
                  filemap_read
                   filemap_get_pages
                    filemap_get_read_batch
                    if (!folio_test_uptodate) // page2 is uptodate
                   copy_folio_to_iter // read uninitialized page content!
copy_page_from_iter_atomic // copy data to cover uninitialized page content

Re: [PATCH 01/15] ubifs: Set page uptodate in the correct place

[Zhihao Cheng]

在 2024/1/22 22:40, Matthew Wilcox 写道:
> On Mon, Jan 22, 2024 at 03:22:45PM +0800, Zhihao Cheng wrote:
>> 在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
>>> Page cache reads are lockless, so setting the freshly allocated page
>>> uptodate before we've overwritten it with the data it's supposed to have
>>> in it will allow a simultaneous reader to see old data.  Move the call
>>> to SetPageUptodate into ubifs_write_end(), which is after we copied the
>>> new data into the page.
>>
>> This solution looks good to me, and I think 'SetPageUptodate' should be
>> removed from write_begin_slow(slow path) too.
> 
> I didn't bother because we have just read into the page so it is
> uptodate.  A racing read will see the data from before the write, but
> that's an acceptable ordering of events.
> .
> 

I can't find where the page is read and set uptodate. I think the 
uninitialized data can be found in following path:

       writer               reader
ubifs_write_begin
  page1 = grab_cache_page_write_begin
  err = allocate_budget // ENOSPC
  unlock_page(page1)
  put_page(page1)
  write_begin_slow
   page2 = grab_cache_page_write_begin
   SetPageChecked(page2)
   SetPageUptodate(page2)
                 generic_file_read_iter
                  filemap_read
                   filemap_get_pages
                    filemap_get_read_batch
                    if (!folio_test_uptodate) // page2 is uptodate
                   copy_folio_to_iter // read uninitialized page content!
copy_page_from_iter_atomic // copy data to cover uninitialized page content

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 08/15] ubifs: Convert ubifs_write_begin() to use a folio

[Matthew Wilcox]

On Tue, Jan 23, 2024 at 10:15:50AM +0800, Zhihao Cheng wrote:
> 在 2024/1/22 22:43, Matthew Wilcox 写道:
> > On Mon, Jan 22, 2024 at 07:51:03PM +0800, Zhihao Cheng wrote:
> > > Why not len == folio_size(folio)? Although 'len >= folio_size(folio)' is not
> > > wrong.
> > 
> > This is based on my experience with iomap.  Today, the caller passes in
> > min(length-of-write, PAGE_SIZE).  In order to be able to create large
> > folios in the write path, we want to change the caller of write_begin
> > to pass in the length of the write, so we can see a len which is larger
> > than the size of the folio we found.
> 
> I guess you are preparing for adding large folio support for
> generic_perform_write path, may I have a look for the demo code? I'm a

https://lore.kernel.org/linux-fsdevel/20240111192513.3505769-1-willy@infradead.org/
(now withdrawn because it breaks ext4)

> little confused for this words "we want to change the caller of write_begin
> to pass in the length of the write". According to the implementation of
> iomap_write_iter(), I find the write length in each iteration is
> min(write_bytes, folio_size), so I guess the write length passed in
> ubifs_write_begin won't exceed folio_size.

But that happens after write_begin, because we don't know what the folio
size will be until we've called write_begin.

ie we call write_begin, passing in the size remaining from the write.
If write_begin finds an existing folio in the page cache, we use it.
If not, we allocate a folio based on the size of the write.  After
write_begin returns a folio, we limit the number of bytes copied to the
size of the folio.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 08/15] ubifs: Convert ubifs_write_begin() to use a folio

[Zhihao Cheng]

在 2024/1/23 11:07, Matthew Wilcox 写道:
> On Tue, Jan 23, 2024 at 10:15:50AM +0800, Zhihao Cheng wrote:
>> 在 2024/1/22 22:43, Matthew Wilcox 写道:
>>> On Mon, Jan 22, 2024 at 07:51:03PM +0800, Zhihao Cheng wrote:
>>>> Why not len == folio_size(folio)? Although 'len >= folio_size(folio)' is not
>>>> wrong.
>>>
>>> This is based on my experience with iomap.  Today, the caller passes in
>>> min(length-of-write, PAGE_SIZE).  In order to be able to create large
>>> folios in the write path, we want to change the caller of write_begin
>>> to pass in the length of the write, so we can see a len which is larger
>>> than the size of the folio we found.
>>
>> I guess you are preparing for adding large folio support for
>> generic_perform_write path, may I have a look for the demo code? I'm a
> 
> https://lore.kernel.org/linux-fsdevel/20240111192513.3505769-1-willy@infradead.org/
> (now withdrawn because it breaks ext4)
> 
>> little confused for this words "we want to change the caller of write_begin
>> to pass in the length of the write". According to the implementation of
>> iomap_write_iter(), I find the write length in each iteration is
>> min(write_bytes, folio_size), so I guess the write length passed in
>> ubifs_write_begin won't exceed folio_size.
> 
> But that happens after write_begin, because we don't know what the folio
> size will be until we've called write_begin.
> 
> ie we call write_begin, passing in the size remaining from the write.
> If write_begin finds an existing folio in the page cache, we use it.
> If not, we allocate a folio based on the size of the write.  After
> write_begin returns a folio, we limit the number of bytes copied to the
> size of the folio.
> .
> 

Makes sense. Thanks for the explaination.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 08/15] ubifs: Convert ubifs_write_begin() to use a folio

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> Save eight calls to compound_head() by using the new folio API.
> Remove a few assumptions that would break with large folios.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 36 +++++++++++++++++-------------------
>   1 file changed, 17 insertions(+), 19 deletions(-)
> 

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 6302ce65e0b3..ef262499f228 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -428,7 +428,7 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   	pgoff_t index = pos >> PAGE_SHIFT;
>   	int err, appending = !!(pos + len > inode->i_size);
>   	int skipped_read = 0;
> -	struct page *page;
> +	struct folio *folio;
>   
>   	ubifs_assert(c, ubifs_inode(inode)->ui_size == inode->i_size);
>   	ubifs_assert(c, !c->ro_media && !c->ro_mount);
> @@ -437,13 +437,14 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   		return -EROFS;
>   
>   	/* Try out the fast-path part first */
> -	page = grab_cache_page_write_begin(mapping, index);
> -	if (unlikely(!page))
> -		return -ENOMEM;
> +	folio = __filemap_get_folio(mapping, index, FGP_WRITEBEGIN,
> +			mapping_gfp_mask(mapping));
> +	if (IS_ERR(folio))
> +		return PTR_ERR(folio);
>   
> -	if (!PageUptodate(page)) {
> +	if (!folio_test_uptodate(folio)) {
>   		/* The page is not loaded from the flash */
> -		if (!(pos & ~PAGE_MASK) && len == PAGE_SIZE) {
> +		if (pos == folio_pos(folio) && len >= folio_size(folio)) {
>   			/*
>   			 * We change whole page so no need to load it. But we
>   			 * do not know whether this page exists on the media or
> @@ -453,29 +454,27 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   			 * media. Thus, we are setting the @PG_checked flag
>   			 * here.
>   			 */
> -			SetPageChecked(page);
> +			folio_set_checked(folio);
>   			skipped_read = 1;
>   		} else {
> -			err = do_readpage(page);
> +			err = do_readpage(&folio->page);
>   			if (err) {
> -				unlock_page(page);
> -				put_page(page);
> +				folio_unlock(folio);
> +				folio_put(folio);
>   				return err;
>   			}
>   		}
>   	}
>   
> -	err = allocate_budget(c, page, ui, appending);
> +	err = allocate_budget(c, &folio->page, ui, appending);
>   	if (unlikely(err)) {
>   		ubifs_assert(c, err == -ENOSPC);
>   		/*
>   		 * If we skipped reading the page because we were going to
>   		 * write all of it, then it is not up to date.
>   		 */
> -		if (skipped_read) {
> -			ClearPageChecked(page);
> -			ClearPageUptodate(page);
> -		}
> +		if (skipped_read)
> +			folio_clear_checked(folio);
>   		/*
>   		 * Budgeting failed which means it would have to force
>   		 * write-back but didn't, because we set the @fast flag in the
> @@ -487,8 +486,8 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   			ubifs_assert(c, mutex_is_locked(&ui->ui_mutex));
>   			mutex_unlock(&ui->ui_mutex);
>   		}
> -		unlock_page(page);
> -		put_page(page);
> +		folio_unlock(folio);
> +		folio_put(folio);
>   
>   		return write_begin_slow(mapping, pos, len, pagep);
>   	}
> @@ -499,9 +498,8 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   	 * with @ui->ui_mutex locked if we are appending pages, and unlocked
>   	 * otherwise. This is an optimization (slightly hacky though).
>   	 */
> -	*pagep = page;
> +	*pagep = &folio->page;
>   	return 0;
> -
>   }
>   
>   /**
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 09/15] ubifs: Convert ubifs_write_end() to use a folio

[Zhihao Cheng]

在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> Convert the incoming page pointer to a folio and use it throughout,
> saving several calls to compound_head().  Also remove some PAGE_SIZE
> assumptions.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> ---
>   fs/ubifs/file.c | 41 +++++++++++++++++++++--------------------
>   1 file changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index ef262499f228..52027b4feebf 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -532,6 +532,7 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
>   			   loff_t pos, unsigned len, unsigned copied,
>   			   struct page *page, void *fsdata)
>   {
> +	struct folio *folio = page_folio(page);
>   	struct inode *inode = mapping->host;
>   	struct ubifs_inode *ui = ubifs_inode(inode);
>   	struct ubifs_info *c = inode->i_sb->s_fs_info;
> @@ -539,47 +540,47 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
>   	int appending = !!(end_pos > inode->i_size);
>   
>   	dbg_gen("ino %lu, pos %llu, pg %lu, len %u, copied %d, i_size %lld",
> -		inode->i_ino, pos, page->index, len, copied, inode->i_size);
> +		inode->i_ino, pos, folio->index, len, copied, inode->i_size);
>   
> -	if (unlikely(copied < len && len == PAGE_SIZE)) {
> +	if (unlikely(copied < len && !folio_test_uptodate(folio))) {
>   		/*
> -		 * VFS copied less data to the page that it intended and
> +		 * VFS copied less data to the folio than it intended and
>   		 * declared in its '->write_begin()' call via the @len
> -		 * argument. If the page was not up-to-date, and @len was
> -		 * @PAGE_SIZE, the 'ubifs_write_begin()' function did
> +		 * argument. If the folio was not up-to-date,
> +		 * the 'ubifs_write_begin()' function did
>   		 * not load it from the media (for optimization reasons). This
> -		 * means that part of the page contains garbage. So read the
> -		 * page now.
> +		 * means that part of the folio contains garbage. So read the
> +		 * folio now.
>   		 */
>   		dbg_gen("copied %d instead of %d, read page and repeat",
>   			copied, len);
> -		cancel_budget(c, page, ui, appending);
> -		ClearPageChecked(page);
> +		cancel_budget(c, &folio->page, ui, appending);
> +		folio_clear_checked(folio);
>   
>   		/*
>   		 * Return 0 to force VFS to repeat the whole operation, or the
>   		 * error code if 'do_readpage()' fails.
>   		 */
> -		copied = do_readpage(page);
> +		copied = do_readpage(&folio->page);
>   		goto out;
>   	}
>   
> -	if (len == PAGE_SIZE)
> -		SetPageUptodate(page);
> +	if (len >= folio_size(folio))
> +		folio_mark_uptodate(folio);

So I think 'len == folio_size(folio)' is right? Since len is passed from 
'bytes' which is recalculated after write_begin:
  if (bytes > folio_size(folio) - offset)
      bytes = folio_size(folio) - offset;
The 'len' can't be greater than folio_size.

>   
> -	if (!PagePrivate(page)) {
> -		attach_page_private(page, (void *)1);
> +	if (!folio->private) {
> +		folio_attach_private(folio, (void *)1);
>   		atomic_long_inc(&c->dirty_pg_cnt);
> -		__set_page_dirty_nobuffers(page);
> +		filemap_dirty_folio(mapping, folio);
>   	}
>   
>   	if (appending) {
>   		i_size_write(inode, end_pos);
>   		ui->ui_size = end_pos;
>   		/*
> -		 * Note, we do not set @I_DIRTY_PAGES (which means that the
> -		 * inode has dirty pages), this has been done in
> -		 * '__set_page_dirty_nobuffers()'.
> +		 * We do not set @I_DIRTY_PAGES (which means that
> +		 * the inode has dirty pages), this was done in
> +		 * filemap_dirty_folio().
>   		 */
>   		__mark_inode_dirty(inode, I_DIRTY_DATASYNC);
>   		ubifs_assert(c, mutex_is_locked(&ui->ui_mutex));
> @@ -587,8 +588,8 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
>   	}
>   
>   out:
> -	unlock_page(page);
> -	put_page(page);
> +	folio_unlock(folio);
> +	folio_put(folio);
>   	return copied;
>   }
>   
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/15] ubifs folio conversion

[Richard Weinberger]

Matthew,

----- Ursprüngliche Mail -----
> Von: "Matthew Wilcox" <willy@infradead.org>
> An: "richard" <richard@nod.at>
> CC: "Matthew Wilcox" <willy@infradead.org>, "linux-mtd" <linux-mtd@lists.infradead.org>
> Gesendet: Sonntag, 21. Januar 2024 00:08:08
> Betreff: [PATCH 00/15] ubifs folio conversion

> This patchset converts ubifs to use folios throughout.  I made some
> attempt to support large folios, but more work definitely needs to be
> done before it can be enabled.  It's not clear to me whether it's worth
> doing, so I've left that for you ;-)

what is the benefit of large folios?

> The first patch fixes a misplaced call to SetPageUptodate which has been
> with us since the initial merge of ubifs.  It looks to be a pretty hard
> race to hit, which is why it probably hasn't affected anyone.

Thanks a lot for identifying and fixing this one!

> With my usual test config, this saves about 2kB of kernel text (almost
> 1%).  Some functions shrink more than others, eg ubifs_write_begin
> reduces by almost a third.  YMMV with different configs.
> 
> I've done a few other updates while I'm in here; eg setting/clearing
> PageError is pointless because the VFS doesn't use it and ubifs doesn't
> test it.  And we're phasing out use of PagePrivate / folio_test_private()
> in favour of just testing whether folio->private is set.
> 
> I ran this through xfstests and it didn't seem any worse.

Can you recommend a specific real load workload to stress test your changes?

Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/15] ubifs folio conversion

[Matthew Wilcox]

On Tue, Jan 23, 2024 at 08:33:49AM +0100, Richard Weinberger wrote:
> Matthew,
> 
> ----- Ursprüngliche Mail -----
> > Von: "Matthew Wilcox" <willy@infradead.org>
> > An: "richard" <richard@nod.at>
> > CC: "Matthew Wilcox" <willy@infradead.org>, "linux-mtd" <linux-mtd@lists.infradead.org>
> > Gesendet: Sonntag, 21. Januar 2024 00:08:08
> > Betreff: [PATCH 00/15] ubifs folio conversion
> 
> > This patchset converts ubifs to use folios throughout.  I made some
> > attempt to support large folios, but more work definitely needs to be
> > done before it can be enabled.  It's not clear to me whether it's worth
> > doing, so I've left that for you ;-)
> 
> what is the benefit of large folios?

For another purpose, I just wrote this up.  Possibly there are more
benefits, but these are the ones I remembered just now:

But why should a filesystem put in the effort to support large folios
(aka multi-page folios)?

    Lower overheads
        Fewer calls to the page allocator
        Shorter LRU list helps vmscan age memory
        Fewer calls to the page cache for large read()/write()
        Fewer calls to the architecture code for handling faults in mmaped files
        Potential use of larger TLB entries (eg contPTE / NAPOT)
    Being a better kernel citizen
        The more code using large folios, the easier it is to allocate large folios

The costs may outweigh the benefits. I would not invest time in making
iso9660, adfs or efs support large folios. But for filesystems which
are used every day (XFS, NFS, ext4, btrfs, ...) the benefit is very much
worth the cost.

> > I ran this through xfstests and it didn't seem any worse.
> 
> Can you recommend a specific real load workload to stress test your changes?

Not really.  Everything should work just as it did before.  I doubt
you'd be able to see any performance differences with, eg, a kernel
compile.  There's usually a few % gain when enabling large folios.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 01/15] ubifs: Set page uptodate in the correct place

[Matthew Wilcox]

On Tue, Jan 23, 2024 at 10:36:14AM +0800, Zhihao Cheng wrote:
> 在 2024/1/22 22:40, Matthew Wilcox 写道:
> > On Mon, Jan 22, 2024 at 03:22:45PM +0800, Zhihao Cheng wrote:
> > > 在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> > > > Page cache reads are lockless, so setting the freshly allocated page
> > > > uptodate before we've overwritten it with the data it's supposed to have
> > > > in it will allow a simultaneous reader to see old data.  Move the call
> > > > to SetPageUptodate into ubifs_write_end(), which is after we copied the
> > > > new data into the page.
> > > 
> > > This solution looks good to me, and I think 'SetPageUptodate' should be
> > > removed from write_begin_slow(slow path) too.
> > 
> > I didn't bother because we have just read into the page so it is
> > uptodate.  A racing read will see the data from before the write, but
> > that's an acceptable ordering of events.
> > .
> > 
> 
> I can't find where the page is read and set uptodate. I think the
> uninitialized data can be found in following path:

You're right; thanks.  I'd misread the code.  I'll send a new version in
a few hours.

Re: [PATCH 01/15] ubifs: Set page uptodate in the correct place

[Matthew Wilcox]

On Tue, Jan 23, 2024 at 10:36:14AM +0800, Zhihao Cheng wrote:
> 在 2024/1/22 22:40, Matthew Wilcox 写道:
> > On Mon, Jan 22, 2024 at 03:22:45PM +0800, Zhihao Cheng wrote:
> > > 在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
> > > > Page cache reads are lockless, so setting the freshly allocated page
> > > > uptodate before we've overwritten it with the data it's supposed to have
> > > > in it will allow a simultaneous reader to see old data.  Move the call
> > > > to SetPageUptodate into ubifs_write_end(), which is after we copied the
> > > > new data into the page.
> > > 
> > > This solution looks good to me, and I think 'SetPageUptodate' should be
> > > removed from write_begin_slow(slow path) too.
> > 
> > I didn't bother because we have just read into the page so it is
> > uptodate.  A racing read will see the data from before the write, but
> > that's an acceptable ordering of events.
> > .
> > 
> 
> I can't find where the page is read and set uptodate. I think the
> uninitialized data can be found in following path:

You're right; thanks.  I'd misread the code.  I'll send a new version in
a few hours.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/15] ubifs folio conversion

[Zhihao Cheng]

在 2024/1/24 13:01, Matthew Wilcox 写道:
> On Tue, Jan 23, 2024 at 08:33:49AM +0100, Richard Weinberger wrote:
>> Matthew,
>>
>> ----- Ursprüngliche Mail -----
>>> Von: "Matthew Wilcox" <willy@infradead.org>
>>> An: "richard" <richard@nod.at>
>>> CC: "Matthew Wilcox" <willy@infradead.org>, "linux-mtd" <linux-mtd@lists.infradead.org>
>>> Gesendet: Sonntag, 21. Januar 2024 00:08:08
>>> Betreff: [PATCH 00/15] ubifs folio conversion
>>
>>> This patchset converts ubifs to use folios throughout.  I made some
>>> attempt to support large folios, but more work definitely needs to be
>>> done before it can be enabled.  It's not clear to me whether it's worth
>>> doing, so I've left that for you ;-)
>>
>> what is the benefit of large folios?
> 
> For another purpose, I just wrote this up.  Possibly there are more
> benefits, but these are the ones I remembered just now:
> 
> But why should a filesystem put in the effort to support large folios
> (aka multi-page folios)?
> 
>      Lower overheads
>          Fewer calls to the page allocator
>          Shorter LRU list helps vmscan age memory
>          Fewer calls to the page cache for large read()/write()
>          Fewer calls to the architecture code for handling faults in mmaped files
>          Potential use of larger TLB entries (eg contPTE / NAPOT)
>      Being a better kernel citizen
>          The more code using large folios, the easier it is to allocate large folios
> 
> The costs may outweigh the benefits. I would not invest time in making
> iso9660, adfs or efs support large folios. But for filesystems which
> are used every day (XFS, NFS, ext4, btrfs, ...) the benefit is very much
> worth the cost.

Ext4 could gain a good performance based on large folio[1], one 
important reason is that blocks allocation method is changed as batch 
allocation from one by one. There is a difference between UBIFS and ext4 
on IO logic, so I'm not sure how much performance the large folio will 
bring to UBIFS. I think page will be replaced by folio some day, so 
these patches can be a folio switching and a preparation for large folio 
support on UBIFS, personally I think.

[1] https://lwn.net/Articles/956575/

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/15] ubifs folio conversion

[Matthew Wilcox]

On Sat, Jan 20, 2024 at 11:08:08PM +0000, Matthew Wilcox (Oracle) wrote:
> This patchset converts ubifs to use folios throughout.  I made some
> attempt to support large folios, but more work definitely needs to be
> done before it can be enabled.  It's not clear to me whether it's worth
> doing, so I've left that for you ;-)

It's been four weeks ... do I need to do anything to make sure this hits
the next merge window?

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/15] ubifs folio conversion

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Matthew Wilcox" <willy@infradead.org>
> On Sat, Jan 20, 2024 at 11:08:08PM +0000, Matthew Wilcox (Oracle) wrote:
>> This patchset converts ubifs to use folios throughout.  I made some
>> attempt to support large folios, but more work definitely needs to be
>> done before it can be enabled.  It's not clear to me whether it's worth
>> doing, so I've left that for you ;-)
> 
> It's been four weeks ... do I need to do anything to make sure this hits
> the next merge window?

Nope, I'll add it to ubifs/next soon. :-)

Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 00/15] ubifs folio conversion

[Richard Weinberger]

----- Ursprüngliche Mail -----
> ----- Ursprüngliche Mail -----
>> Von: "Matthew Wilcox" <willy@infradead.org>
>> On Sat, Jan 20, 2024 at 11:08:08PM +0000, Matthew Wilcox (Oracle) wrote:
>>> This patchset converts ubifs to use folios throughout.  I made some
>>> attempt to support large folios, but more work definitely needs to be
>>> done before it can be enabled.  It's not clear to me whether it's worth
>>> doing, so I've left that for you ;-)
>> 
>> It's been four weeks ... do I need to do anything to make sure this hits
>> the next merge window?
> 
> Nope, I'll add it to ubifs/next soon. :-)

I'm not sure whether you're checking the MTD patchwork status.
So, patches applied. :)

Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/