* [PATCH] nilfs2: add logic for the case of file growing (old size <= new size) in nilfs_truncate() @ 2013-04-24 11:44 Vyacheslav Dubeyko 2013-04-24 15:48 ` Ryusuke Konishi 0 siblings, 1 reply; 4+ messages in thread From: Vyacheslav Dubeyko @ 2013-04-24 11:44 UTC (permalink / raw) To: linux-nilfs; +Cc: Ryusuke Konishi, linux-fsdevel, Andrew Morton From: Vyacheslav Dubeyko <slava@dubeyko.com> Subject: [PATCH] nilfs2: add logic for the case of file growing (old size <= new size) in nilfs_truncate() There are situations when nilfs_truncate() is called with new value of i_size that is greater than old one. This patch adds logic for such case. Signed-off-by: Vyacheslav Dubeyko <slava@dubeyko.com> CC: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp> --- fs/nilfs2/inode.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c index 6b49f14..5ccaace 100644 --- a/fs/nilfs2/inode.c +++ b/fs/nilfs2/inode.c @@ -698,6 +698,36 @@ void nilfs_truncate(struct inode *inode) blocksize = sb->s_blocksize; blkoff = (inode->i_size + blocksize - 1) >> sb->s_blocksize_bits; + + if (blkoff > inode->i_blocks) { + int err; + struct address_space *mapping = inode->i_mapping; + struct page *page; + void *fsdata; + loff_t size = inode->i_size; + + err = pagecache_write_begin(NULL, mapping, size, 0, + AOP_FLAG_UNINTERRUPTIBLE, + &page, &fsdata); + if (err) { + printk(KERN_ERR + "NILFS: pagecache_write_begin() failed: err %d", + err); + return; + } + err = pagecache_write_end(NULL, mapping, size, + 0, 0, page, fsdata); + if (err < 0) { + printk(KERN_ERR + "NILFS: pagecache_write_end() failed: err %d", + err); + return; + } + mark_inode_dirty(inode); + return; + } else if (blkoff == inode->i_blocks) + return; + nilfs_transaction_begin(sb, &ti, 0); /* never fails */ block_truncate_page(inode->i_mapping, inode->i_size, nilfs_get_block); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] nilfs2: add logic for the case of file growing (old size <= new size) in nilfs_truncate() 2013-04-24 11:44 [PATCH] nilfs2: add logic for the case of file growing (old size <= new size) in nilfs_truncate() Vyacheslav Dubeyko @ 2013-04-24 15:48 ` Ryusuke Konishi 2013-04-25 9:58 ` Vyacheslav Dubeyko 0 siblings, 1 reply; 4+ messages in thread From: Ryusuke Konishi @ 2013-04-24 15:48 UTC (permalink / raw) To: Vyacheslav Dubeyko Cc: linux-nilfs-u79uwXL29TY76Z2rM5mHXA, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Andrew Morton Hi Vyacheslav, On Wed, 24 Apr 2013 15:44:26 +0400, Vyacheslav Dubeyko wrote: > From: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org> > Subject: [PATCH] nilfs2: add logic for the case of file growing (old size <= new size) in nilfs_truncate() > > There are situations when nilfs_truncate() is called with new value of i_size that is greater than old one. This patch adds logic for such case. > > Signed-off-by: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org> > CC: Ryusuke Konishi <konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org> Did you confirm that nilfs_truncate has real problem in such situtation? I think hole blocks should be appended in that case. Doesn't the current implementation work so? With regards, Ryusuke Konishi > --- > fs/nilfs2/inode.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c > index 6b49f14..5ccaace 100644 > --- a/fs/nilfs2/inode.c > +++ b/fs/nilfs2/inode.c > @@ -698,6 +698,36 @@ void nilfs_truncate(struct inode *inode) > > blocksize = sb->s_blocksize; > blkoff = (inode->i_size + blocksize - 1) >> sb->s_blocksize_bits; > + > + if (blkoff > inode->i_blocks) { > + int err; > + struct address_space *mapping = inode->i_mapping; > + struct page *page; > + void *fsdata; > + loff_t size = inode->i_size; > + > + err = pagecache_write_begin(NULL, mapping, size, 0, > + AOP_FLAG_UNINTERRUPTIBLE, > + &page, &fsdata); > + if (err) { > + printk(KERN_ERR > + "NILFS: pagecache_write_begin() failed: err %d", > + err); > + return; > + } > + err = pagecache_write_end(NULL, mapping, size, > + 0, 0, page, fsdata); > + if (err < 0) { > + printk(KERN_ERR > + "NILFS: pagecache_write_end() failed: err %d", > + err); > + return; > + } > + mark_inode_dirty(inode); > + return; > + } else if (blkoff == inode->i_blocks) > + return; > + > nilfs_transaction_begin(sb, &ti, 0); /* never fails */ > > block_truncate_page(inode->i_mapping, inode->i_size, nilfs_get_block); > -- > 1.7.9.5 > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nilfs2: add logic for the case of file growing (old size <= new size) in nilfs_truncate() 2013-04-24 15:48 ` Ryusuke Konishi @ 2013-04-25 9:58 ` Vyacheslav Dubeyko 2013-04-26 17:19 ` Ryusuke Konishi 0 siblings, 1 reply; 4+ messages in thread From: Vyacheslav Dubeyko @ 2013-04-25 9:58 UTC (permalink / raw) To: Ryusuke Konishi; +Cc: linux-nilfs, linux-fsdevel, Andrew Morton On Thu, 2013-04-25 at 00:48 +0900, Ryusuke Konishi wrote: > Hi Vyacheslav, > On Wed, 24 Apr 2013 15:44:26 +0400, Vyacheslav Dubeyko wrote: > > From: Vyacheslav Dubeyko <slava@dubeyko.com> > > Subject: [PATCH] nilfs2: add logic for the case of file growing (old size <= new size) in nilfs_truncate() > > > > There are situations when nilfs_truncate() is called with new value of i_size that is greater than old one. This patch adds logic for such case. > > > > Signed-off-by: Vyacheslav Dubeyko <slava@dubeyko.com> > > CC: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp> > > Did you confirm that nilfs_truncate has real problem in such situtation? > I haven't any reproducing path that can reveal real problem for such situation. But I think that, as minimum, it makes sense to return from nilfs_truncate() without any activity for the case of blkoff == inode->i_blocks. > I think hole blocks should be appended in that case. > > Doesn't the current implementation work so? > As I understand, in current NILFS2 implementation nilfs_truncate() results in calling nilfs_get_block() with create flag equals by zero. The nilfs_get_block() simply returns without any activity for the case of file growing: 135 } else if (ret == -ENOENT) { 136 /* not found is not error (e.g. hole); must return without 137 the mapped state flag. */ 138 ; 139 } Suggested implementation calls nilfs_get_block() with create flag equals by 1, in result. So, it will allocate the block really. If the real allocation is not desirable then I am wrong with suggested implementation for the case of blkoff > inode->i_blocks. Could you share your vision about it? With the best regards, Vyacheslav Dubeyko. > > With regards, > Ryusuke Konishi > > > --- > > fs/nilfs2/inode.c | 30 ++++++++++++++++++++++++++++++ > > 1 file changed, 30 insertions(+) > > > > diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c > > index 6b49f14..5ccaace 100644 > > --- a/fs/nilfs2/inode.c > > +++ b/fs/nilfs2/inode.c > > @@ -698,6 +698,36 @@ void nilfs_truncate(struct inode *inode) > > > > blocksize = sb->s_blocksize; > > blkoff = (inode->i_size + blocksize - 1) >> sb->s_blocksize_bits; > > + > > + if (blkoff > inode->i_blocks) { > > + int err; > > + struct address_space *mapping = inode->i_mapping; > > + struct page *page; > > + void *fsdata; > > + loff_t size = inode->i_size; > > + > > + err = pagecache_write_begin(NULL, mapping, size, 0, > > + AOP_FLAG_UNINTERRUPTIBLE, > > + &page, &fsdata); > > + if (err) { > > + printk(KERN_ERR > > + "NILFS: pagecache_write_begin() failed: err %d", > > + err); > > + return; > > + } > > + err = pagecache_write_end(NULL, mapping, size, > > + 0, 0, page, fsdata); > > + if (err < 0) { > > + printk(KERN_ERR > > + "NILFS: pagecache_write_end() failed: err %d", > > + err); > > + return; > > + } > > + mark_inode_dirty(inode); > > + return; > > + } else if (blkoff == inode->i_blocks) > > + return; > > + > > nilfs_transaction_begin(sb, &ti, 0); /* never fails */ > > > > block_truncate_page(inode->i_mapping, inode->i_size, nilfs_get_block); > > -- > > 1.7.9.5 > > > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nilfs2: add logic for the case of file growing (old size <= new size) in nilfs_truncate() 2013-04-25 9:58 ` Vyacheslav Dubeyko @ 2013-04-26 17:19 ` Ryusuke Konishi 0 siblings, 0 replies; 4+ messages in thread From: Ryusuke Konishi @ 2013-04-26 17:19 UTC (permalink / raw) To: Vyacheslav Dubeyko Cc: linux-nilfs-u79uwXL29TY76Z2rM5mHXA, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Andrew Morton On Thu, 25 Apr 2013 13:58:19 +0400, Vyacheslav Dubeyko wrote: > On Thu, 2013-04-25 at 00:48 +0900, Ryusuke Konishi wrote: >> Hi Vyacheslav, >> On Wed, 24 Apr 2013 15:44:26 +0400, Vyacheslav Dubeyko wrote: >> > From: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org> >> > Subject: [PATCH] nilfs2: add logic for the case of file growing (old size <= new size) in nilfs_truncate() >> > >> > There are situations when nilfs_truncate() is called with new value of i_size that is greater than old one. This patch adds logic for such case. >> > >> > Signed-off-by: Vyacheslav Dubeyko <slava-yeENwD64cLxBDgjK7y7TUQ@public.gmane.org> >> > CC: Ryusuke Konishi <konishi.ryusuke-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org> >> >> Did you confirm that nilfs_truncate has real problem in such situtation? >> > > I haven't any reproducing path that can reveal real problem for such > situation. But I think that, as minimum, it makes sense to return from > nilfs_truncate() without any activity for the case of blkoff == > inode->i_blocks. block_truncate_page() must be called even if blkoff == inode->i_blocks because it handles marginal block. >> I think hole blocks should be appended in that case. >> >> Doesn't the current implementation work so? >> > > As I understand, in current NILFS2 implementation nilfs_truncate() > results in calling nilfs_get_block() with create flag equals by zero. > The nilfs_get_block() simply returns without any activity for the case > of file growing: > > 135 } else if (ret == -ENOENT) { > 136 /* not found is not error (e.g. hole); must return without > 137 the mapped state flag. */ > 138 ; > 139 } > That is the desired behavior. > Suggested implementation calls nilfs_get_block() with create flag equals > by 1, in result. So, it will allocate the block really. If the real > allocation is not desirable then I am wrong with suggested > implementation for the case of blkoff > inode->i_blocks. > > Could you share your vision about it? As I commented on the previous mail, we don't have to do the real allocation for the case of blkoff > inode->i_blocks. We just have to set up inode so that the extended part is regarded as hole blocks. Hole blocks are read as null bytes ('\0'), and this complies with the spec of truncate/ftruncate. Regards, Ryusuke Konishi > With the best regards, > Vyacheslav Dubeyko. > >> >> With regards, >> Ryusuke Konishi >> >> > --- >> > fs/nilfs2/inode.c | 30 ++++++++++++++++++++++++++++++ >> > 1 file changed, 30 insertions(+) >> > >> > diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c >> > index 6b49f14..5ccaace 100644 >> > --- a/fs/nilfs2/inode.c >> > +++ b/fs/nilfs2/inode.c >> > @@ -698,6 +698,36 @@ void nilfs_truncate(struct inode *inode) >> > >> > blocksize = sb->s_blocksize; >> > blkoff = (inode->i_size + blocksize - 1) >> sb->s_blocksize_bits; >> > + >> > + if (blkoff > inode->i_blocks) { >> > + int err; >> > + struct address_space *mapping = inode->i_mapping; >> > + struct page *page; >> > + void *fsdata; >> > + loff_t size = inode->i_size; >> > + >> > + err = pagecache_write_begin(NULL, mapping, size, 0, >> > + AOP_FLAG_UNINTERRUPTIBLE, >> > + &page, &fsdata); >> > + if (err) { >> > + printk(KERN_ERR >> > + "NILFS: pagecache_write_begin() failed: err %d", >> > + err); >> > + return; >> > + } >> > + err = pagecache_write_end(NULL, mapping, size, >> > + 0, 0, page, fsdata); >> > + if (err < 0) { >> > + printk(KERN_ERR >> > + "NILFS: pagecache_write_end() failed: err %d", >> > + err); >> > + return; >> > + } >> > + mark_inode_dirty(inode); >> > + return; >> > + } else if (blkoff == inode->i_blocks) >> > + return; >> > + >> > nilfs_transaction_begin(sb, &ti, 0); /* never fails */ >> > >> > block_truncate_page(inode->i_mapping, inode->i_size, nilfs_get_block); >> > -- >> > 1.7.9.5 >> > >> > >> > >> > -- >> > To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in >> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org >> > More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in >> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-04-26 17:19 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-04-24 11:44 [PATCH] nilfs2: add logic for the case of file growing (old size <= new size) in nilfs_truncate() Vyacheslav Dubeyko 2013-04-24 15:48 ` Ryusuke Konishi 2013-04-25 9:58 ` Vyacheslav Dubeyko 2013-04-26 17:19 ` Ryusuke Konishi
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).