From: OGAWA Hirofumi Subject: Re: [PATCH v2] fat: fix data past EOF resulting from fsx testsuite Date: Wed, 12 Nov 2014 19:09:32 +0900 Message-ID: <87a93w20ir.fsf@devron.myhome.or.jp> References: <009a01cffe5f$ba714f50$2f53edf0$@samsung.com> Mime-Version: 1.0 Content-Type: text/plain Cc: Andrew Morton , linux-fsdevel@vger.kernel.org To: Namjae Jeon Return-path: Received: from mail.parknet.co.jp ([210.171.160.6]:50921 "EHLO mail.parknet.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751904AbaKLKJg (ORCPT ); Wed, 12 Nov 2014 05:09:36 -0500 In-Reply-To: <009a01cffe5f$ba714f50$2f53edf0$@samsung.com> (Namjae Jeon's message of "Wed, 12 Nov 2014 19:02:11 +0900") Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Namjae Jeon writes: > When running FSX with direct I/O mode, fsx resulted in DATA past EOF issues. > > fsx ./file2 -Z -r 4096 -w 4096 > ... > .. > truncating to largest ever: 0x907c > fallocating to largest ever: 0x11137 > truncating to largest ever: 0x2c6fe > truncating to largest ever: 0x2cfdf > fallocating to largest ever: 0x40000 > Mapped Read: non-zero data past EOF (0x18628) page offset 0x629 is 0x2a4e > ... > .. > > The reason being, it is doing a truncate down, but the zeroing does > not happen on the last block boundary when offset is not aligned. > Even though it calls truncate_setsize()->truncate_inode_pages()-> > truncate_inode_pages_range() and considers the partial zeroout but it > retrieves the page using find_lock_page() - which only looks the page in > the cache. So, zeroing out does not happen in case of direct IO. > > Make a truncate page based around block_truncate_page for FAT filesystem and > invoke that helper to zerout in case the offset is not aligned with > the blocksize. Acked-by: OGAWA Hirofumi Thanks. > Signed-off-by: Namjae Jeon > Signed-off-by: Amit Sahrawat > --- > fs/fat/fat.h | 1 + > fs/fat/file.c | 3 +++ > fs/fat/inode.c | 12 ++++++++++++ > 3 files changed, 16 insertions(+) > > diff --git a/fs/fat/fat.h b/fs/fat/fat.h > index eff027d..61801da 100644 > --- a/fs/fat/fat.h > +++ b/fs/fat/fat.h > @@ -374,6 +374,7 @@ extern int fat_file_fsync(struct file *file, loff_t start, loff_t end, > int datasync); > > /* fat/inode.c */ > +extern int fat_block_truncate_page(struct inode *inode, loff_t from); > extern void fat_attach(struct inode *inode, loff_t i_pos); > extern void fat_detach(struct inode *inode); > extern struct inode *fat_iget(struct super_block *sb, loff_t i_pos); > diff --git a/fs/fat/file.c b/fs/fat/file.c > index f2c73ae..757fc20 100644 > --- a/fs/fat/file.c > +++ b/fs/fat/file.c > @@ -523,6 +523,9 @@ int fat_setattr(struct dentry *dentry, struct iattr *attr) > } > > if (attr->ia_valid & ATTR_SIZE) { > + error = fat_block_truncate_page(inode, attr->ia_size); > + if (error) > + goto out; > down_write(&MSDOS_I(inode)->truncate_lock); > truncate_setsize(inode, attr->ia_size); > fat_truncate_blocks(inode, attr->ia_size); > diff --git a/fs/fat/inode.c b/fs/fat/inode.c > index f362796..9bf44eb 100644 > --- a/fs/fat/inode.c > +++ b/fs/fat/inode.c > @@ -339,6 +339,18 @@ static sector_t _fat_bmap(struct address_space *mapping, sector_t block) > return blocknr; > } > > +/* > + * fat_block_truncate_page() zeroes out a mapping from file offset `from' > + * up to the end of the block which corresponds to `from'. > + * This is required during truncate to physically zeroout the tail end > + * of that block so it doesn't yield old data if the file is later grown. > + * Also, avoid causing failure from fsx for cases of "data past EOF" > + */ > +int fat_block_truncate_page(struct inode *inode, loff_t from) > +{ > + return block_truncate_page(inode->i_mapping, from, fat_get_block); > +} > + > static const struct address_space_operations fat_aops = { > .readpage = fat_readpage, > .readpages = fat_readpages, -- OGAWA Hirofumi