diff mbox series

[05/11] iomap: zero newly allocated mapped blocks

Message ID 20191006154608.24738-6-hch@lst.de (mailing list archive)
State New, archived
Headers show
Series [01/11] iomap: add tracing for the readpage / readpages | expand

Commit Message

Christoph Hellwig Oct. 6, 2019, 3:46 p.m. UTC 
File systems like gfs2 don't support delayed allocations or unwritten
extents and thus allocate normal mapped blocks to fill holes.  To
cover the case of such file systems allocating new blocks to fill holes
also zero out mapped blocks with the new flag.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/iomap/buffered-io.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Dave Chinner Oct. 7, 2019, 9:46 p.m. UTC | #1 
On Sun, Oct 06, 2019 at 05:46:02PM +0200, Christoph Hellwig wrote:
> File systems like gfs2 don't support delayed allocations or unwritten
> extents and thus allocate normal mapped blocks to fill holes.  To
> cover the case of such file systems allocating new blocks to fill holes
> also zero out mapped blocks with the new flag.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  fs/iomap/buffered-io.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 23cc308f971d..4132c0cccb0a 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -207,6 +207,14 @@ iomap_read_inline_data(struct inode *inode, struct page *page,
>  	SetPageUptodate(page);
>  }
>  
> +static inline bool iomap_block_needs_zeroing(struct inode *inode,
> +		struct iomap *iomap, loff_t pos)
> +{
> +	return iomap->type != IOMAP_MAPPED ||
> +		(iomap->flags & IOMAP_F_NEW) ||
> +		pos >= i_size_read(inode);

This is a change of logic - why is the IOMAP_F_NEW check added here
and what bug does it fix?

Cheers,

Dave.
Dave Chinner Oct. 7, 2019, 10:08 p.m. UTC | #2 
On Tue, Oct 08, 2019 at 08:46:32AM +1100, Dave Chinner wrote:
> On Sun, Oct 06, 2019 at 05:46:02PM +0200, Christoph Hellwig wrote:
> > File systems like gfs2 don't support delayed allocations or unwritten
> > extents and thus allocate normal mapped blocks to fill holes.  To
> > cover the case of such file systems allocating new blocks to fill holes
> > also zero out mapped blocks with the new flag.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  fs/iomap/buffered-io.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> > index 23cc308f971d..4132c0cccb0a 100644
> > --- a/fs/iomap/buffered-io.c
> > +++ b/fs/iomap/buffered-io.c
> > @@ -207,6 +207,14 @@ iomap_read_inline_data(struct inode *inode, struct page *page,
> >  	SetPageUptodate(page);
> >  }
> >  
> > +static inline bool iomap_block_needs_zeroing(struct inode *inode,
> > +		struct iomap *iomap, loff_t pos)
> > +{
> > +	return iomap->type != IOMAP_MAPPED ||
> > +		(iomap->flags & IOMAP_F_NEW) ||
> > +		pos >= i_size_read(inode);
> 
> This is a change of logic - why is the IOMAP_F_NEW check added here
> and what bug does it fix?

Sorry, brain-fart here - that's what this patch is adding, it's not
a pure factoring patch.... :/

Cheers,

Dave.
diff mbox series

Patch

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 23cc308f971d..4132c0cccb0a 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -207,6 +207,14 @@  iomap_read_inline_data(struct inode *inode, struct page *page,
 	SetPageUptodate(page);
 }
 
+static inline bool iomap_block_needs_zeroing(struct inode *inode,
+		struct iomap *iomap, loff_t pos)
+{
+	return iomap->type != IOMAP_MAPPED ||
+		(iomap->flags & IOMAP_F_NEW) ||
+		pos >= i_size_read(inode);
+}
+
 static loff_t
 iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 		struct iomap *iomap)
@@ -230,7 +238,7 @@  iomap_readpage_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
 	if (plen == 0)
 		goto done;
 
-	if (iomap->type != IOMAP_MAPPED || pos >= i_size_read(inode)) {
+	if (iomap_block_needs_zeroing(inode, iomap, pos)) {
 		zero_user(page, poff, plen);
 		iomap_set_range_uptodate(page, poff, plen);
 		goto done;
@@ -544,7 +552,7 @@  iomap_read_page_sync(struct inode *inode, loff_t block_start, struct page *page,
 	struct bio_vec bvec;
 	struct bio bio;
 
-	if (iomap->type != IOMAP_MAPPED || block_start >= i_size_read(inode)) {
+	if (iomap_block_needs_zeroing(inode, iomap, block_start)) {
 		zero_user_segments(page, poff, from, to, poff + plen);
 		iomap_set_range_uptodate(page, poff, plen);
 		return 0;