All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Jeffery <djeffery@redhat.com>
To: Max Gurtovoy <mgurtovoy@nvidia.com>
Cc: linux-rdma@vger.kernel.org, target-devel@vger.kernel.org,
	Sagi Grimberg <sagi@grimberg.me>,
	Laurence Oberman <loberman@redhat.com>
Subject: Re: [Patch 0/2] iscsit/isert deadlock prevention under heavy I/O
Date: Mon, 14 Mar 2022 09:57:45 -0400	[thread overview]
Message-ID: <CA+-xHTFCPXe-vALE1ApWdhNOJOByGSgmn5=fF1A_P57zYQGNcQ@mail.gmail.com> (raw)
In-Reply-To: <b97dd278-eedd-1324-1334-78addee204f9@nvidia.com>

On Sun, Mar 13, 2022 at 5:59 AM Max Gurtovoy <mgurtovoy@nvidia.com> wrote:
>
> Hi David,
>
> thanks for the report.
>
> Please check how we fixed that in NVMf in Sagi's commit:
>
> nvmet-rdma: fix possible bogus dereference under heavy load (commit:
> 8407879c4e0d77)
>
> Maybe this can be done in isert and will solve this problem in a simpler
> way.
>
> is it necessary to change max_cmd_sn ?
>
>

Hello,

Sure, there are alternative methods which could fix this immediate
issue. e.g. We could make the command structs for scsi commands get
allocated from a mempool. Is there a particular reason you don't want
to do anything to modify max_cmd_sn behavior?

I didn't do something like this as it seems to me to go against the
intent of the design. It makes the iscsi window mostly meaningless in
some conditions and complicates any allocation path since it now must
gracefully and sanely handle an iscsi_cmd/isert_cmd not existing. I
assume special commands like task-management, logouts, and pings would
need a separate allocation source to keep from being dropped under
memory load.

David Jeffery


  reply	other threads:[~2022-03-14 13:58 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-11 17:57 [Patch 0/2] iscsit/isert deadlock prevention under heavy I/O David Jeffery
2022-03-11 17:57 ` [PATCH 1/2] isert: support for unsolicited NOPIN with no response David Jeffery
2022-03-11 17:57 ` [PATCH 2/2] iscsit: increment max_cmd_sn for isert on command release David Jeffery
2022-03-11 19:08 ` [Patch 0/2] iscsit/isert deadlock prevention under heavy I/O Laurence Oberman
2022-03-13  9:59 ` Max Gurtovoy
2022-03-14 13:57   ` David Jeffery [this message]
2022-03-14 14:52     ` Max Gurtovoy
2022-03-14 15:55       ` David Jeffery
2022-03-14 17:40         ` Laurence Oberman
2022-03-16 10:38           ` Max Gurtovoy
2022-03-16 13:07             ` Laurence Oberman
2022-03-16 14:39               ` Sagi Grimberg
2022-03-16 15:26                 ` Laurence Oberman

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='CA+-xHTFCPXe-vALE1ApWdhNOJOByGSgmn5=fF1A_P57zYQGNcQ@mail.gmail.com' \
    --to=djeffery@redhat.com \
    --cc=linux-rdma@vger.kernel.org \
    --cc=loberman@redhat.com \
    --cc=mgurtovoy@nvidia.com \
    --cc=sagi@grimberg.me \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.