Re: [Patch 0/2] iscsit/isert deadlock prevention under heavy I/O

[Max Gurtovoy <mgurtovoy@nvidia.com>]

On 3/14/2022 3:57 PM, David Jeffery wrote:
> 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?

according to the description the command was parsed successful and sent 
to the initiator.

Why do we need to change the window ? it's just a race of putting the 
context back to the pool.

And this race is rare.


>
> 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.

it won't be dropped. It would be allocated dynamically and freed 
(instead of putting it back to the pool).


> David Jeffery
>