public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 4.14 1/6] parisc: sba: Fix compile warning wrt list of SBA devices
@ 2023-09-24 13:20 Sasha Levin
  2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 2/6] parisc: iosapic.c: Fix sparse warnings Sasha Levin
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Sasha Levin @ 2023-09-24 13:20 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Helge Deller, Sasha Levin, James.Bottomley, airlied, linux-parisc,
	dri-devel

From: Helge Deller <deller@gmx.de>

[ Upstream commit eb3255ee8f6f4691471a28fbf22db5e8901116cd ]

Fix this makecheck warning:
drivers/parisc/sba_iommu.c:98:19: warning: symbol 'sba_list'
	was not declared. Should it be static?

Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/parisc/include/asm/ropes.h | 3 +++
 drivers/char/agp/parisc-agp.c   | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/parisc/include/asm/ropes.h b/arch/parisc/include/asm/ropes.h
index 8e51c775c80a6..62399c7ea94a1 100644
--- a/arch/parisc/include/asm/ropes.h
+++ b/arch/parisc/include/asm/ropes.h
@@ -86,6 +86,9 @@ struct sba_device {
 	struct ioc		ioc[MAX_IOC];
 };
 
+/* list of SBA's in system, see drivers/parisc/sba_iommu.c */
+extern struct sba_device *sba_list;
+
 #define ASTRO_RUNWAY_PORT	0x582
 #define IKE_MERCED_PORT		0x803
 #define REO_MERCED_PORT		0x804
diff --git a/drivers/char/agp/parisc-agp.c b/drivers/char/agp/parisc-agp.c
index 1d5510cb6db4e..1962ff624b7c5 100644
--- a/drivers/char/agp/parisc-agp.c
+++ b/drivers/char/agp/parisc-agp.c
@@ -385,8 +385,6 @@ find_quicksilver(struct device *dev, void *data)
 static int __init
 parisc_agp_init(void)
 {
-	extern struct sba_device *sba_list;
-
 	int err = -1;
 	struct parisc_device *sba = NULL, *lba = NULL;
 	struct lba_device *lbadev = NULL;
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH AUTOSEL 4.14 2/6] parisc: iosapic.c: Fix sparse warnings
  2023-09-24 13:20 [PATCH AUTOSEL 4.14 1/6] parisc: sba: Fix compile warning wrt list of SBA devices Sasha Levin
@ 2023-09-24 13:20 ` Sasha Levin
  2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 3/6] parisc: irq: Make irq_stack_union static to avoid sparse warning Sasha Levin
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2023-09-24 13:20 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Helge Deller, Sasha Levin, James.Bottomley, linux-parisc

From: Helge Deller <deller@gmx.de>

[ Upstream commit 927c6c8aa27c284a799b8c18784e37d3373af908 ]

Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/parisc/iosapic.c         | 4 ++--
 drivers/parisc/iosapic_private.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/parisc/iosapic.c b/drivers/parisc/iosapic.c
index eb9137faccf74..4cc08d13b82fa 100644
--- a/drivers/parisc/iosapic.c
+++ b/drivers/parisc/iosapic.c
@@ -216,9 +216,9 @@ static inline void iosapic_write(void __iomem *iosapic, unsigned int reg, u32 va
 
 static DEFINE_SPINLOCK(iosapic_lock);
 
-static inline void iosapic_eoi(void __iomem *addr, unsigned int data)
+static inline void iosapic_eoi(__le32 __iomem *addr, __le32 data)
 {
-	__raw_writel(data, addr);
+	__raw_writel((__force u32)data, addr);
 }
 
 /*
diff --git a/drivers/parisc/iosapic_private.h b/drivers/parisc/iosapic_private.h
index 6e05e30a2450a..7a928c03d5201 100644
--- a/drivers/parisc/iosapic_private.h
+++ b/drivers/parisc/iosapic_private.h
@@ -132,8 +132,8 @@ struct iosapic_irt {
 struct vector_info {
 	struct iosapic_info *iosapic;	/* I/O SAPIC this vector is on */
 	struct irt_entry *irte;		/* IRT entry */
-	u32 __iomem *eoi_addr;		/* precalculate EOI reg address */
-	u32	eoi_data;		/* IA64: ?       PA: swapped txn_data */
+	__le32 __iomem *eoi_addr;	/* precalculate EOI reg address */
+	__le32	eoi_data;		/* IA64: ?       PA: swapped txn_data */
 	int	txn_irq;		/* virtual IRQ number for processor */
 	ulong	txn_addr;		/* IA64: id_eid  PA: partial HPA */
 	u32	txn_data;		/* CPU interrupt bit */
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH AUTOSEL 4.14 3/6] parisc: irq: Make irq_stack_union static to avoid sparse warning
  2023-09-24 13:20 [PATCH AUTOSEL 4.14 1/6] parisc: sba: Fix compile warning wrt list of SBA devices Sasha Levin
  2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 2/6] parisc: iosapic.c: Fix sparse warnings Sasha Levin
@ 2023-09-24 13:20 ` Sasha Levin
  2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 4/6] selftests/ftrace: Correctly enable event in instance-event.tc Sasha Levin
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2023-09-24 13:20 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Helge Deller, Sasha Levin, James.Bottomley, linux-parisc

From: Helge Deller <deller@gmx.de>

[ Upstream commit b1bef1388c427cdad7331a9c8eb4ebbbe5b954b0 ]

Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/parisc/kernel/irq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/parisc/kernel/irq.c b/arch/parisc/kernel/irq.c
index c152c30c2d06d..11c1505775f87 100644
--- a/arch/parisc/kernel/irq.c
+++ b/arch/parisc/kernel/irq.c
@@ -392,7 +392,7 @@ union irq_stack_union {
 	volatile unsigned int lock[1];
 };
 
-DEFINE_PER_CPU(union irq_stack_union, irq_stack_union) = {
+static DEFINE_PER_CPU(union irq_stack_union, irq_stack_union) = {
 		.slock = { 1,1,1,1 },
 	};
 #endif
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH AUTOSEL 4.14 4/6] selftests/ftrace: Correctly enable event in instance-event.tc
  2023-09-24 13:20 [PATCH AUTOSEL 4.14 1/6] parisc: sba: Fix compile warning wrt list of SBA devices Sasha Levin
  2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 2/6] parisc: iosapic.c: Fix sparse warnings Sasha Levin
  2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 3/6] parisc: irq: Make irq_stack_union static to avoid sparse warning Sasha Levin
@ 2023-09-24 13:20 ` Sasha Levin
  2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 5/6] ring-buffer: Avoid softlockup in ring_buffer_resize() Sasha Levin
  2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 6/6] ata: libata-eh: do not clear ATA_PFLAG_EH_PENDING in ata_eh_reset() Sasha Levin
  4 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2023-09-24 13:20 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Zheng Yejian, Masami Hiramatsu, Steven Rostedt, Shuah Khan,
	Sasha Levin, shuah, linux-trace-kernel, linux-kselftest

From: Zheng Yejian <zhengyejian1@huawei.com>

[ Upstream commit f4e4ada586995b17f828c6d147d1800eb1471450 ]

Function instance_set() expects to enable event 'sched_switch', so we
should set 1 to its 'enable' file.

Testcase passed after this patch:
  # ./ftracetest test.d/instances/instance-event.tc
  === Ftrace unit tests ===
  [1] Test creation and deletion of trace instances while setting an event
  [PASS]

  # of passed:  1
  # of failed:  0
  # of unresolved:  0
  # of untested:  0
  # of unsupported:  0
  # of xfailed:  0
  # of undefined(test bug):  0

Signed-off-by: Zheng Yejian <zhengyejian1@huawei.com>
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 .../testing/selftests/ftrace/test.d/instances/instance-event.tc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc b/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
index d7f48b55df51c..ee11b42014c83 100644
--- a/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
+++ b/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
@@ -43,7 +43,7 @@ instance_read() {
 
 instance_set() {
         while :; do
-                echo 1 > foo/events/sched/sched_switch
+                echo 1 > foo/events/sched/sched_switch/enable
         done 2> /dev/null
 }
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH AUTOSEL 4.14 5/6] ring-buffer: Avoid softlockup in ring_buffer_resize()
  2023-09-24 13:20 [PATCH AUTOSEL 4.14 1/6] parisc: sba: Fix compile warning wrt list of SBA devices Sasha Levin
                   ` (2 preceding siblings ...)
  2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 4/6] selftests/ftrace: Correctly enable event in instance-event.tc Sasha Levin
@ 2023-09-24 13:20 ` Sasha Levin
  2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 6/6] ata: libata-eh: do not clear ATA_PFLAG_EH_PENDING in ata_eh_reset() Sasha Levin
  4 siblings, 0 replies; 7+ messages in thread
From: Sasha Levin @ 2023-09-24 13:20 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Zheng Yejian, mhiramat, Steven Rostedt, Sasha Levin,
	linux-trace-kernel

From: Zheng Yejian <zhengyejian1@huawei.com>

[ Upstream commit f6bd2c92488c30ef53b5bd80c52f0a7eee9d545a ]

When user resize all trace ring buffer through file 'buffer_size_kb',
then in ring_buffer_resize(), kernel allocates buffer pages for each
cpu in a loop.

If the kernel preemption model is PREEMPT_NONE and there are many cpus
and there are many buffer pages to be allocated, it may not give up cpu
for a long time and finally cause a softlockup.

To avoid it, call cond_resched() after each cpu buffer allocation.

Link: https://lore.kernel.org/linux-trace-kernel/20230906081930.3939106-1-zhengyejian1@huawei.com

Cc: <mhiramat@kernel.org>
Signed-off-by: Zheng Yejian <zhengyejian1@huawei.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 kernel/trace/ring_buffer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 1949d7bbe145d..f0d4ff2db2ef0 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1686,6 +1686,8 @@ int ring_buffer_resize(struct ring_buffer *buffer, unsigned long size,
 				err = -ENOMEM;
 				goto out_err;
 			}
+
+			cond_resched();
 		}
 
 		get_online_cpus();
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH AUTOSEL 4.14 6/6] ata: libata-eh: do not clear ATA_PFLAG_EH_PENDING in ata_eh_reset()
  2023-09-24 13:20 [PATCH AUTOSEL 4.14 1/6] parisc: sba: Fix compile warning wrt list of SBA devices Sasha Levin
                   ` (3 preceding siblings ...)
  2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 5/6] ring-buffer: Avoid softlockup in ring_buffer_resize() Sasha Levin
@ 2023-09-24 13:20 ` Sasha Levin
  2023-09-28 16:00   ` Niklas Cassel
  4 siblings, 1 reply; 7+ messages in thread
From: Sasha Levin @ 2023-09-24 13:20 UTC (permalink / raw)
  To: linux-kernel, stable
  Cc: Niklas Cassel, Damien Le Moal, Sasha Levin, linux-ide

From: Niklas Cassel <niklas.cassel@wdc.com>

[ Upstream commit 80cc944eca4f0baa9c381d0706f3160e491437f2 ]

ata_scsi_port_error_handler() starts off by clearing ATA_PFLAG_EH_PENDING,
before calling ap->ops->error_handler() (without holding the ap->lock).

If an error IRQ is received while ap->ops->error_handler() is running,
the irq handler will set ATA_PFLAG_EH_PENDING.

Once ap->ops->error_handler() returns, ata_scsi_port_error_handler()
checks if ATA_PFLAG_EH_PENDING is set, and if it is, another iteration
of ATA EH is performed.

The problem is that ATA_PFLAG_EH_PENDING is not only cleared by
ata_scsi_port_error_handler(), it is also cleared by ata_eh_reset().

ata_eh_reset() is called by ap->ops->error_handler(). This additional
clearing done by ata_eh_reset() breaks the whole retry logic in
ata_scsi_port_error_handler(). Thus, if an error IRQ is received while
ap->ops->error_handler() is running, the port will currently remain
frozen and will never get re-enabled.

The additional clearing in ata_eh_reset() was introduced in commit
1e641060c4b5 ("libata: clear eh_info on reset completion").

Looking at the original error report:
https://marc.info/?l=linux-ide&m=124765325828495&w=2

We can see the following happening:
[    1.074659] ata3: XXX port freeze
[    1.074700] ata3: XXX hardresetting link, stopping engine
[    1.074746] ata3: XXX flipping SControl

[    1.411471] ata3: XXX irq_stat=400040 CONN|PHY
[    1.411475] ata3: XXX port freeze

[    1.420049] ata3: XXX starting engine
[    1.420096] ata3: XXX rc=0, class=1
[    1.420142] ata3: XXX clearing IRQs for thawing
[    1.420188] ata3: XXX port thawed
[    1.420234] ata3: SATA link up 3.0 Gbps (SStatus 123 SControl 300)

We are not supposed to be able to receive an error IRQ while the port is
frozen (PxIE is set to 0, i.e. all IRQs for the port are disabled).

AHCI 1.3.1 section 10.7.1.1 First Tier (IS Register) states:
"Each bit location can be thought of as reporting a '1' if the virtual
"interrupt line" for that port is indicating it wishes to generate an
interrupt. That is, if a port has one or more interrupt status bit set,
and the enables for those status bits are set, then this bit shall be set."

Additionally, AHCI state P:ComInit clearly shows that the state machine
will only jump to P:ComInitSetIS (which sets IS.IPS(x) to '1'), if PxIE.PCE
is set to '1'. In our case, PxIE is set to 0, so IS.IPS(x) won't get set.

So IS.IPS(x) only gets set if PxIS and PxIE is set.

AHCI 1.3.1 section 10.7.1.1 First Tier (IS Register) also states:
"The bits in this register are read/write clear. It is set by the level of
the virtual interrupt line being a set, and cleared by a write of '1' from
the software."

So if IS.IPS(x) is set, you need to explicitly clear it by writing a 1 to
IS.IPS(x) for that port.

Since PxIE is cleared, the only way to get an interrupt while the port is
frozen, is if IS.IPS(x) is set, and the only way IS.IPS(x) can be set when
the port is frozen, is if it was set before the port was frozen.

However, since commit 737dd811a3db ("ata: libahci: clear pending interrupt
status"), we clear both PxIS and IS.IPS(x) after freezing the port, but
before the COMRESET, so the problem that commit 1e641060c4b5 ("libata:
clear eh_info on reset completion") fixed can no longer happen.

Thus, revert commit 1e641060c4b5 ("libata: clear eh_info on reset
completion"), so that the retry logic in ata_scsi_port_error_handler()
works once again. (The retry logic is still needed, since we can still
get an error IRQ _after_ the port has been thawed, but before
ata_scsi_port_error_handler() takes the ap->lock in order to check
if ATA_PFLAG_EH_PENDING is set.)

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/ata/libata-eh.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index cbe9af624a06f..8a789de056807 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2948,18 +2948,11 @@ int ata_eh_reset(struct ata_link *link, int classify,
 			postreset(slave, classes);
 	}
 
-	/*
-	 * Some controllers can't be frozen very well and may set spurious
-	 * error conditions during reset.  Clear accumulated error
-	 * information and re-thaw the port if frozen.  As reset is the
-	 * final recovery action and we cross check link onlineness against
-	 * device classification later, no hotplug event is lost by this.
-	 */
+	/* clear cached SError */
 	spin_lock_irqsave(link->ap->lock, flags);
-	memset(&link->eh_info, 0, sizeof(link->eh_info));
+	link->eh_info.serror = 0;
 	if (slave)
-		memset(&slave->eh_info, 0, sizeof(link->eh_info));
-	ap->pflags &= ~ATA_PFLAG_EH_PENDING;
+		slave->eh_info.serror = 0;
 	spin_unlock_irqrestore(link->ap->lock, flags);
 
 	if (ap->pflags & ATA_PFLAG_FROZEN)
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH AUTOSEL 4.14 6/6] ata: libata-eh: do not clear ATA_PFLAG_EH_PENDING in ata_eh_reset()
  2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 6/6] ata: libata-eh: do not clear ATA_PFLAG_EH_PENDING in ata_eh_reset() Sasha Levin
@ 2023-09-28 16:00   ` Niklas Cassel
  0 siblings, 0 replies; 7+ messages in thread
From: Niklas Cassel @ 2023-09-28 16:00 UTC (permalink / raw)
  To: Sasha Levin
  Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Damien Le Moal, linux-ide@vger.kernel.org

Hello Sasha,

The upstream commit that you are backporting here:
80cc944eca4f ("ata: libata-eh: do not clear ATA_PFLAG_EH_PENDING in ata_eh_reset()")

Requires that we have:
737dd811a3db ("ata: libahci: clear pending interrupt status")
in stable. This is currently not the case.

See my comment in:
https://lore.kernel.org/stable/ZRWf7Avtdv3DeqCU@x1-carbon/T/#t

Perhaps both:
93c7711494f4 ("ata: ahci: Drop pointless VPRINTK() calls and convert the remaining ones")
and
737dd811a3db ("ata: libahci: clear pending interrupt status")

Could be backported to stable.
That way,
80cc944eca4f ("ata: libata-eh: do not clear ATA_PFLAG_EH_PENDING in ata_eh_reset()")
should be safe to backport to stable as well.


Kind regards,
Niklas


On Sun, Sep 24, 2023 at 09:20:49AM -0400, Sasha Levin wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> [ Upstream commit 80cc944eca4f0baa9c381d0706f3160e491437f2 ]
> 
> ata_scsi_port_error_handler() starts off by clearing ATA_PFLAG_EH_PENDING,
> before calling ap->ops->error_handler() (without holding the ap->lock).
> 
> If an error IRQ is received while ap->ops->error_handler() is running,
> the irq handler will set ATA_PFLAG_EH_PENDING.
> 
> Once ap->ops->error_handler() returns, ata_scsi_port_error_handler()
> checks if ATA_PFLAG_EH_PENDING is set, and if it is, another iteration
> of ATA EH is performed.
> 
> The problem is that ATA_PFLAG_EH_PENDING is not only cleared by
> ata_scsi_port_error_handler(), it is also cleared by ata_eh_reset().
> 
> ata_eh_reset() is called by ap->ops->error_handler(). This additional
> clearing done by ata_eh_reset() breaks the whole retry logic in
> ata_scsi_port_error_handler(). Thus, if an error IRQ is received while
> ap->ops->error_handler() is running, the port will currently remain
> frozen and will never get re-enabled.
> 
> The additional clearing in ata_eh_reset() was introduced in commit
> 1e641060c4b5 ("libata: clear eh_info on reset completion").
> 
> Looking at the original error report:
> https://marc.info/?l=linux-ide&m=124765325828495&w=2
> 
> We can see the following happening:
> [    1.074659] ata3: XXX port freeze
> [    1.074700] ata3: XXX hardresetting link, stopping engine
> [    1.074746] ata3: XXX flipping SControl
> 
> [    1.411471] ata3: XXX irq_stat=400040 CONN|PHY
> [    1.411475] ata3: XXX port freeze
> 
> [    1.420049] ata3: XXX starting engine
> [    1.420096] ata3: XXX rc=0, class=1
> [    1.420142] ata3: XXX clearing IRQs for thawing
> [    1.420188] ata3: XXX port thawed
> [    1.420234] ata3: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
> 
> We are not supposed to be able to receive an error IRQ while the port is
> frozen (PxIE is set to 0, i.e. all IRQs for the port are disabled).
> 
> AHCI 1.3.1 section 10.7.1.1 First Tier (IS Register) states:
> "Each bit location can be thought of as reporting a '1' if the virtual
> "interrupt line" for that port is indicating it wishes to generate an
> interrupt. That is, if a port has one or more interrupt status bit set,
> and the enables for those status bits are set, then this bit shall be set."
> 
> Additionally, AHCI state P:ComInit clearly shows that the state machine
> will only jump to P:ComInitSetIS (which sets IS.IPS(x) to '1'), if PxIE.PCE
> is set to '1'. In our case, PxIE is set to 0, so IS.IPS(x) won't get set.
> 
> So IS.IPS(x) only gets set if PxIS and PxIE is set.
> 
> AHCI 1.3.1 section 10.7.1.1 First Tier (IS Register) also states:
> "The bits in this register are read/write clear. It is set by the level of
> the virtual interrupt line being a set, and cleared by a write of '1' from
> the software."
> 
> So if IS.IPS(x) is set, you need to explicitly clear it by writing a 1 to
> IS.IPS(x) for that port.
> 
> Since PxIE is cleared, the only way to get an interrupt while the port is
> frozen, is if IS.IPS(x) is set, and the only way IS.IPS(x) can be set when
> the port is frozen, is if it was set before the port was frozen.
> 
> However, since commit 737dd811a3db ("ata: libahci: clear pending interrupt
> status"), we clear both PxIS and IS.IPS(x) after freezing the port, but
> before the COMRESET, so the problem that commit 1e641060c4b5 ("libata:
> clear eh_info on reset completion") fixed can no longer happen.
> 
> Thus, revert commit 1e641060c4b5 ("libata: clear eh_info on reset
> completion"), so that the retry logic in ata_scsi_port_error_handler()
> works once again. (The retry logic is still needed, since we can still
> get an error IRQ _after_ the port has been thawed, but before
> ata_scsi_port_error_handler() takes the ap->lock in order to check
> if ATA_PFLAG_EH_PENDING is set.)
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  drivers/ata/libata-eh.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
> index cbe9af624a06f..8a789de056807 100644
> --- a/drivers/ata/libata-eh.c
> +++ b/drivers/ata/libata-eh.c
> @@ -2948,18 +2948,11 @@ int ata_eh_reset(struct ata_link *link, int classify,
>  			postreset(slave, classes);
>  	}
>  
> -	/*
> -	 * Some controllers can't be frozen very well and may set spurious
> -	 * error conditions during reset.  Clear accumulated error
> -	 * information and re-thaw the port if frozen.  As reset is the
> -	 * final recovery action and we cross check link onlineness against
> -	 * device classification later, no hotplug event is lost by this.
> -	 */
> +	/* clear cached SError */
>  	spin_lock_irqsave(link->ap->lock, flags);
> -	memset(&link->eh_info, 0, sizeof(link->eh_info));
> +	link->eh_info.serror = 0;
>  	if (slave)
> -		memset(&slave->eh_info, 0, sizeof(link->eh_info));
> -	ap->pflags &= ~ATA_PFLAG_EH_PENDING;
> +		slave->eh_info.serror = 0;
>  	spin_unlock_irqrestore(link->ap->lock, flags);
>  
>  	if (ap->pflags & ATA_PFLAG_FROZEN)
> -- 
> 2.40.1
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-09-28 16:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-24 13:20 [PATCH AUTOSEL 4.14 1/6] parisc: sba: Fix compile warning wrt list of SBA devices Sasha Levin
2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 2/6] parisc: iosapic.c: Fix sparse warnings Sasha Levin
2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 3/6] parisc: irq: Make irq_stack_union static to avoid sparse warning Sasha Levin
2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 4/6] selftests/ftrace: Correctly enable event in instance-event.tc Sasha Levin
2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 5/6] ring-buffer: Avoid softlockup in ring_buffer_resize() Sasha Levin
2023-09-24 13:20 ` [PATCH AUTOSEL 4.14 6/6] ata: libata-eh: do not clear ATA_PFLAG_EH_PENDING in ata_eh_reset() Sasha Levin
2023-09-28 16:00   ` Niklas Cassel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox