From: michael.christie@oracle.com
To: Maurizio Lombardi <mlombard@redhat.com>, martin.petersen@oracle.com
Cc: target-devel@vger.kernel.org
Subject: Re: [PATCH V2] target: iscsi: use GFP_NOIO with loopback connections
Date: Thu, 30 Mar 2023 15:59:30 -0500 [thread overview]
Message-ID: <4a13281e-6076-c8be-3bb2-341e50f0973f@oracle.com> (raw)
In-Reply-To: <20230327104535.20984-1-mlombard@redhat.com>
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.
prev parent reply other threads:[~2023-03-30 20:59 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-27 10:45 [PATCH V2] target: iscsi: use GFP_NOIO with loopback connections Maurizio Lombardi
2023-03-28 17:15 ` Mike Christie
2023-03-30 20:59 ` michael.christie [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4a13281e-6076-c8be-3bb2-341e50f0973f@oracle.com \
--to=michael.christie@oracle.com \
--cc=martin.petersen@oracle.com \
--cc=mlombard@redhat.com \
--cc=target-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).