Linux-ext4 Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3 00/18] Allow readpage to return a locked page
@ 2020-10-16 16:04 Matthew Wilcox (Oracle)
  2020-10-16 16:04 ` [PATCH v3 11/18] ext4: Tell the VFS that readpage was synchronous Matthew Wilcox (Oracle)
  2020-10-16 16:04 ` [PATCH v3 12/18] ext4: Return error from ext4_readpage Matthew Wilcox (Oracle)
  0 siblings, 2 replies; 6+ messages in thread
From: Matthew Wilcox (Oracle) @ 2020-10-16 16:04 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Matthew Wilcox (Oracle),
	linux-mm, David Howells, Steve French, linux-cifs, Nicolas Pitre,
	Tyler Hicks, ecryptfs, Theodore Ts'o, Andreas Dilger,
	linux-ext4, Miklos Szeredi, Hans de Goede

I've dropped the conversion of readpage implementations to synchronous
from this patchset.  I realised I'd neglected the requirement for making
the sleep killable, and that turns out to be more convoluted to fix.

So these patches add:
 - An error-path bugfix for cachefiles.
 - The ability for the filesystem to tell the caller of ->readpage
   that the read was successful and the page was not unlocked.  This is
   a performance improvement for some scenarios that I think are rare.
 - Mildly improved error handling for ext4.

v2: https://lore.kernel.org/linux-fsdevel/20201009143104.22673-1-willy@infradead.org/
v1: https://lore.kernel.org/linux-fsdevel/20200917151050.5363-1-willy@infradead.org/
Matthew Wilcox (Oracle) (18):
  cachefiles: Handle readpage error correctly
  swap: Call aops->readahead if appropriate
  fs: Add AOP_UPDATED_PAGE return value
  mm/filemap: Inline wait_on_page_read into its one caller
  9p: Tell the VFS that readpage was synchronous
  afs: Tell the VFS that readpage was synchronous
  ceph: Tell the VFS that readpage was synchronous
  cifs: Tell the VFS that readpage was synchronous
  cramfs: Tell the VFS that readpage was synchronous
  ecryptfs: Tell the VFS that readpage was synchronous
  ext4: Tell the VFS that readpage was synchronous
  ext4: Return error from ext4_readpage
  fuse: Tell the VFS that readpage was synchronous
  hostfs: Tell the VFS that readpage was synchronous
  jffs2: Tell the VFS that readpage was synchronous
  ubifs: Tell the VFS that readpage was synchronous
  udf: Tell the VFS that readpage was synchronous
  vboxsf: Tell the VFS that readpage was synchronous

 Documentation/filesystems/locking.rst |  7 +++---
 Documentation/filesystems/vfs.rst     | 21 +++++++++++------
 fs/9p/vfs_addr.c                      |  6 ++++-
 fs/afs/file.c                         |  3 ++-
 fs/buffer.c                           | 15 +++++++-----
 fs/cachefiles/rdwr.c                  |  9 ++++++++
 fs/ceph/addr.c                        |  9 ++++----
 fs/cifs/file.c                        |  8 +++++--
 fs/cramfs/inode.c                     |  5 ++--
 fs/ecryptfs/mmap.c                    | 11 +++++----
 fs/ext4/inline.c                      |  9 +++++---
 fs/ext4/readpage.c                    | 24 +++++++++++--------
 fs/fuse/file.c                        |  2 ++
 fs/hostfs/hostfs_kern.c               |  2 ++
 fs/jffs2/file.c                       |  6 +++--
 fs/ubifs/file.c                       | 16 ++++++++-----
 fs/udf/file.c                         |  3 +--
 fs/vboxsf/file.c                      |  2 ++
 include/linux/fs.h                    |  5 ++++
 mm/filemap.c                          | 33 +++++++++++++--------------
 mm/page_io.c                          | 27 ++++++++++++++++++++--
 mm/readahead.c                        |  3 ++-
 22 files changed, 151 insertions(+), 75 deletions(-)

-- 
2.28.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 11/18] ext4: Tell the VFS that readpage was synchronous
  2020-10-16 16:04 [PATCH v3 00/18] Allow readpage to return a locked page Matthew Wilcox (Oracle)
@ 2020-10-16 16:04 ` Matthew Wilcox (Oracle)
  2020-10-18 14:24   ` Theodore Y. Ts'o
  2020-10-16 16:04 ` [PATCH v3 12/18] ext4: Return error from ext4_readpage Matthew Wilcox (Oracle)
  1 sibling, 1 reply; 6+ messages in thread
From: Matthew Wilcox (Oracle) @ 2020-10-16 16:04 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Matthew Wilcox (Oracle),
	linux-mm, Theodore Ts'o, Andreas Dilger, linux-ext4

The ext4 inline data readpage implementation was already synchronous,
so use AOP_UPDATED_PAGE to avoid cycling the page lock.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/ext4/inline.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 75c97bca0815..2a489243e4de 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -490,7 +490,8 @@ static int ext4_read_inline_page(struct inode *inode, struct page *page)
 	zero_user_segment(page, len, PAGE_SIZE);
 	SetPageUptodate(page);
 	brelse(iloc.bh);
-
+	if (ret >= 0)
+		return AOP_UPDATED_PAGE;
 out:
 	return ret;
 }
@@ -514,12 +515,14 @@ int ext4_readpage_inline(struct inode *inode, struct page *page)
 	else if (!PageUptodate(page)) {
 		zero_user_segment(page, 0, PAGE_SIZE);
 		SetPageUptodate(page);
+		ret = AOP_UPDATED_PAGE;
 	}
 
 	up_read(&EXT4_I(inode)->xattr_sem);
 
-	unlock_page(page);
-	return ret >= 0 ? 0 : ret;
+	if (ret < 0)
+		unlock_page(page);
+	return ret;
 }
 
 static int ext4_convert_inline_data_to_extent(struct address_space *mapping,
-- 
2.28.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 12/18] ext4: Return error from ext4_readpage
  2020-10-16 16:04 [PATCH v3 00/18] Allow readpage to return a locked page Matthew Wilcox (Oracle)
  2020-10-16 16:04 ` [PATCH v3 11/18] ext4: Tell the VFS that readpage was synchronous Matthew Wilcox (Oracle)
@ 2020-10-16 16:04 ` Matthew Wilcox (Oracle)
  2020-10-18 14:25   ` Theodore Y. Ts'o
  1 sibling, 1 reply; 6+ messages in thread
From: Matthew Wilcox (Oracle) @ 2020-10-16 16:04 UTC (permalink / raw)
  To: linux-fsdevel
  Cc: Matthew Wilcox (Oracle),
	linux-mm, Theodore Ts'o, Andreas Dilger, linux-ext4

The error returned from ext4_map_blocks() was being discarded, leading
to the generic -EIO being returned to userspace.  Now ext4 can return
more precise errors.

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

diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
index f014c5e473a9..00a024f3a954 100644
--- a/fs/ext4/readpage.c
+++ b/fs/ext4/readpage.c
@@ -237,7 +237,7 @@ int ext4_mpage_readpages(struct inode *inode,
 	sector_t blocks[MAX_BUF_PER_PAGE];
 	unsigned page_block;
 	struct block_device *bdev = inode->i_sb->s_bdev;
-	int length;
+	int length, err = 0;
 	unsigned relative_block = 0;
 	struct ext4_map_blocks map;
 	unsigned int nr_pages = rac ? readahead_count(rac) : 1;
@@ -301,14 +301,9 @@ int ext4_mpage_readpages(struct inode *inode,
 				map.m_lblk = block_in_file;
 				map.m_len = last_block - block_in_file;
 
-				if (ext4_map_blocks(NULL, inode, &map, 0) < 0) {
-				set_error_page:
-					SetPageError(page);
-					zero_user_segment(page, 0,
-							  PAGE_SIZE);
-					unlock_page(page);
-					goto next_page;
-				}
+				err = ext4_map_blocks(NULL, inode, &map, 0);
+				if (err < 0)
+					goto err;
 			}
 			if ((map.m_flags & EXT4_MAP_MAPPED) == 0) {
 				fully_mapped = 0;
@@ -395,6 +390,15 @@ int ext4_mpage_readpages(struct inode *inode,
 		} else
 			last_block_in_bio = blocks[blocks_per_page - 1];
 		goto next_page;
+	set_error_page:
+		err = -EIO;
+	err:
+		if (rac) {
+			SetPageError(page);
+			zero_user_segment(page, 0, PAGE_SIZE);
+		}
+		unlock_page(page);
+		goto next_page;
 	confused:
 		if (bio) {
 			submit_bio(bio);
@@ -410,7 +414,7 @@ int ext4_mpage_readpages(struct inode *inode,
 	}
 	if (bio)
 		submit_bio(bio);
-	return 0;
+	return err;
 }
 
 int __init ext4_init_post_read_processing(void)
-- 
2.28.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 11/18] ext4: Tell the VFS that readpage was synchronous
  2020-10-16 16:04 ` [PATCH v3 11/18] ext4: Tell the VFS that readpage was synchronous Matthew Wilcox (Oracle)
@ 2020-10-18 14:24   ` Theodore Y. Ts'o
  0 siblings, 0 replies; 6+ messages in thread
From: Theodore Y. Ts'o @ 2020-10-18 14:24 UTC (permalink / raw)
  To: Matthew Wilcox (Oracle)
  Cc: linux-fsdevel, linux-mm, Andreas Dilger, linux-ext4

On Fri, Oct 16, 2020 at 05:04:36PM +0100, Matthew Wilcox (Oracle) wrote:
> The ext4 inline data readpage implementation was already synchronous,
> so use AOP_UPDATED_PAGE to avoid cycling the page lock.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Acked-by: Theodore Ts'o <tytso@mit.edu>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 12/18] ext4: Return error from ext4_readpage
  2020-10-16 16:04 ` [PATCH v3 12/18] ext4: Return error from ext4_readpage Matthew Wilcox (Oracle)
@ 2020-10-18 14:25   ` Theodore Y. Ts'o
  2020-10-18 15:04     ` Matthew Wilcox
  0 siblings, 1 reply; 6+ messages in thread
From: Theodore Y. Ts'o @ 2020-10-18 14:25 UTC (permalink / raw)
  To: Matthew Wilcox (Oracle)
  Cc: linux-fsdevel, linux-mm, Andreas Dilger, linux-ext4

On Fri, Oct 16, 2020 at 05:04:37PM +0100, Matthew Wilcox (Oracle) wrote:
> The error returned from ext4_map_blocks() was being discarded, leading
> to the generic -EIO being returned to userspace.  Now ext4 can return
> more precise errors.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

This change is independent of the synchronous readpage changes,
correct?  Or am I missing something?

Cheers,

					- Ted

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 12/18] ext4: Return error from ext4_readpage
  2020-10-18 14:25   ` Theodore Y. Ts'o
@ 2020-10-18 15:04     ` Matthew Wilcox
  0 siblings, 0 replies; 6+ messages in thread
From: Matthew Wilcox @ 2020-10-18 15:04 UTC (permalink / raw)
  To: Theodore Y. Ts'o; +Cc: linux-fsdevel, linux-mm, Andreas Dilger, linux-ext4

On Sun, Oct 18, 2020 at 10:25:57AM -0400, Theodore Y. Ts'o wrote:
> On Fri, Oct 16, 2020 at 05:04:37PM +0100, Matthew Wilcox (Oracle) wrote:
> > The error returned from ext4_map_blocks() was being discarded, leading
> > to the generic -EIO being returned to userspace.  Now ext4 can return
> > more precise errors.
> > 
> > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> 
> This change is independent of the synchronous readpage changes,
> correct?  Or am I missing something?

It's a step along the way.  If you want to queue it up independently of
the other changes, I see no problem with that.  The requirement to make
a synchronous ->readpage killable is making the conversion quite thorny
and I'm not sure I'm going to get it done this merge window.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-16 16:04 [PATCH v3 00/18] Allow readpage to return a locked page Matthew Wilcox (Oracle)
2020-10-16 16:04 ` [PATCH v3 11/18] ext4: Tell the VFS that readpage was synchronous Matthew Wilcox (Oracle)
2020-10-18 14:24   ` Theodore Y. Ts'o
2020-10-16 16:04 ` [PATCH v3 12/18] ext4: Return error from ext4_readpage Matthew Wilcox (Oracle)
2020-10-18 14:25   ` Theodore Y. Ts'o
2020-10-18 15:04     ` Matthew Wilcox

Linux-ext4 Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ext4/0 linux-ext4/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ext4 linux-ext4/ https://lore.kernel.org/linux-ext4 \
		linux-ext4@vger.kernel.org
	public-inbox-index linux-ext4

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ext4


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git