ACK/cmt : [SRU][B][F][PATCH 1/1] scsi: zfcp: Fix panic on ERP timeout for previously dismissed ERP action
Frank Heimes
frank.heimes at canonical.com
Thu Jul 16 15:26:05 UTC 2020
Hi Colin, well, yeah - what we see here is the official message from the
upstream commit.
I thought about summarizing this for the SRU Justification, but then
thought I will eventually end up in just repeating information,
since the bug description in LP covers that level of detail, too (in add.
to the justification).
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1887774
(need to think about improving it next time ...)
On Thu, Jul 16, 2020 at 4:50 PM Colin Ian King <colin.king at canonical.com>
wrote:
> On 16/07/2020 15:44, frank.heimes at canonical.com wrote:
> > From: Steffen Maier <maier at linux.ibm.com>
> >
> > BugLink: https://bugs.launchpad.net/bugs/1887774
> >
> > Suppose that, for unrelated reasons, FSF requests on behalf of recovery
> are
> > very slow and can run into the ERP timeout.
> >
> > In the case at hand, we did adapter recovery to a large degree. However
> > due to the slowness a LUN open is pending so the corresponding fc_rport
> > remains blocked. After fast_io_fail_tmo we trigger close physical port
> > recovery for the port under which the LUN should have been opened. The
> new
> > higher order port recovery dismisses the pending LUN open ERP action and
> > dismisses the pending LUN open FSF request. Such dismissal decouples the
> > ERP action from the pending corresponding FSF request by setting
> > zfcp_fsf_req->erp_action to NULL (among other things)
> > [zfcp_erp_strategy_check_fsfreq()].
> >
> > If now the ERP timeout for the pending open LUN request runs out, we must
> > not use zfcp_fsf_req->erp_action in the ERP timeout handler. This is a
> > problem since v4.15 commit 75492a51568b ("s390/scsi: Convert timers to
> use
> > timer_setup()"). Before that we intentionally only passed zfcp_erp_action
> > as context argument to zfcp_erp_timeout_handler().
> >
> > Note: The lifetime of the corresponding zfcp_fsf_req object continues
> until
> > a (late) response or an (unrelated) adapter recovery.
> >
> > Just like the regular response path ignores dismissed requests
> > [zfcp_fsf_req_complete() => zfcp_fsf_protstatus_eval() => return early]
> the
> > ERP timeout handler now needs to ignore dismissed requests. So simply
> > return early in the ERP timeout handler if the FSF request is marked as
> > dismissed in its status flags. To protect against the race where
> > zfcp_erp_strategy_check_fsfreq() dismisses and sets
> > zfcp_fsf_req->erp_action to NULL after our previous status flag check,
> > return early if zfcp_fsf_req->erp_action is NULL. After all, the former
> > ERP action does not need to be woken up as that was already done as part
> of
> > the dismissal above [zfcp_erp_action_dismiss()].
> >
> > This fixes the following panic due to kernel page fault in IRQ context:
> >
> > Unable to handle kernel pointer dereference in virtual kernel address
> space
> > Failing address: 0000000000000000 TEID: 0000000000000483
> > Fault in home space mode while using kernel ASCE.
> > AS:000009859238c00b R2:00000e3e7ffd000b R3:00000e3e7ffcc007
> S:00000e3e7ffd7000 P:000000000000013d
> > Oops: 0004 ilc:2 [#1] SMP
> > Modules linked in: ...
> > CPU: 82 PID: 311273 Comm: stress Kdump: loaded Tainted: G E
> X ...
> > Hardware name: IBM 8561 T01 701 (LPAR)
> > Krnl PSW : 0404c00180000000 001fffff80549be0 (zfcp_erp_notify+0x40/0xc0
> [zfcp])
> > R:0 T:1 IO:0 EX:0 Key:0 M:1 W:0 P:0 AS:3 CC:0 PM:0 RI:0 EA:3
> > Krnl GPRS: 0000000000000080 00000e3d00000000 00000000000000f0
> 0000000000030000
> > 000000010028e700 000000000400a39c 000000010028e700
> 00000e3e7cf87e02
> > 0000000010000000 0700098591cb67f0 0000000000000000
> 0000000000000000
> > 0000033840e9a000 0000000000000000 001fffe008d6bc18
> 001fffe008d6bbc8
> > Krnl Code: 001fffff80549bd4: a7180000 lhi %r1,0
> > 001fffff80549bd8: 4120a0f0 la %r2,240(%r10)
> > #001fffff80549bdc: a53e0003 llilh %r3,3
> > >001fffff80549be0: ba132000 cs %r1,%r3,0(%r2)
> > 001fffff80549be4: a7740037 brc 7,1fffff80549c52
> > 001fffff80549be8: e320b0180004 lg %r2,24(%r11)
> > 001fffff80549bee: e31020e00004 lg %r1,224(%r2)
> > 001fffff80549bf4: 412020e0 la %r2,224(%r2)
> > Call Trace:
> > [<001fffff80549be0>] zfcp_erp_notify+0x40/0xc0 [zfcp]
> > [<00000985915e26f0>] call_timer_fn+0x38/0x190
> > [<00000985915e2944>] expire_timers+0xfc/0x190
> > [<00000985915e2ac4>] run_timer_softirq+0xec/0x218
> > [<0000098591ca7c4c>] __do_softirq+0x144/0x398
> > [<00000985915110aa>] do_softirq_own_stack+0x72/0x88
> > [<0000098591551b58>] irq_exit+0xb0/0xb8
> > [<0000098591510c6a>] do_IRQ+0x82/0xb0
> > [<0000098591ca7140>] ext_int_handler+0x128/0x12c
> > [<0000098591722d98>] clear_subpage.constprop.13+0x38/0x60
> > ([<000009859172ae4c>] clear_huge_page+0xec/0x250)
> > [<000009859177e7a2>] do_huge_pmd_anonymous_page+0x32a/0x768
> > [<000009859172a712>] __handle_mm_fault+0x88a/0x900
> > [<000009859172a860>] handle_mm_fault+0xd8/0x1b0
> > [<0000098591529ef6>] do_dat_exception+0x136/0x3e8
> > [<0000098591ca6d34>] pgm_check_handler+0x1c8/0x220
> > Last Breaking-Event-Address:
> > [<001fffff80549c88>] zfcp_erp_timeout_handler+0x10/0x18 [zfcp]
> > Kernel panic - not syncing: Fatal exception in interrupt
> >
> > Link:
> https://lore.kernel.org/r/20200623140242.98864-1-maier@linux.ibm.com
> > Fixes: 75492a51568b ("s390/scsi: Convert timers to use timer_setup()")
> > Cc: <stable at vger.kernel.org> #4.15+
> > Reviewed-by: Julian Wiedmann <jwi at linux.ibm.com>
> > Signed-off-by: Steffen Maier <maier at linux.ibm.com>
> > Signed-off-by: Martin K. Petersen <martin.petersen at oracle.com>
> > (cherry picked from commit 936e6b85da0476dd2edac7c51c68072da9fb4ba2)
> > Signed-off-by: Frank Heimes <frank.heimes at canonical.com>
> > ---
> > drivers/s390/scsi/zfcp_erp.c | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/s390/scsi/zfcp_erp.c b/drivers/s390/scsi/zfcp_erp.c
> > index db320dab1fee..79f6e8fb03ca 100644
> > --- a/drivers/s390/scsi/zfcp_erp.c
> > +++ b/drivers/s390/scsi/zfcp_erp.c
> > @@ -577,7 +577,10 @@ static void zfcp_erp_strategy_check_fsfreq(struct
> zfcp_erp_action *act)
> > ZFCP_STATUS_ERP_TIMEDOUT)) {
> > req->status |= ZFCP_STATUS_FSFREQ_DISMISSED;
> > zfcp_dbf_rec_run("erscf_1", act);
> > - req->erp_action = NULL;
> > + /* lock-free concurrent access with
> > + * zfcp_erp_timeout_handler()
> > + */
> > + WRITE_ONCE(req->erp_action, NULL);
> > }
> > if (act->status & ZFCP_STATUS_ERP_TIMEDOUT)
> > zfcp_dbf_rec_run("erscf_2", act);
> > @@ -613,8 +616,14 @@ void zfcp_erp_notify(struct zfcp_erp_action
> *erp_action, unsigned long set_mask)
> > void zfcp_erp_timeout_handler(struct timer_list *t)
> > {
> > struct zfcp_fsf_req *fsf_req = from_timer(fsf_req, t, timer);
> > - struct zfcp_erp_action *act = fsf_req->erp_action;
> > + struct zfcp_erp_action *act;
> >
> > + if (fsf_req->status & ZFCP_STATUS_FSFREQ_DISMISSED)
> > + return;
> > + /* lock-free concurrent access with
> zfcp_erp_strategy_check_fsfreq() */
> > + act = READ_ONCE(fsf_req->erp_action);
> > + if (!act)
> > + return;
> > zfcp_erp_notify(act, ZFCP_STATUS_ERP_TIMEDOUT);
> > }
> >
> >
> Fix looks sane, upstream fix that addresses this issue. The SRU
> justification in the bug report may need a bit of a clean up to match
> the normal SRU template style.
>
> Acked-by: Colin Ian King <colin.king at canonical.com>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20200716/7176aae9/attachment.html>
More information about the kernel-team
mailing list