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

[Chao Yu]

On 2019/9/16 16:23, Jan Kara wrote:
> 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.

Agreed, :)

Thanks,

> 
> 								Honza
>