Re: [rfc][patch 4/4] fs: tmpfs, ext2 use new truncate

[Nick Piggin <npiggin@suse.de>]

On Tue, Jul 07, 2009 at 12:38:29PM -0400, Christoph Hellwig wrote:
> I'd still prefer this to be split into one patch for shmem, and one for
> ext2 to make bisecting easier.

Definitely agreed. I would prefer to even send individual fs patches
to respective maintainers so they can do their specific review and
QA on them and merge them as appropriate. The core change is
nicely back compatible, so there is no reason to tie it to the fs
patches.

 
> > @@ -68,7 +70,7 @@ void ext2_delete_inode (struct inode * i
> >  
> >  	inode->i_size = 0;
> >  	if (inode->i_blocks)
> > -		ext2_truncate (inode);
> > +		ext2_truncate_blocks(inode, 0);
> >  	ext2_free_inode (inode);
> >  
> >  	return;
> 
> > -void ext2_truncate(struct inode *inode)
> > +static void ext2_truncate_blocks(struct inode *inode, loff_t offset)
> >  {
> >  	__le32 *i_data = EXT2_I(inode)->i_data;
> >  	struct ext2_inode_info *ei = EXT2_I(inode);
> > @@ -1032,27 +1074,8 @@ void ext2_truncate(struct inode *inode)
> >  	int n;
> >  	long iblock;
> >  	unsigned blocksize;
> > -
> > -	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
> > -	    S_ISLNK(inode->i_mode)))
> > -		return;
> > -	if (ext2_inode_is_fast_symlink(inode))
> > -		return;
> > -	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
> > -		return;
> > -
> 
> We can't move this to the caller easily.  ext2_delete_inode gets
> called for all inodes, but we only want to go on truncating for the
> limited set that passes this check.

Hmm, shouldn't they have no ->i_blocks in that case?


> > -	if (mapping_is_xip(inode->i_mapping))
> > -		xip_truncate_page(inode->i_mapping, inode->i_size);
> > -	else if (test_opt(inode->i_sb, NOBH))
> > -		nobh_truncate_page(inode->i_mapping,
> > -				inode->i_size, ext2_get_block);
> > -	else
> > -		block_truncate_page(inode->i_mapping,
> > -				inode->i_size, ext2_get_block);
> 
> The patch header should have an explanation for why we don't need this
> anymore for the various existing callers.

OK. I guess it's not needed when blocks were completely outside
of i_size or the inode no longer has references and all blocks
will be freed. I will add the description.