* [PATCH] xfs: fix sub-page uptodate handling
@ 2019-12-04 17:28 Christoph Hellwig
2019-12-04 20:58 ` Dave Chinner
2019-12-04 21:57 ` Darrick J. Wong
0 siblings, 2 replies; 4+ messages in thread
From: Christoph Hellwig @ 2019-12-04 17:28 UTC (permalink / raw)
To: linux-xfs; +Cc: jstancek
bio complentions 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>
---
fs/iomap/buffered-io.c | 35 +++++++++++++++++++++++++----------
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 512856a88106..340c15400423 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -28,6 +28,7 @@
struct iomap_page {
atomic_t read_count;
atomic_t write_count;
+ spinlock_t uptodate_lock;
DECLARE_BITMAP(uptodate, PAGE_SIZE / 512);
};
@@ -51,6 +52,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);
/*
@@ -139,25 +141,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);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: fix sub-page uptodate handling
2019-12-04 17:28 [PATCH] xfs: fix sub-page uptodate handling Christoph Hellwig
@ 2019-12-04 20:58 ` Dave Chinner
2019-12-04 21:57 ` Darrick J. Wong
1 sibling, 0 replies; 4+ messages in thread
From: Dave Chinner @ 2019-12-04 20:58 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, jstancek
On Wed, Dec 04, 2019 at 06:28:04PM +0100, Christoph Hellwig wrote:
> bio complentions 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>
Looks sane, similar fix to previous completion issues like this.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: fix sub-page uptodate handling
2019-12-04 17:28 [PATCH] xfs: fix sub-page uptodate handling Christoph Hellwig
2019-12-04 20:58 ` Dave Chinner
@ 2019-12-04 21:57 ` Darrick J. Wong
2019-12-05 7:44 ` Christoph Hellwig
1 sibling, 1 reply; 4+ messages in thread
From: Darrick J. Wong @ 2019-12-04 21:57 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, jstancek
On Wed, Dec 04, 2019 at 06:28:04PM +0100, Christoph Hellwig wrote:
> bio complentions 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>
Looks good to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> ---
> fs/iomap/buffered-io.c | 35 +++++++++++++++++++++++++----------
> 1 file changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 512856a88106..340c15400423 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -28,6 +28,7 @@
> struct iomap_page {
> atomic_t read_count;
> atomic_t write_count;
> + spinlock_t uptodate_lock;
> DECLARE_BITMAP(uptodate, PAGE_SIZE / 512);
> };
>
> @@ -51,6 +52,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);
>
> /*
> @@ -139,25 +141,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);
> }
>
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: fix sub-page uptodate handling
2019-12-04 21:57 ` Darrick J. Wong
@ 2019-12-05 7:44 ` Christoph Hellwig
0 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2019-12-05 7:44 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs, jstancek
Btw, the subject should say iomap instead of xfs, sorry.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-12-05 7:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-04 17:28 [PATCH] xfs: fix sub-page uptodate handling Christoph Hellwig
2019-12-04 20:58 ` Dave Chinner
2019-12-04 21:57 ` Darrick J. Wong
2019-12-05 7:44 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).