Re: [PATCH] tune2fs: do not update quota when not needed

["Theodore Ts'o" <tytso@mit.edu>]

On Thu, Sep 12, 2024 at 11:15:58AM +0200, Jan Kara wrote:
> On Fri 30-08-24 11:59:21, Gwendal Grignou wrote:
> > Enabling quota is expensive: All inodes in the filesystem are scanned.
> > Only do it when the requested quota configuration does not match the
> > existing configuration.
> > 
> > Test:
> > Add a tiny patch to print out when core of function
> > handle_quota_options() is triggered.
> > Issue commands:
> > truncate -s 1G unused ; mkfs.ext4 unused
> > 
> > | commands                                                | trigger | comments
> > +---------------------------------------------------------+---------+---------
> > | tune2fs -Qusrquota,grpquota -Qprjquota -O quota unused  | Y       |
> >                   Quota not set at formatting.
> > | tune2fs -Qusrquota,grpquota -Qprjquota -O quota unused  | N       |
> >                   Already set just above
> > | tune2fs -Qusrquota,grpquota -Q^prjquota -O quota unused | Y       |
> >                   Disabling a quota option always force a deep look.

I'm not sure why disabling a quota should always "force a deep look"?
What was your thinking behind this change?

> Why don't you do it like:
> 
> 	for (qtype = 0 ;qtype < MAXQUOTAS; qtype++) {
> 		if (quota_enable[qtype] == QOPT_ENABLE &&
> 		     *quota_sb_inump(fs->super, qtype) == 0) {
> 			/* Need to enable this quota type. */
> 			break;
> 		}
> 		if (quota_enable[qtype] == QOPT_DISABLE &&
> 		    *quota_sb_inump(fs->super, qtype)) {
> 			/* Need to disable this quota type. */
> 			break;
> 		}
> 	}
> 	if (qtype == MAXQUOTAS) {
> 		/* Nothing to do. */
> 		return 0;
> 	}

I like Jan's suggestion here, and this could easily just be done at
the beginning of handle_quota_options(), replacing what's currently
there:

	for (qtype = 0 ; qtype < MAXQUOTAS; qtype++)
		if (quota_enable[qtype] != 0)
			break;
	if (qtype == MAXQUOTAS)
		/* Nothing to do. */
		return 0;

It results in simpler and cleaner code, I think....

   	      	      	  	  	  - Ted