linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thorsten Leemhuis <regressions@leemhuis.info>
To: Arun Easi <aeasi@marvell.com>, Tony Battersby <tonyb@cybernetics.com>
Cc: Saurav Kashyap <skashyap@marvell.com>,
	Nilesh Javali <njavali@marvell.com>,
	GR-QLogic-Storage-Upstream@marvell.com,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
	regressions@lists.linux.dev
Subject: Re: [EXT] Re: [REGRESSION] qla2xxx: tape drive not removed after unplug FC cable
Date: Mon, 4 Jul 2022 14:06:06 +0200	[thread overview]
Message-ID: <30feb08e-83d0-34e2-06bb-40f4960c8be4@leemhuis.info> (raw)
In-Reply-To: <alpine.LRH.2.21.9999.2206221557150.4730@mvluser05.qlc.com>

On 23.06.22 01:03, Arun Easi wrote:
> On Wed, 22 Jun 2022, 7:56am, Tony Battersby wrote:
> 
>> On 6/21/22 18:05, Arun Easi wrote:
>>> Thanks for the info. Just to reiterate, you've reported two issues (though 
>>> this log was showing only 1 of them).
>>>
>>> Issue 1 - Tape device never disappears when removed
>>> Issue 2 - When a direct connected tape 1 was replaced with tape 2, tape 2 
>>>           was not discovered.
>>>
>>> For Issue-2, please try the attached patch. This may not be the final fix, 
>>> but wanted to check if that would fix the issue for you.
>>>
>>> For Issue-1, the behavior was intentional, though that behavior needs 
>>> refinement. These tape drives support something called FC sequence level 
>>> error recovery (added in FCP-2), which can make tape I/Os survive even 
>>> across a short cable pull. This is not a simple retry of the I/O, rather a 
>>> retry done at the FC sequence level that gives the IO a better chance of
>>> revival. In other words, the said patch that caused regression, while 
>>> introduces an incorrect reporting of the state of the device, makes backup 
>>> more resilient.
>>>
>>> Now, onto the behavior when device state is reported immediately. What we 
>>> have observed, at least with one tape drive from a major vendor, is that, 
>>> across a device loss and device back case with both the events reported to 
>>> upper layers, the backup operation was getting failed. This is due to a 
>>> REPORT LUNS command being issued during device reappearance reporting 
>>> (fc_remote_port_add -> SCSI scan), which the tape drive was not expecting 
>>> and caused the backup to fail.
>>>
>>> I know that some tape drives do not support multiple commands to it at the 
>>> same time, but not sure if that is still the norm these days.
>>>
>>> So, perhaps one way to make the behavior better, is to either report the 
>>> disappearing device a bit delayed or have intelligence added in SCSI scan 
>>> to detect ongoing tape IO operations and delay/avoid the REPORT LUNs. 
>>> Former is a more contained (in the LLD) fix.
>>>
>>> Regards,
>>> -Arun
>>
>> Your patch does fix Issue-2 for me.  For Issue-1, it would be fine with
>> me if qla2xxx reported device removal to the upper level a bit delayed,
>> as you said.
>>
> 
> Thanks for testing and verifying the patch.

BTW, that patch should have 'Link:' tags pointing to all reports about
this issue, e.g. the start of this thread.

These tags are important, as they allow others to look into the
backstory now and years from now. That is why they should be placed in
cases like this, as Documentation/process/submitting-patches.rst and
Documentation/process/5.Posting.rst explain in more detail.
Additionally, my regression tracking bot ‘regzbot’ relies on these tags
to automatically connect reports with patches that are posted or
committed to fix the reported issue. BTW, let me tell regzbot to monitor
this thread:

> We will post the patch upstream after due testing.
That was more than two weeks ago now and I didn't see any progress. Or
did I miss it?

Reminder, things should take this long. For details see the section
"Prioritize work on fixing regressions" in this document:
https://docs.kernel.org/process/handling-regressions.html

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

#regzbot poke

  reply	other threads:[~2022-07-04 12:06 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-25 22:03 [REGRESSION] qla2xxx: tape drive not removed after unplug FC cable Tony Battersby
2022-05-28  0:27 ` Arun Easi
2022-06-20  6:56   ` Thorsten Leemhuis
2022-06-20 14:33     ` Tony Battersby
2022-06-21 22:05       ` [EXT] " Arun Easi
2022-06-22 14:56         ` Tony Battersby
2022-06-22 23:03           ` Arun Easi
2022-07-04 12:06             ` Thorsten Leemhuis [this message]
2022-07-06 17:29               ` Arun Easi

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=30feb08e-83d0-34e2-06bb-40f4960c8be4@leemhuis.info \
    --to=regressions@leemhuis.info \
    --cc=GR-QLogic-Storage-Upstream@marvell.com \
    --cc=aeasi@marvell.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=njavali@marvell.com \
    --cc=regressions@lists.linux.dev \
    --cc=skashyap@marvell.com \
    --cc=tonyb@cybernetics.com \
    /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 NNTP newsgroup(s).