Dan reports the following smatch detected the following:

  block/blk-cgroup.c:1863 blkcg_schedule_throttle() warn: sleeping in atomic context

caused by blkcg_schedule_throttle() calling blk_put_queue() in an
non-sleepable context.

blk_put_queue() acquired might_sleep() in 63f93fd6fa57 ("block: mark
blk_put_queue as potentially blocking") which transferred the might_sleep()
from blk_free_queue().

blk_free_queue() acquired might_sleep() in e8c7d14ac6c3 ("block: revert back
to synchronous request_queue removal") while turning request_queue removal
synchronous. However, this isn't necessary as nothing in the free path
actually requires sleeping.

It's pretty unusual to require a sleeping context in a put operation and
it's not needed in the first place. Let's drop it.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Dan Carpenter <error27@gmail.com>
Link: https://lkml.kernel.org/r/Y7g3L6fntnTtOm63@kili
Cc: Christoph Hellwig <hch@lst.de>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Fixes: e8c7d14ac6c3 ("block: revert back to synchronous request_queue removal") # v5.9+
--
 block/blk-core.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 9321767470dc..b5098355d8b2 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -283,12 +283,9 @@ static void blk_free_queue(struct request_queue *q)
  *
  * Decrements the refcount of the request_queue and free it when the refcount
  * reaches 0.
- *
- * Context: Can sleep.
  */
 void blk_put_queue(struct request_queue *q)
 {
-	might_sleep();
 	if (refcount_dec_and_test(&q->refs))
 		blk_free_queue(q);
 }

On Fri, 06 Jan 2023 10:34:10 -1000, Tejun Heo wrote:
> Dan reports the following smatch detected the following:
> 
>   block/blk-cgroup.c:1863 blkcg_schedule_throttle() warn: sleeping in atomic context
> 
> caused by blkcg_schedule_throttle() calling blk_put_queue() in an
> non-sleepable context.
> 
> [...]

Applied, thanks!

[1/1] block: Drop spurious might_sleep() from blk_put_queue()
      commit: 49e4d04f0486117ac57a97890eb1db6d52bf82b3

Best regards,
-- 
Jens Axboe

I walked through everything called from blk_free_queue and can't find
anything that sleeps either, so:

Reviewed-by: Christoph Hellwig <hch@lst.de>

I wonder if we could not also revert
d578c770c85233af592e54537f93f3831bde7e9a as I think that was added
to avoid calling blk_put_queue from atomic context.

On Fri, Jan 06, 2023 at 10:34:10AM -1000, Tejun Heo wrote:
> Dan reports the following smatch detected the following:
> 
>   block/blk-cgroup.c:1863 blkcg_schedule_throttle() warn: sleeping in atomic context
> 
> caused by blkcg_schedule_throttle() calling blk_put_queue() in an
> non-sleepable context.
> 
> blk_put_queue() acquired might_sleep() in 63f93fd6fa57 ("block: mark
> blk_put_queue as potentially blocking") which transferred the might_sleep()
> from blk_free_queue().
> 
> blk_free_queue() acquired might_sleep() in e8c7d14ac6c3 ("block: revert back
> to synchronous request_queue removal") while turning request_queue removal
> synchronous. However, this isn't necessary as nothing in the free path
> actually requires sleeping.
> 
> It's pretty unusual to require a sleeping context in a put operation and
> it's not needed in the first place. Let's drop it.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Dan Carpenter <error27@gmail.com>
> Link: https://lkml.kernel.org/r/Y7g3L6fntnTtOm63@kili
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Fixes: e8c7d14ac6c3 ("block: revert back to synchronous request_queue removal") # v5.9+

*tons* has changed since e8c7d14ac6c3 and so the bots might think that
*if* this patch is applied upstream it is justified for older kernels
and I don't think that's yet been verified and doubt it.

And so I think adding a "Fixes" tag is not appropriate here.

First I'd like to hear from Christoph if he agrees with this patch
upstream. For stable, someone would have to do the homework.

  Luis

On Fri, Jan 06, 2023 at 12:45:12PM -0800, Luis Chamberlain wrote:
> > Signed-off-by: Tejun Heo <tj@kernel.org>
> > Reported-by: Dan Carpenter <error27@gmail.com>
> > Link: https://lkml.kernel.org/r/Y7g3L6fntnTtOm63@kili
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Luis Chamberlain <mcgrof@kernel.org>
> > Fixes: e8c7d14ac6c3 ("block: revert back to synchronous request_queue removal") # v5.9+
> 
> *tons* has changed since e8c7d14ac6c3 and so the bots might think that
> *if* this patch is applied upstream it is justified for older kernels
> and I don't think that's yet been verified and doubt it.

If you enable the correct debug option then the might_sleep() causes a
stack trace.  Eventually syzbot will find it.

I would backport it.

regards,
dan carpenter

On 1/6/23 1:45 PM, Luis Chamberlain wrote:
> On Fri, Jan 06, 2023 at 10:34:10AM -1000, Tejun Heo wrote:
>> Dan reports the following smatch detected the following:
>>
>>   block/blk-cgroup.c:1863 blkcg_schedule_throttle() warn: sleeping in atomic context
>>
>> caused by blkcg_schedule_throttle() calling blk_put_queue() in an
>> non-sleepable context.
>>
>> blk_put_queue() acquired might_sleep() in 63f93fd6fa57 ("block: mark
>> blk_put_queue as potentially blocking") which transferred the might_sleep()
>> from blk_free_queue().
>>
>> blk_free_queue() acquired might_sleep() in e8c7d14ac6c3 ("block: revert back
>> to synchronous request_queue removal") while turning request_queue removal
>> synchronous. However, this isn't necessary as nothing in the free path
>> actually requires sleeping.
>>
>> It's pretty unusual to require a sleeping context in a put operation and
>> it's not needed in the first place. Let's drop it.
>>
>> Signed-off-by: Tejun Heo <tj@kernel.org>
>> Reported-by: Dan Carpenter <error27@gmail.com>
>> Link: https://lkml.kernel.org/r/Y7g3L6fntnTtOm63@kili
>> Cc: Christoph Hellwig <hch@lst.de>
>> Cc: Luis Chamberlain <mcgrof@kernel.org>
>> Fixes: e8c7d14ac6c3 ("block: revert back to synchronous request_queue removal") # v5.9+
> 
> *tons* has changed since e8c7d14ac6c3 and so the bots might think that
> *if* this patch is applied upstream it is justified for older kernels
> and I don't think that's yet been verified and doubt it.
> 
> And so I think adding a "Fixes" tag is not appropriate here.
> 
> First I'd like to hear from Christoph if he agrees with this patch
> upstream. For stable, someone would have to do the homework.

Outside of the easily audited paths, the kobj release paths are the
only ones of concern. And I didn't spot anything that sleeps. Looks
fine to me.

-- 
Jens Axboe

On Fri, Jan 06, 2023 at 12:45:12PM -0800, Luis Chamberlain wrote:
> *tons* has changed since e8c7d14ac6c3 and so the bots might think that
> *if* this patch is applied upstream it is justified for older kernels
> and I don't think that's yet been verified and doubt it.

Didn't look like anything relevant changed to me. Besides, all that the
patch does is removing a might_sleep() which can't hurt anything.

-- 
tejun