Received-SPF: pass (google.com: domain of linux-kernel+bounces-136306-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <a2f7ae45-9e13-4b0c-a16f-637324565e14@huawei.com>
Date: Tue, 9 Apr 2024 14:31:14 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] quota: don't let mark_dquot_dirty() fail silently
To: Chao Yu <chao@kernel.org>, <jack@suse.com>
CC: <linux-fsdevel@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20240407073128.3489785-1-chao@kernel.org>
Content-Language: en-US
From: "wangjianjian (C)" <wangjianjian3@huawei.com>
In-Reply-To: <20240407073128.3489785-1-chao@kernel.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

On 2024/4/7 15:31, Chao Yu wrote:
> mark_dquot_dirty() will callback to specified filesystem function,
> it may fail due to any reasons, however, no caller will check return
> value of mark_dquot_dirty(), so, it may fail silently, let's print
> one line message for such case.
> 
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>   fs/quota/dquot.c | 23 +++++++++++++----------
>   1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index dacbee455c03..c5df7863942a 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -399,21 +399,20 @@ int dquot_mark_dquot_dirty(struct dquot *dquot)
>   EXPORT_SYMBOL(dquot_mark_dquot_dirty);
>   
>   /* Dirtify all the dquots - this can block when journalling */
> -static inline int mark_all_dquot_dirty(struct dquot __rcu * const *dquots)
> +static inline void mark_all_dquot_dirty(struct dquot __rcu * const *dquots)
>   {
> -	int ret, err, cnt;
> +	int ret, cnt;
>   	struct dquot *dquot;
>   
> -	ret = err = 0;
>   	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
>   		dquot = srcu_dereference(dquots[cnt], &dquot_srcu);
> -		if (dquot)
> -			/* Even in case of error we have to continue */
> -			ret = mark_dquot_dirty(dquot);
> -		if (!err)
> -			err = ret;
> +		if (!dquot)
> +			continue;
> +		ret = mark_dquot_dirty(dquot);
> +		if (ret < 0)
> +			quota_error(dquot->dq_sb,
> +				"mark_all_dquot_dirty fails, ret: %d", ret);
>   	}
> -	return err;
>   }
>   
>   static inline void dqput_all(struct dquot **dquot)
> @@ -2725,6 +2724,7 @@ static int do_set_dqblk(struct dquot *dquot, struct qc_dqblk *di)
>   {
>   	struct mem_dqblk *dm = &dquot->dq_dqb;
>   	int check_blim = 0, check_ilim = 0;
> +	int ret;
>   	struct mem_dqinfo *dqi = &sb_dqopt(dquot->dq_sb)->info[dquot->dq_id.type];
>   
>   	if (di->d_fieldmask & ~VFS_QC_MASK)
> @@ -2807,7 +2807,10 @@ static int do_set_dqblk(struct dquot *dquot, struct qc_dqblk *di)
>   	else
>   		set_bit(DQ_FAKE_B, &dquot->dq_flags);
>   	spin_unlock(&dquot->dq_dqb_lock);
> -	mark_dquot_dirty(dquot);
> +	ret = mark_dquot_dirty(dquot);
Here it overwrite previous error.

> +	if (ret < 0)
> +		quota_error(dquot->dq_sb,
> +			"mark_dquot_dirty fails, ret: %d", ret);
>   
>   	return 0;
>   }
-- 
Regards