Messages in this thread | | | Date | Thu, 11 Apr 2024 18:27:40 +0800 | Subject | Re: [PATCH] quota: don't let mark_dquot_dirty() fail silently | From | Chao Yu <> |
| |
On 2024/4/8 22:30, Jan Kara wrote: > On Sun 07-04-24 15:31:28, 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); > > Do you have any practical case you care about? Because in practice the
Actually, no.
> filesystem will usually report if there's some catastrophic error (and the > errors from ->mark_dirty() all mean the filesystem is in unhealthy state). > So this message just adds to the noise in the error log - and e.g. if the > disk goes bad so we cannot write, we could spew a lot of messages like > this.
Agreed,
I guess we can propagate the error to caller rather than printing redundant message in log.
> >> } >> - 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); >> + if (ret < 0) >> + quota_error(dquot->dq_sb, >> + "mark_dquot_dirty fails, ret: %d", ret); > > Here, we can propagate the error back to userspace, which is probably > better than spamming the logs.
Yes, let me submit a new patch for this.
Thanks,
> > Honza
| |