Re: [PATCH V2] target: iscsi: use GFP_NOIO with loopback connections

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


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

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.

[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux