On Wed, 2023-02-08 at 15:58 -0500, Laurence Oberman wrote:
> 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@xxxxxxxxxx>
> > ---
> > 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
>
Tested-by: Laurence Oberman <loberman@xxxxxxxxxx>
Reviewed-by: Laurence Oberman <loberman@xxxxxxxxxx>
I hate to nag here but we have a pressing customer issue and are keen
to get others to weigh in here.
Regards
Laurence
Thanks
Laurence
