Re: [PATCH 1/2] loop: respect REQ_NOWAIT for memory allocation

[Damien Le Moal <dlemoal@kernel.org>]

On 11/16/25 14:43, Chaitanya Kulkarni wrote:
> On 11/15/25 19:50, Damien Le Moal wrote:
>> On 11/16/25 11:52, Chaitanya Kulkarni wrote:
>>>    6. Loop driver:
>>>     loop_queue_rq()
>>>      lo_rw_aio()
>>>       kmalloc_array(..., GFP_NOIO) <-- BLOCKS (REQ_NOWAIT violation)
>>>        -> Should use GFP_NOWAIT when rq->cmd_flags & REQ_NOWAIT
>> Same comment as for zloop. Re-read the code and see that loop_queue_rq() calls
>> loop_queue_work(). That function has a memory allocation that is already marked
>> with GFP_NOWAIT, and that this function does not directly execute lo_rw_aio() as
>> that is done from loop_workfn(), in the work item context.
>> So again, no blocking violation that I can see here.
>> As far as I can tell, this patch is not needed.
>>
> Thanks for pointing that out. Since REQ_NOWAIT is not valid in the
> workqueue, then REQ_NOWAIT flag needs to be cleared before
> handing it over to workqueue ? is that the right interpretation?

No. the queue_rq context does not block, so REQ_NOWAIT is being respected. I do
not see any issue with it. REQ_NOWAIT simply means that ->queue_rq() should not
block. It does not mean that the IO should/will be completed instantaneously...

Did you by any chance trigger a warning or something ? If yes, waht is the
reproducer ?

> 
> e.g.
> 
> loop_queue_rq()
>   loop_queue_work()
>     ...
>     ...
>     rq->cmd_flags &= ~REQ_NOWAIT; <---
>     
>     list_add_tail(&cmd->list_entry, cmd_list);
>     queue_work(lo->workqueue, work);
>     spin_unlock_irq(&lo->lo_work_lock);
> 
> I have read the code [1] and the commit that added the flag [2] as well.
> I could not find any mention of how switching to a workqueue context
> affects the interpretation of REQ_NOWAIT, or whether its scope is
> strictly limited to XXX_queue_rq() in the request lifecycle.
> 
> -ck
> 
> [1]
> 
> fio context =============>>
> 
> loop_queue_rq()
>   loop_queue_work()
>     queue_work(lo->workqueue, work);
> 
> fio ===> workqueue context
> 
> Work queue context =====>>>
> 
> loop_workfn
>   loop_process_work
>    loop_handle_cmd
>     do_req_filebacked()
> 
>      struct request *rq = blk_mq_rq_from_pdu(cmd);
> 
> [2]
> 
>  From 03a07c92a9ed9938d828ca7f1d11b8bc63a7bb89 Mon Sep 17 00:00:00 2001
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> Date: Tue, 20 Jun 2017 07:05:46 -0500
> Subject: [PATCH] block: return on congested block device
> 
> A new bio operation flag REQ_NOWAIT is introduced to identify bio's
> orignating from iocb with IOCB_NOWAIT. This flag indicates
> to return immediately if a request cannot be made instead
> of retrying.
> 
> Stacked devices such as md (the ones with make_request_fn hooks)
> currently are not supported because it may block for housekeeping.
> For example, an md can have a part of the device suspended.
> For this reason, only request based devices are supported.
> In the future, this feature will be expanded to stacked devices
> by teaching them how to handle the REQ_NOWAIT flags.
> 


-- 
Damien Le Moal
Western Digital Research