Message-ID: <caff1add88d1e46c962ee52aae441769450c4dd9.camel@redhat.com>
Subject: Re: [PATCH] scsi: target: iscsi: set memalloc_noio with loopback
 network connections
From:   Laurence Oberman <loberman@redhat.com>
To:     David Jeffery <djeffery@redhat.com>, target-devel@vger.kernel.org
Cc:     "Martin K . Petersen" <martin.petersen@oracle.com>,
        Mike Christie <michael.christie@oracle.com>,
        Maurizio Lombardi <mlombard@redhat.com>,
        linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Date:   Wed, 08 Feb 2023 15:58:01 -0500
In-Reply-To: <20230208200957.14073-1-djeffery@redhat.com>
References: <20230208200957.14073-1-djeffery@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 2023-02-08 at 15:09 -0500, David Jeffery wrote:
> If an admin connects an iscsi initiator to an iscsi target on the
> same
> system, the iscsi connection is vulnerable to deadlocks during memory
> allocations. Memory allocations in the target task accepting the I/O
> from
> the initiator can wait on the initiator's I/O when the system is
> under
> memory pressure, causing a deadlock situation between the iscsi
> target and
> initiator.
> 
> When in this configuration, the deadlock scenario can be avoided by
> use of
> GFP_NOIO allocations. Rather than force all configurations to use
> NOIO,
> memalloc_noio_save/restore can be used to force GFP_NOIO allocations
> only
> when in this loopback configuration.
> 
> Signed-off-by: David Jeffery <djeffery@redhat.com>
> ---
>  drivers/target/iscsi/iscsi_target.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/target/iscsi/iscsi_target.c
> b/drivers/target/iscsi/iscsi_target.c
> index baf4da7bb3b4..a68e47e2cdf9 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -16,6 +16,7 @@
>  #include <linux/vmalloc.h>
>  #include <linux/idr.h>
>  #include <linux/delay.h>
> +#include <linux/sched/mm.h>
>  #include <linux/sched/signal.h>
>  #include <asm/unaligned.h>
>  #include <linux/inet.h>
> @@ -4168,7 +4169,10 @@ int iscsi_target_rx_thread(void *arg)
>  {
>  	int rc;
>  	struct iscsit_conn *conn = arg;
> +	struct dst_entry *dst;
>  	bool conn_freed = false;
> +	bool loopback = false;
> +	unsigned int flags;
>  
>  	/*
>  	 * Allow ourselves to be interrupted by SIGINT so that a
> @@ -4186,8 +4190,25 @@ int iscsi_target_rx_thread(void *arg)
>  	if (!conn->conn_transport->iscsit_get_rx_pdu)
>  		return 0;
>  
> +	/*
> +	 * If the iscsi connection is over a loopback device from using
> +	 * iscsi and iscsit on the same system, we need to set
> memalloc_noio to
> +	 * prevent memory allocation deadlocks between target and
> initiator.
> +	 */
> +	rcu_read_lock();
> +	dst = rcu_dereference(conn->sock->sk->sk_dst_cache);
> +	if (dst && dst->dev && dst->dev->flags & IFF_LOOPBACK)
> +		loopback = true;
> +	rcu_read_unlock();
> +
> +	if (loopback)
> +		flags = memalloc_noio_save();
> +
>  	conn->conn_transport->iscsit_get_rx_pdu(conn);
>  
> +	if (loopback)
> +		memalloc_noio_restore(flags);
> +
>  	if (!signal_pending(current))
>  		atomic_set(&conn->transport_failed, 1);
>  	iscsit_take_action_for_connection_exit(conn, &conn_freed);


I had mentioned to Mike that this was already tested at a large
customer and in our labs and resolved the deadlocks .

Regards
Laurence Oberman