diff mbox series

[5.4] iomap: fix sub-page uptodate handling

Message ID 20210516150328.2881778-1-willy@infradead.org (mailing list archive)
State Superseded
Headers show
Series [5.4] iomap: fix sub-page uptodate handling | expand

Commit Message

Matthew Wilcox May 16, 2021, 3:03 p.m. UTC
From: Christoph Hellwig <hch@lst.de>

commit 1cea335d1db1ce6ab71b3d2f94a807112b738a0f upstream

bio completions can race when a page spans more than one file system
block.  Add a spinlock to synchronize marking the page uptodate.

Fixes: 9dc55f1389f9 ("iomap: add support for sub-pagesize buffered I/O without buffer heads")
Reported-by: Jan Stancek <jstancek@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/iomap/buffered-io.c | 34 ++++++++++++++++++++++++----------
 include/linux/iomap.h  |  1 +
 2 files changed, 25 insertions(+), 10 deletions(-)

Comments

Greg KH May 17, 2021, 7:42 a.m. UTC | #1
On Sun, May 16, 2021 at 04:03:28PM +0100, Matthew Wilcox (Oracle) wrote:
> From: Christoph Hellwig <hch@lst.de>
> 
> commit 1cea335d1db1ce6ab71b3d2f94a807112b738a0f upstream
> 
> bio completions can race when a page spans more than one file system
> block.  Add a spinlock to synchronize marking the page uptodate.
> 
> Fixes: 9dc55f1389f9 ("iomap: add support for sub-pagesize buffered I/O without buffer heads")
> Reported-by: Jan Stancek <jstancek@redhat.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/iomap/buffered-io.c | 34 ++++++++++++++++++++++++----------
>  include/linux/iomap.h  |  1 +
>  2 files changed, 25 insertions(+), 10 deletions(-)

No s-o-b from you as you did the backport?  :(

Anyway, what about a 4.19.y version?

thanks,

greg k-h
Matthew Wilcox May 17, 2021, 11:17 a.m. UTC | #2
On Mon, May 17, 2021 at 09:42:26AM +0200, Greg KH wrote:
> On Sun, May 16, 2021 at 04:03:28PM +0100, Matthew Wilcox (Oracle) wrote:
> > From: Christoph Hellwig <hch@lst.de>
> > 
> > commit 1cea335d1db1ce6ab71b3d2f94a807112b738a0f upstream
> > 
> > bio completions can race when a page spans more than one file system
> > block.  Add a spinlock to synchronize marking the page uptodate.
> > 
> > Fixes: 9dc55f1389f9 ("iomap: add support for sub-pagesize buffered I/O without buffer heads")
> > Reported-by: Jan Stancek <jstancek@redhat.com>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > Reviewed-by: Dave Chinner <dchinner@redhat.com>
> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/iomap/buffered-io.c | 34 ++++++++++++++++++++++++----------
> >  include/linux/iomap.h  |  1 +
> >  2 files changed, 25 insertions(+), 10 deletions(-)
> 
> No s-o-b from you as you did the backport?  :(

My mistake.  I don't do stable backports of other people's patches very
often.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

> Anyway, what about a 4.19.y version?

I'll look into it and see what I can do.
diff mbox series

Patch

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 80867a1a94f2..5c73751adb2d 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -30,6 +30,7 @@  iomap_page_create(struct inode *inode, struct page *page)
 	iop = kmalloc(sizeof(*iop), GFP_NOFS | __GFP_NOFAIL);
 	atomic_set(&iop->read_count, 0);
 	atomic_set(&iop->write_count, 0);
+	spin_lock_init(&iop->uptodate_lock);
 	bitmap_zero(iop->uptodate, PAGE_SIZE / SECTOR_SIZE);
 
 	/*
@@ -118,25 +119,38 @@  iomap_adjust_read_range(struct inode *inode, struct iomap_page *iop,
 }
 
 static void
-iomap_set_range_uptodate(struct page *page, unsigned off, unsigned len)
+iomap_iop_set_range_uptodate(struct page *page, unsigned off, unsigned len)
 {
 	struct iomap_page *iop = to_iomap_page(page);
 	struct inode *inode = page->mapping->host;
 	unsigned first = off >> inode->i_blkbits;
 	unsigned last = (off + len - 1) >> inode->i_blkbits;
-	unsigned int i;
 	bool uptodate = true;
+	unsigned long flags;
+	unsigned int i;
 
-	if (iop) {
-		for (i = 0; i < PAGE_SIZE / i_blocksize(inode); i++) {
-			if (i >= first && i <= last)
-				set_bit(i, iop->uptodate);
-			else if (!test_bit(i, iop->uptodate))
-				uptodate = false;
-		}
+	spin_lock_irqsave(&iop->uptodate_lock, flags);
+	for (i = 0; i < PAGE_SIZE / i_blocksize(inode); i++) {
+		if (i >= first && i <= last)
+			set_bit(i, iop->uptodate);
+		else if (!test_bit(i, iop->uptodate))
+			uptodate = false;
 	}
 
-	if (uptodate && !PageError(page))
+	if (uptodate)
+		SetPageUptodate(page);
+	spin_unlock_irqrestore(&iop->uptodate_lock, flags);
+}
+
+static void
+iomap_set_range_uptodate(struct page *page, unsigned off, unsigned len)
+{
+	if (PageError(page))
+		return;
+
+	if (page_has_private(page))
+		iomap_iop_set_range_uptodate(page, off, len);
+	else
 		SetPageUptodate(page);
 }
 
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 7aa5d6117936..53b16f104081 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -139,6 +139,7 @@  loff_t iomap_apply(struct inode *inode, loff_t pos, loff_t length,
 struct iomap_page {
 	atomic_t		read_count;
 	atomic_t		write_count;
+	spinlock_t		uptodate_lock;
 	DECLARE_BITMAP(uptodate, PAGE_SIZE / 512);
 };