Re: [PATCH 1/2] ext4, project: expand inode extra size if possible

[Miao Xie <miaoxie@huawei.com>]

on 2017/7/3 at 9:16, Wang Shilong wrote:
> 
> ________________________________________
> From: Miao Xie [miaoxie@huawei.com]
> Sent: Friday, June 30, 2017 16:48
> To: Wang Shilong; linux-ext4@vger.kernel.org
> Cc: tytso@mit.edu; Li Xi; yi.zhang@huawei.com; adilger@dilger.ca; Wang Shilong; Shuichi Ihara
> Subject: Re: [PATCH 1/2] ext4, project: expand inode extra size if possible
> 
> on 2017/6/30 at 11:51, Wang Shilong wrote:
>> when upgrading from old format, try to set project id
>> to old file first time, it will return EOVERFLOW, but if
>> that file is dirtied(touch etc), changing project id will
> <...SNIP...>
> 
> ext4_expand_extra_isize should be invoked after ext4_reserve_inode_write.
> 
> And I think it is better to restructure ext4_expand_extra_isize by moving NO_EXPAND check,
> nojournal check and journal credits extend into it, and then we just if i_projid is in
> 
> ---->I agreed we could move NO_EXPAND check, but i don't think it good idea to move journal
> credits extend to it, jbd2_extend_journal() might fail, and we'd better avoid it.
> 
> For changing projectid, we could know how many credits before start transaction..

I found most check in set_projectid is the same as ext4_mark_inode_dirty, so I think it's better
to move those checks into ext4_expand_extra_isize to avoid the reduplicated code.

And I don't think jbd2_journal_extend's failure is a big problem, because we just invoke it once
most of the time, and even if we fail to extend the inode because of jbd2_journal_extend's failure,
the metadata is still safe, so it is unnecessary to change many codes in set_projectid, or
we will impact readability of the code.

> 
> the inode or not, if not, invoke ext4_expand_extra_isize. (don't forget to do cleanup for
> ext4_mark_inode_dirty), And then we can remove many check in the above code.
> 
> 
> ---->what do you mean cleanup for ext4_mark_inode_dirty()? I supposed you mean don't
> call ext4_mark_iloc_dirty() if extend fail? i think that is expected, even inode extend fail,
> if ext4_mark_inode_dirty() is called, it means inode is dirtied already before we call the
> function.

I means if we move the checks into ext4_expand_extra_isize, we should do cleanup for ext4_mark_inode_dirty.
(ext4_mark_iloc_dirty() shoud be invoked even if extend fail, because we need update the on-disk inode)

Thanks
Miao

> Thanks,
> Shilong
> 
> 
> .
>