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@xxxxxxxxxxx>
> > 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@xxxxxxxxxxx>
> > CC: Ryusuke Konishi <konishi.ryusuke@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx
> > 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@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
