WTF: patch "[PATCH] quota: Generate warnings for DQUOT_SPACE_NOFAIL allocations" was seriously submitted to be applied to the 4.13-stable tree?

WTF: patch "[PATCH] quota: Generate warnings for DQUOT_SPACE_NOFAIL allocations" was seriously submitted to be applied to the 4.13-stable tree?

[gregkh]

The patch below was submitted to be applied to the 4.13-stable tree.

I fail to see how this patch meets the stable kernel rules as found at
Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to 
<stable@vger.kernel.org> and let me know why this patch should be
applied.  Otherwise, it is now dropped from my patch queues, never to be
seen again.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

>From ac3d79392f8c2728f7600dd32ed88b3a1bfdc1af Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Tue, 10 Oct 2017 14:40:42 +0200
Subject: [PATCH] quota: Generate warnings for DQUOT_SPACE_NOFAIL allocations

Eryu has reported that since commit 7b9ca4c61bc2 "quota: Reduce
contention on dq_data_lock" test generic/233 occasionally fails. This is
caused by the fact that since that commit we don't generate warning and
set grace time for quota allocations that have DQUOT_SPACE_NOFAIL set
(these are for example some metadata allocations in ext4). We need these
allocations to behave regularly wrt warning generation and grace time
setting so fix the code to return to the original behavior.

Reported-and-tested-by: Eryu Guan <eguan@redhat.com>
CC: stable@vger.kernel.org
Fixes: 7b9ca4c61bc278b771fb57d6290a31ab1fc7fdac
Signed-off-by: Jan Kara <jack@suse.cz>

diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 50b0556a124f..52ad15192e72 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -1297,21 +1297,18 @@ static int dquot_add_space(struct dquot *dquot, qsize_t space,
 	spin_lock(&dquot->dq_dqb_lock);
 	if (!sb_has_quota_limits_enabled(sb, dquot->dq_id.type) ||
 	    test_bit(DQ_FAKE_B, &dquot->dq_flags))
-		goto add;
+		goto finish;
 
 	tspace = dquot->dq_dqb.dqb_curspace + dquot->dq_dqb.dqb_rsvspace
 		+ space + rsv_space;
 
-	if (flags & DQUOT_SPACE_NOFAIL)
-		goto add;
-
 	if (dquot->dq_dqb.dqb_bhardlimit &&
 	    tspace > dquot->dq_dqb.dqb_bhardlimit &&
             !ignore_hardlimit(dquot)) {
 		if (flags & DQUOT_SPACE_WARN)
 			prepare_warning(warn, dquot, QUOTA_NL_BHARDWARN);
 		ret = -EDQUOT;
-		goto out;
+		goto finish;
 	}
 
 	if (dquot->dq_dqb.dqb_bsoftlimit &&
@@ -1322,7 +1319,7 @@ static int dquot_add_space(struct dquot *dquot, qsize_t space,
 		if (flags & DQUOT_SPACE_WARN)
 			prepare_warning(warn, dquot, QUOTA_NL_BSOFTLONGWARN);
 		ret = -EDQUOT;
-		goto out;
+		goto finish;
 	}
 
 	if (dquot->dq_dqb.dqb_bsoftlimit &&
@@ -1338,13 +1335,21 @@ static int dquot_add_space(struct dquot *dquot, qsize_t space,
 			 * be always printed
 			 */
 			ret = -EDQUOT;
-			goto out;
+			goto finish;
 		}
 	}
-add:
-	dquot->dq_dqb.dqb_rsvspace += rsv_space;
-	dquot->dq_dqb.dqb_curspace += space;
-out:
+finish:
+	/*
+	 * We have to be careful and go through warning generation & grace time
+	 * setting even if DQUOT_SPACE_NOFAIL is set. That's why we check it
+	 * only here...
+	 */
+	if (flags & DQUOT_SPACE_NOFAIL)
+		ret = 0;
+	if (!ret) {
+		dquot->dq_dqb.dqb_rsvspace += rsv_space;
+		dquot->dq_dqb.dqb_curspace += space;
+	}
 	spin_unlock(&dquot->dq_dqb_lock);
 	return ret;
 }

Re: WTF: patch "[PATCH] quota: Generate warnings for DQUOT_SPACE_NOFAIL allocations" was seriously submitted to be applied to the 4.13-stable tree?

[Jan Kara]

Hi,

On Sun 15-10-17 15:34:33, gregkh@linuxfoundation.org wrote:
> The patch below was submitted to be applied to the 4.13-stable tree.
> 
> I fail to see how this patch meets the stable kernel rules as found at
> Documentation/process/stable-kernel-rules.rst.
> 
> I could be totally wrong, and if so, please respond to 
> <stable@vger.kernel.org> and let me know why this patch should be
> applied.  Otherwise, it is now dropped from my patch queues, never to be
> seen again.

So I guess your complaint is that the problem is not serious enough to
warrant a stable fix? My thinking was:

1) This is a userspace visible regression caused by the below mentioned
   commit. 
2) It can result in quota grace times not to be obeyed properly although in
   practice it would be hard to come up with a scenario where this would
   seriously matter (as they are likely to be setup properly on the next
   write).

When reviewing the decision, I agree that my CC to stable was boundary at
best so I'm fine with you just dropping the patch. Thanks for pushing back!

								Honza

> ------------------ original commit in Linus's tree ------------------
> 
> From ac3d79392f8c2728f7600dd32ed88b3a1bfdc1af Mon Sep 17 00:00:00 2001
> From: Jan Kara <jack@suse.cz>
> Date: Tue, 10 Oct 2017 14:40:42 +0200
> Subject: [PATCH] quota: Generate warnings for DQUOT_SPACE_NOFAIL allocations
> 
> Eryu has reported that since commit 7b9ca4c61bc2 "quota: Reduce
> contention on dq_data_lock" test generic/233 occasionally fails. This is
> caused by the fact that since that commit we don't generate warning and
> set grace time for quota allocations that have DQUOT_SPACE_NOFAIL set
> (these are for example some metadata allocations in ext4). We need these
> allocations to behave regularly wrt warning generation and grace time
> setting so fix the code to return to the original behavior.
> 
> Reported-and-tested-by: Eryu Guan <eguan@redhat.com>
> CC: stable@vger.kernel.org
> Fixes: 7b9ca4c61bc278b771fb57d6290a31ab1fc7fdac
> Signed-off-by: Jan Kara <jack@suse.cz>
> 
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 50b0556a124f..52ad15192e72 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -1297,21 +1297,18 @@ static int dquot_add_space(struct dquot *dquot, qsize_t space,
>  	spin_lock(&dquot->dq_dqb_lock);
>  	if (!sb_has_quota_limits_enabled(sb, dquot->dq_id.type) ||
>  	    test_bit(DQ_FAKE_B, &dquot->dq_flags))
> -		goto add;
> +		goto finish;
>  
>  	tspace = dquot->dq_dqb.dqb_curspace + dquot->dq_dqb.dqb_rsvspace
>  		+ space + rsv_space;
>  
> -	if (flags & DQUOT_SPACE_NOFAIL)
> -		goto add;
> -
>  	if (dquot->dq_dqb.dqb_bhardlimit &&
>  	    tspace > dquot->dq_dqb.dqb_bhardlimit &&
>              !ignore_hardlimit(dquot)) {
>  		if (flags & DQUOT_SPACE_WARN)
>  			prepare_warning(warn, dquot, QUOTA_NL_BHARDWARN);
>  		ret = -EDQUOT;
> -		goto out;
> +		goto finish;
>  	}
>  
>  	if (dquot->dq_dqb.dqb_bsoftlimit &&
> @@ -1322,7 +1319,7 @@ static int dquot_add_space(struct dquot *dquot, qsize_t space,
>  		if (flags & DQUOT_SPACE_WARN)
>  			prepare_warning(warn, dquot, QUOTA_NL_BSOFTLONGWARN);
>  		ret = -EDQUOT;
> -		goto out;
> +		goto finish;
>  	}
>  
>  	if (dquot->dq_dqb.dqb_bsoftlimit &&
> @@ -1338,13 +1335,21 @@ static int dquot_add_space(struct dquot *dquot, qsize_t space,
>  			 * be always printed
>  			 */
>  			ret = -EDQUOT;
> -			goto out;
> +			goto finish;
>  		}
>  	}
> -add:
> -	dquot->dq_dqb.dqb_rsvspace += rsv_space;
> -	dquot->dq_dqb.dqb_curspace += space;
> -out:
> +finish:
> +	/*
> +	 * We have to be careful and go through warning generation & grace time
> +	 * setting even if DQUOT_SPACE_NOFAIL is set. That's why we check it
> +	 * only here...
> +	 */
> +	if (flags & DQUOT_SPACE_NOFAIL)
> +		ret = 0;
> +	if (!ret) {
> +		dquot->dq_dqb.dqb_rsvspace += rsv_space;
> +		dquot->dq_dqb.dqb_curspace += space;
> +	}
>  	spin_unlock(&dquot->dq_dqb_lock);
>  	return ret;
>  }
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: WTF: patch "[PATCH] quota: Generate warnings for DQUOT_SPACE_NOFAIL allocations" was seriously submitted to be applied to the 4.13-stable tree?

[Greg KH]

On Mon, Oct 16, 2017 at 12:42:02PM +0200, Jan Kara wrote:
> Hi,
> 
> On Sun 15-10-17 15:34:33, gregkh@linuxfoundation.org wrote:
> > The patch below was submitted to be applied to the 4.13-stable tree.
> > 
> > I fail to see how this patch meets the stable kernel rules as found at
> > Documentation/process/stable-kernel-rules.rst.
> > 
> > I could be totally wrong, and if so, please respond to 
> > <stable@vger.kernel.org> and let me know why this patch should be
> > applied.  Otherwise, it is now dropped from my patch queues, never to be
> > seen again.
> 
> So I guess your complaint is that the problem is not serious enough to
> warrant a stable fix? My thinking was:
> 
> 1) This is a userspace visible regression caused by the below mentioned
>    commit. 
> 2) It can result in quota grace times not to be obeyed properly although in
>    practice it would be hard to come up with a scenario where this would
>    seriously matter (as they are likely to be setup properly on the next
>    write).


I would like to say I was thinking deep thoughts like this, but what I
really was looking at was this in the patch:

> > Fixes: 7b9ca4c61bc278b771fb57d6290a31ab1fc7fdac

Which showd up in 4.14-rc1, and as such, doesn't make much sense for
4.13-stable :)

thanks,

greg k-h

Re: WTF: patch "[PATCH] quota: Generate warnings for DQUOT_SPACE_NOFAIL allocations" was seriously submitted to be applied to the 4.13-stable tree?

[Jan Kara]

On Mon 16-10-17 14:48:59, Greg KH wrote:
> On Mon, Oct 16, 2017 at 12:42:02PM +0200, Jan Kara wrote:
> > Hi,
> > 
> > On Sun 15-10-17 15:34:33, gregkh@linuxfoundation.org wrote:
> > > The patch below was submitted to be applied to the 4.13-stable tree.
> > > 
> > > I fail to see how this patch meets the stable kernel rules as found at
> > > Documentation/process/stable-kernel-rules.rst.
> > > 
> > > I could be totally wrong, and if so, please respond to 
> > > <stable@vger.kernel.org> and let me know why this patch should be
> > > applied.  Otherwise, it is now dropped from my patch queues, never to be
> > > seen again.
> > 
> > So I guess your complaint is that the problem is not serious enough to
> > warrant a stable fix? My thinking was:
> > 
> > 1) This is a userspace visible regression caused by the below mentioned
> >    commit. 
> > 2) It can result in quota grace times not to be obeyed properly although in
> >    practice it would be hard to come up with a scenario where this would
> >    seriously matter (as they are likely to be setup properly on the next
> >    write).
> 
> 
> I would like to say I was thinking deep thoughts like this, but what I
> really was looking at was this in the patch:
> 
> > > Fixes: 7b9ca4c61bc278b771fb57d6290a31ab1fc7fdac
> 
> Which showd up in 4.14-rc1, and as such, doesn't make much sense for
> 4.13-stable :)

Bah :) Somehow I thought it has been merged already to 4.13-rc1.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR