On 3/27/23 5:45 AM, Maurizio Lombardi wrote: > When an iscsi initiator is connected to a target running on the > same machine, the system may trigger a deadlock when working > under memory pressure. > This may happen, for example, when the iscsi rx thread tries to > allocate memory and a memory reclaim is performed, the rx thread may > therefore end up waiting for the initiator to complete I/O operations, > causing a deadlock. > I'm still not sure this patch is the right way to go. Here are my issues: 1. We have a driver for local use, tcm loop. IMO, you should be using that. I don't really buy into the argument about it makes the tooling easier, because the tools have to create a local LIO target. They can just as easily make a loop one vs a iscsi one. For the iscsi discovery/login phase, you don't need to run the iscsiadm command for the loop case so that saves you a step. I would also not want to add another user that needs to dip in the memory pools for skbs on the same system. 2. Block drivers in linux have to be able to make forward progress. 2.A We use mempools, preallocate, etc to make sure we have the resources needed to be able to handle writes out to our device. For the iscsit side of things we use the memalloc related flags to dip into pools. For the LIO side of things, we don't have anything with or without this patch, so the iblock_req allocation could fail for example. 2.B We can't swing back on our self in the main IO path, so we don't use GFP_KERNEL there. For the iscsit side of things and for iscsit's specific implementation this patch handles the LIO side issue. The drawback for the LIO side of things is that it relies on iscsit rx thread submitting and executing IO from that thread. If we ever do something like fix the issue where we can block in the submission path by deferring to the submission thread, then the patch does not work. For the general LIO side of things for loop the patch does not help. I think if I'm wrong about #1, and we are going to support this then we: 1. need to handle the iscsit allocations using whatever we are supposed to use like you are doing in this patch (memalloc_noio_save from the tx/rx thread and/or sk_set_memalloc on the sock). 2. We also should fix the issue of LIO core using the wrong flags for this type of setup so it works for loop and also for iscsit. For this, we should have a gfp_t allocation field or flag on the se_cmd or se_session so LIO core and use the right gfp_t. 3. For the second part of the problem of making forward progress in LIO core/backends by using pools/preallocation then I'm not sure what to do.
