Re: [PATCH] quota: don't let mark_dquot_dirty() fail silently

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
  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




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux