| Message ID | 20200824145511.10500-2-willy@infradead.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | THP iomap patches for 5.10 | expand |
On Mon, Aug 24, 2020 at 03:55:02PM +0100, Matthew Wilcox (Oracle) wrote: > If iomap_unshare_actor() unshares to an inline iomap, the page was > not being flushed. block_write_end() and __iomap_write_end() already > contain flushes, so adding it to iomap_write_end_inline() seems like > the best place. That means we can remove it from iomap_write_actor(). > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > fs/iomap/buffered-io.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) looks good. Reviewed-by: Dave Chinner <dchinner@redhat.com> Cheers, Dave.
On Mon, Aug 24, 2020 at 03:55:02PM +0100, Matthew Wilcox (Oracle) wrote: > If iomap_unshare_actor() unshares to an inline iomap, the page was > not being flushed. block_write_end() and __iomap_write_end() already > contain flushes, so adding it to iomap_write_end_inline() seems like > the best place. That means we can remove it from iomap_write_actor(). > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Seems reasonable to me... Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/iomap/buffered-io.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index bcfc288dba3f..cffd575e57b6 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -715,6 +715,7 @@ iomap_write_end_inline(struct inode *inode, struct page *page, > { > void *addr; > > + flush_dcache_page(page); > WARN_ON_ONCE(!PageUptodate(page)); > BUG_ON(pos + copied > PAGE_SIZE - offset_in_page(iomap->inline_data)); > > @@ -811,8 +812,6 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data, > > copied = iov_iter_copy_from_user_atomic(page, i, offset, bytes); > > - flush_dcache_page(page); > - > status = iomap_write_end(inode, pos, bytes, copied, page, iomap, > srcmap); > if (unlikely(status < 0)) > -- > 2.28.0 >
On Mon, Aug 24, 2020 at 03:55:02PM +0100, Matthew Wilcox (Oracle) wrote: > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index bcfc288dba3f..cffd575e57b6 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -715,6 +715,7 @@ iomap_write_end_inline(struct inode *inode, struct page *page, > { > void *addr; > > + flush_dcache_page(page); > WARN_ON_ONCE(!PageUptodate(page)); > BUG_ON(pos + copied > PAGE_SIZE - offset_in_page(iomap->inline_data)); Please move the call down below the asserts. Otherwise looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index bcfc288dba3f..cffd575e57b6 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -715,6 +715,7 @@ iomap_write_end_inline(struct inode *inode, struct page *page, { void *addr; + flush_dcache_page(page); WARN_ON_ONCE(!PageUptodate(page)); BUG_ON(pos + copied > PAGE_SIZE - offset_in_page(iomap->inline_data)); @@ -811,8 +812,6 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data, copied = iov_iter_copy_from_user_atomic(page, i, offset, bytes); - flush_dcache_page(page); - status = iomap_write_end(inode, pos, bytes, copied, page, iomap, srcmap); if (unlikely(status < 0))
If iomap_unshare_actor() unshares to an inline iomap, the page was not being flushed. block_write_end() and __iomap_write_end() already contain flushes, so adding it to iomap_write_end_inline() seems like the best place. That means we can remove it from iomap_write_actor(). Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/iomap/buffered-io.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)