From: OGAWA Hirofumi Subject: Re: [PATCH] fat: fix data past EOF resulting from fsx testsuite Date: Mon, 10 Nov 2014 01:01:33 +0900 Message-ID: <87sihsnz1e.fsf@devron.myhome.or.jp> References: <000001cff3fe$effbc0d0$cff34270$@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]:50305 "EHLO mail.parknet.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751712AbaKIQBk (ORCPT ); Sun, 9 Nov 2014 11:01:40 -0500 In-Reply-To: <000001cff3fe$effbc0d0$cff34270$@samsung.com> (Namjae Jeon's message of "Thu, 30 Oct 2014 14:04:08 +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. Good catch. However implementation looks like strange a bit. > @@ -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); We can't use "static" for fat_block_truncate_page()? > --- a/fs/fat/file.c > +++ b/fs/fat/file.c > @@ -526,6 +526,8 @@ int fat_setattr(struct dentry *dentry, struct iattr *attr) > down_write(&MSDOS_I(inode)->truncate_lock); > truncate_setsize(inode, attr->ia_size); > fat_truncate_blocks(inode, attr->ia_size); > + if (inode->i_size & (inode->i_sb->s_blocksize - 1)) > + fat_block_truncate_page(inode, inode->i_size); > up_write(&MSDOS_I(inode)->truncate_lock); > } Let's remove block alignment, because it was done in block_truncate_page(). Then, move fat_block_truncate_page() before truncate_setsize(). Looks like other fs calls it before truncate_setsize(). Thanks. -- OGAWA Hirofumi