Re: [PATCH] quota: fix wrong condition in is_quota_modification()

[Jan Kara <jack@suse.cz>]

On Mon 16-09-19 10:53:08, Chao Yu wrote:
> On 2019/9/12 18:06, Jan Kara wrote:
> > On Wed 11-09-19 17:36:50, Chao Yu wrote:
> >> diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h
> >> index dc905a4ff8d7..bd30acad3a7f 100644
> >> --- a/include/linux/quotaops.h
> >> +++ b/include/linux/quotaops.h
> >> @@ -22,7 +22,7 @@ static inline struct quota_info *sb_dqopt(struct super_block *sb)
> >>  /* i_mutex must being held */
> >>  static inline bool is_quota_modification(struct inode *inode, struct iattr *ia)
> >>  {
> >> -	return (ia->ia_valid & ATTR_SIZE && ia->ia_size != inode->i_size) ||
> >> +	return (ia->ia_valid & ATTR_SIZE && ia->ia_size <= inode->i_size) ||
> >>  		(ia->ia_valid & ATTR_UID && !uid_eq(ia->ia_uid, inode->i_uid)) ||
> >>  		(ia->ia_valid & ATTR_GID && !gid_eq(ia->ia_gid, inode->i_gid));
> >>  }
> > 
> > OK, but your change makes i_size extension not to be quota modification
> 
> I just try to adapt below rules covered with generic/092, which restrict
> to not trim preallocate blocks beyond i_size, in that case, filesystem
> won't change i_blocks.
> 
> 1) truncate(i_size) will trim all blocks past i_size.
> 2) truncate(x) where x > i_size will not trim all blocks past i_size.

Ah, OK.

> However, I'm okay with your change, because there could be filesystems won't
> follow above rule.

Yes, I'm concerned that some filesystem may change i_blocks in some corner
case when growing inode size (e.g. when it decides to convert inode from
inline format to a normal block based format or something like that). So I
don't think the optimization is really worth the chance for breakage.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR