on 2017/7/3 at 9:16, Wang Shilong wrote: > > ________________________________________ > From: Miao Xie [miaoxie@xxxxxxxxxx] > Sent: Friday, June 30, 2017 16:48 > To: Wang Shilong; linux-ext4@xxxxxxxxxxxxxxx > Cc: tytso@xxxxxxx; Li Xi; yi.zhang@xxxxxxxxxx; adilger@xxxxxxxxx; 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 > > > . >
