[Cluster-devel] [GFS2 PATCH 2/2] gfs2: limit quota log messages

[Abhijith Das <adas@redhat.com>]

----- Original Message -----
> From: "Bob Peterson" <rpeterso@redhat.com>
> To: "Abhi Das" <adas@redhat.com>
> Cc: cluster-devel at redhat.com
> Sent: Monday, June 1, 2015 11:27:59 AM
> Subject: Re: [Cluster-devel] [GFS2 PATCH 2/2] gfs2: limit quota log messages
> 
> Hi,
> 
> Comment embedded below:
> 
> ----- Original Message -----
> > This patch makes the quota subsystem only report once that a
> > particular user/group has exceeded their allotted quota.
> > 
> > Previously, it was possible for a program to continuously try
> > exceeding quota (despite receiving EDQUOT) and in turn trigger
> > gfs2 to issue a kernel log message about quota exceed. In theory,
> > this could get out of hand and flood the log and the filesystem
> > hosting the log files.
> > 
> > Resolves: rhbz#1174295
> > Signed-off-by: Abhi Das <adas@redhat.com>
> > ---
> >  fs/gfs2/incore.h |  1 +
> >  fs/gfs2/quota.c  | 13 +++++++++----
> >  2 files changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
> > index 58b75ab..304a223 100644
> > --- a/fs/gfs2/incore.h
> > +++ b/fs/gfs2/incore.h
> > @@ -432,6 +432,7 @@ enum {
> >  	QDF_CHANGE		= 1,
> >  	QDF_LOCKED		= 2,
> >  	QDF_REFRESH		= 3,
> > +	QDF_QMSG_QUIET          = 4,
> >  };
> >  
> >  struct gfs2_quota_data {
> > diff --git a/fs/gfs2/quota.c b/fs/gfs2/quota.c
> > index 01f4d40..3dc13b53 100644
> > --- a/fs/gfs2/quota.c
> > +++ b/fs/gfs2/quota.c
> > @@ -649,6 +649,8 @@ static void do_qc(struct gfs2_quota_data *qd, s64
> > change)
> >  		slot_hold(qd);
> >  	}
> >  
> > +	if (change < 0) /* Reset quiet flag if we freed some blocks */
> > +		clear_bit(QDF_QMSG_QUIET, &qd->qd_flags);
> >  	mutex_unlock(&sdp->sd_quota_mutex);
> >  }
> >  
> > @@ -1187,10 +1189,13 @@ int gfs2_quota_check(struct gfs2_inode *ip, kuid_t
> > uid, kgid_t gid,
> >  			/* If no min_target specified or we don't meet
> >  			 * min_target, return -EDQUOT */
> >  			if (!ap->min_target || ap->min_target > ap->allowed) {
> > -				print_message(qd, "exceeded");
> > -				quota_send_warning(qd->qd_id,
> > -						   sdp->sd_vfs->s_dev,
> > -						   QUOTA_NL_BHARDWARN);
> > +				if (!test_bit(QDF_QMSG_QUIET, &qd->qd_flags)) {
> > +					print_message(qd, "exceeded");
> > +					quota_send_warning(qd->qd_id,
> > +							   sdp->sd_vfs->s_dev,
> > +							   QUOTA_NL_BHARDWARN);
> > +					set_bit(QDF_QMSG_QUIET, &qd->qd_flags);
> > +				}
> 
> Looks good, except that this is a perfect place to use test_and_set_bit().
> 
> I don't know the gfs2 quota code like you do, but does the bit also need to
> be cleared if the user's quotas are increased?
> 

That's a good catch. I think even on decrease, it might make sense to un-silence
the warning for the next time the exceed event occurs.

Cheers!
--Abhi