[PATCH] powerpc64/idle: Fix SP offsets when saving GPRs
The idle entry/exit code saves/restores GPRs in the stack "red zone" (Protected Zone according to PowerPC64 ELF ABI v2). However, the offset used for the first GPR is incorrect and overwrites the back chain - the Protected Zone actually starts below the current SP. In practice this is probably not an issue, but it's still incorrect so fix it. Signed-off-by: Christopher M. Riedl --- arch/powerpc/kernel/idle_book3s.S | 126 +++--- 1 file changed, 63 insertions(+), 63 deletions(-) diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S index 22f249b6f58d..80cf35183e9d 100644 --- a/arch/powerpc/kernel/idle_book3s.S +++ b/arch/powerpc/kernel/idle_book3s.S @@ -53,27 +53,27 @@ _GLOBAL(isa300_idle_stop_mayloss) mflrr4 mfcrr5 /* use stack red zone rather than a new frame for saving regs */ - std r2,-8*0(r1) - std r14,-8*1(r1) - std r15,-8*2(r1) - std r16,-8*3(r1) - std r17,-8*4(r1) - std r18,-8*5(r1) - std r19,-8*6(r1) - std r20,-8*7(r1) - std r21,-8*8(r1) - std r22,-8*9(r1) - std r23,-8*10(r1) - std r24,-8*11(r1) - std r25,-8*12(r1) - std r26,-8*13(r1) - std r27,-8*14(r1) - std r28,-8*15(r1) - std r29,-8*16(r1) - std r30,-8*17(r1) - std r31,-8*18(r1) - std r4,-8*19(r1) - std r5,-8*20(r1) + std r2,-8*1(r1) + std r14,-8*2(r1) + std r15,-8*3(r1) + std r16,-8*4(r1) + std r17,-8*5(r1) + std r18,-8*6(r1) + std r19,-8*7(r1) + std r20,-8*8(r1) + std r21,-8*9(r1) + std r22,-8*10(r1) + std r23,-8*11(r1) + std r24,-8*12(r1) + std r25,-8*13(r1) + std r26,-8*14(r1) + std r27,-8*15(r1) + std r28,-8*16(r1) + std r29,-8*17(r1) + std r30,-8*18(r1) + std r31,-8*19(r1) + std r4,-8*20(r1) + std r5,-8*21(r1) /* 168 bytes */ PPC_STOP b . /* catch bugs */ @@ -89,8 +89,8 @@ _GLOBAL(isa300_idle_stop_mayloss) */ _GLOBAL(idle_return_gpr_loss) ld r1,PACAR1(r13) - ld r4,-8*19(r1) - ld r5,-8*20(r1) + ld r4,-8*20(r1) + ld r5,-8*21(r1) mtlrr4 mtcrr5 /* @@ -98,25 +98,25 @@ _GLOBAL(idle_return_gpr_loss) * from PACATOC. This could be avoided for that less common case * if KVM saved its r2. */ - ld r2,-8*0(r1) - ld r14,-8*1(r1) - ld r15,-8*2(r1) - ld r16,-8*3(r1) - ld r17,-8*4(r1) - ld r18,-8*5(r1) - ld r19,-8*6(r1) - ld r20,-8*7(r1) - ld r21,-8*8(r1) - ld r22,-8*9(r1) - ld r23,-8*10(r1) - ld r24,-8*11(r1) - ld r25,-8*12(r1) - ld r26,-8*13(r1) - ld r27,-8*14(r1) - ld r28,-8*15(r1) - ld r29,-8*16(r1) - ld r30,-8*17(r1) - ld r31,-8*18(r1) + ld r2,-8*1(r1) + ld r14,-8*2(r1) + ld r15,-8*3(r1) + ld r16,-8*4(r1) + ld r17,-8*5(r1) + ld r18,-8*6(r1) + ld r19,-8*7(r1) + ld r20,-8*8(r1) + ld r21,-8*9(r1) + ld r22,-8*10(r1) + ld r23,-8*11(r1) + ld r24,-8*12(r1) + ld r25,-8*13(r1) + ld r26,-8*14(r1) + ld r27,-8*15(r1) + ld r28,-8*16(r1) + ld r29,-8*17(r1) + ld r30,-8*18(r1) + ld r31,-8*19(r1) blr /* @@ -155,27 +155,27 @@ _GLOBAL(isa206_idle_insn_mayloss) mflrr4 mfcrr5 /* use stack red zone rather than a new frame for saving regs */ - std r2,-8*0(r1) - std r14,-8*1(r1) - std r15,-8*2(r1) - std r16,-8*3(r1) - std r17,-8*4(r1) - std r18,-8*5(r1) - std r19,-8*6(r1) - std r20,-8*7(r1) - std r21,-8*8(r1) - std r22,-8*9(r1) - std r23,-8*10(r1) - std r24,-8*11(r1) - std r25,-8*12(r1) - std r26,-8*13(r1) - std r27,-8*14(r1) - std r28,-8*15(r1) - std r29,-8*16(r1) - std r30,-8*17(r1) - std r31,-8*18(r1) - std r4,-8*19(r1) - std r5,-8*20(r1) + std r2,-8*1(r1) + std r14,-8*2(r1) + std r15,-8*3(r1) + std r16,-8*4(r1) + std r17,-8*5(r1) + std r18,-8*6(r1) + std r19,-8*7(r1) + std r20,-8*8(r1) + std r21,-8*9(r1) + std r22,-8*10(r1) + std r23,-8*11(r1) + std r24,-8*12(r1) + std r25,-8*13(r1) + std r26,-8*14(r1) + std r27,
Re: [PATCH] powerpc: remove unneeded semicolons
Although they are harmless, I think we should keep the consistency of kernel coding style. Thanks On 2021/1/29 19:48, Michael Ellerman wrote: Chengyang Fan writes: Remove superfluous semicolons after function definitions. Is there a good reason why? I realise they're superfluous, but they're also harmless as far as I'm aware. cheers arch/powerpc/include/asm/book3s/32/mmu-hash.h | 2 +- arch/powerpc/include/asm/book3s/64/mmu.h| 2 +- arch/powerpc/include/asm/book3s/64/tlbflush-radix.h | 2 +- arch/powerpc/include/asm/book3s/64/tlbflush.h | 2 +- arch/powerpc/include/asm/firmware.h | 2 +- arch/powerpc/include/asm/kvm_ppc.h | 6 +++--- arch/powerpc/include/asm/paca.h | 6 +++--- arch/powerpc/include/asm/rtas.h | 2 +- arch/powerpc/include/asm/setup.h| 6 +++--- arch/powerpc/include/asm/simple_spinlock.h | 4 ++-- arch/powerpc/include/asm/smp.h | 2 +- arch/powerpc/include/asm/xmon.h | 4 ++-- arch/powerpc/kernel/prom.c | 2 +- arch/powerpc/kernel/setup.h | 12 ++-- arch/powerpc/platforms/powernv/subcore.h| 2 +- arch/powerpc/platforms/pseries/pseries.h| 2 +- 16 files changed, 29 insertions(+), 29 deletions(-) .
Re: [PATCH 04/13] module: use RCU to synchronize find_module
Christoph Hellwig writes:
> On Thu, Jan 28, 2021 at 05:50:56PM -0300, Thiago Jung Bauermann wrote:
>> > struct module *find_module(const char *name)
>> > {
>> > - module_assert_mutex();
>>
>> Does it make sense to replace the assert above with the warn below
>> (untested)?
>>
>> RCU_LOCKDEP_WARN(rcu_read_lock_sched_held());
>
> One caller actually holds module_mutex still. And find_module_all,
> which implements the actual logic already asserts that either
> module_mutex is held or rcu_read_lock, so I don't tink we need an
> extra one here.
Ok, thanks for the clarification.
--
Thiago Jung Bauermann
IBM Linux Technology Center
Re: [PATCH] vio: make remove callback return void
On Wed, Jan 27, 2021 at 6:41 PM Uwe Kleine-König wrote: > > The driver core ignores the return value of struct bus_type::remove() > because there is only little that can be done. To simplify the quest to > make this function return void, let struct vio_driver::remove() return > void, too. All users already unconditionally return 0, this commit makes > it obvious that returning an error code is a bad idea and makes it > obvious for future driver authors that returning an error code isn't > intended. > > Note there are two nominally different implementations for a vio bus: > one in arch/sparc/kernel/vio.c and the other in > arch/powerpc/platforms/pseries/vio.c. I didn't care to check which > driver is using which of these busses (or if even some of them can be > used with both) and simply adapt all drivers and the two bus codes in > one go. > > Note that for the powerpc implementation there is a semantical change: > Before this patch for a device that was bound to a driver without a > remove callback vio_cmo_bus_remove(viodev) wasn't called. As the device > core still considers the device unbound after vio_bus_remove() returns > calling this unconditionally is the consistent behaviour which is > implemented here. > > Signed-off-by: Uwe Kleine-König Acked-by: Lijun Pan
Re: [PATCH] vio: make remove callback return void
On 1/27/21 1:50 PM, Uwe Kleine-König wrote:
> The driver core ignores the return value of struct bus_type::remove()
> because there is only little that can be done. To simplify the quest to
> make this function return void, let struct vio_driver::remove() return
> void, too. All users already unconditionally return 0, this commit makes
> it obvious that returning an error code is a bad idea and makes it
> obvious for future driver authors that returning an error code isn't
> intended.
>
> Note there are two nominally different implementations for a vio bus:
> one in arch/sparc/kernel/vio.c and the other in
> arch/powerpc/platforms/pseries/vio.c. I didn't care to check which
> driver is using which of these busses (or if even some of them can be
> used with both) and simply adapt all drivers and the two bus codes in
> one go.
>
> Note that for the powerpc implementation there is a semantical change:
> Before this patch for a device that was bound to a driver without a
> remove callback vio_cmo_bus_remove(viodev) wasn't called. As the device
> core still considers the device unbound after vio_bus_remove() returns
> calling this unconditionally is the consistent behaviour which is
> implemented here.
>
> Signed-off-by: Uwe Kleine-König
Reviewed-by: Tyrel Datwyler
> ---
> Hello,
>
> note that this change depends on
> https://lore.kernel.org/r/20210121062005.53271-1-...@linux.ibm.com which
> removes
> an (ignored) return -EBUSY in drivers/net/ethernet/ibm/ibmvnic.c.
> I don't know when/if this latter patch will be applied, so it might take
> some time until my patch can go in.
>
> Best regards
> Uwe
>
> arch/powerpc/include/asm/vio.h | 2 +-
> arch/powerpc/platforms/pseries/vio.c | 7 +++
> arch/sparc/include/asm/vio.h | 2 +-
> arch/sparc/kernel/ds.c | 6 --
> arch/sparc/kernel/vio.c | 4 ++--
> drivers/block/sunvdc.c | 3 +--
> drivers/char/hw_random/pseries-rng.c | 3 +--
> drivers/char/tpm/tpm_ibmvtpm.c | 4 +---
> drivers/crypto/nx/nx-842-pseries.c | 4 +---
> drivers/crypto/nx/nx.c | 4 +---
> drivers/misc/ibmvmc.c| 4 +---
> drivers/net/ethernet/ibm/ibmveth.c | 4 +---
> drivers/net/ethernet/ibm/ibmvnic.c | 4 +---
> drivers/net/ethernet/sun/ldmvsw.c| 4 +---
> drivers/net/ethernet/sun/sunvnet.c | 3 +--
> drivers/scsi/ibmvscsi/ibmvfc.c | 3 +--
> drivers/scsi/ibmvscsi/ibmvscsi.c | 4 +---
> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 4 +---
> drivers/tty/hvc/hvcs.c | 3 +--
> drivers/tty/vcc.c| 4 +---
> 20 files changed, 22 insertions(+), 54 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/vio.h b/arch/powerpc/include/asm/vio.h
> index 0cf52746531b..721c0d6715ac 100644
> --- a/arch/powerpc/include/asm/vio.h
> +++ b/arch/powerpc/include/asm/vio.h
> @@ -113,7 +113,7 @@ struct vio_driver {
> const char *name;
> const struct vio_device_id *id_table;
> int (*probe)(struct vio_dev *dev, const struct vio_device_id *id);
> - int (*remove)(struct vio_dev *dev);
> + void (*remove)(struct vio_dev *dev);
> /* A driver must have a get_desired_dma() function to
>* be loaded in a CMO environment if it uses DMA.
>*/
> diff --git a/arch/powerpc/platforms/pseries/vio.c
> b/arch/powerpc/platforms/pseries/vio.c
> index b2797cfe4e2b..9cb4fc839fd5 100644
> --- a/arch/powerpc/platforms/pseries/vio.c
> +++ b/arch/powerpc/platforms/pseries/vio.c
> @@ -1261,7 +1261,6 @@ static int vio_bus_remove(struct device *dev)
> struct vio_dev *viodev = to_vio_dev(dev);
> struct vio_driver *viodrv = to_vio_driver(dev->driver);
> struct device *devptr;
> - int ret = 1;
>
> /*
>* Hold a reference to the device after the remove function is called
> @@ -1270,13 +1269,13 @@ static int vio_bus_remove(struct device *dev)
> devptr = get_device(dev);
>
> if (viodrv->remove)
> - ret = viodrv->remove(viodev);
> + viodrv->remove(viodev);
>
> - if (!ret && firmware_has_feature(FW_FEATURE_CMO))
> + if (firmware_has_feature(FW_FEATURE_CMO))
> vio_cmo_bus_remove(viodev);
>
> put_device(devptr);
> - return ret;
> + return 0;
> }
>
> /**
> diff --git a/arch/sparc/include/asm/vio.h b/arch/sparc/include/asm/vio.h
> index 059f0eb678e0..8a1a83bbb6d5 100644
> --- a/arch/sparc/include/asm/vio.h
> +++ b/arch/sparc/include/asm/vio.h
> @@ -362,7 +362,7 @@ struct vio_driver {
> struct list_headnode;
> const struct vio_device_id *id_table;
> int (*probe)(struct vio_dev *dev, const struct vio_device_id *id);
> - int (*remove)(struct vio_dev *dev);
> + void (*remove)(struct vio_dev *dev);
> void (*shutdown)(struct vio_dev *dev);
> unsigned long driver_data;
> struct device_driv
Re: [PATCH v2] tpm: ibmvtpm: fix error return code in tpm_ibmvtpm_probe()
On 1/29/21 12:35 PM, Jarkko Sakkinen wrote:
On Mon, Jan 25, 2021 at 08:47:53PM -0500, Stefan Berger wrote:
From: Stefan Berger
Return error code -ETIMEDOUT rather than '0' when waiting for the
rtce_buf to be set has timed out.
Fixes: d8d74ea3c002 ("tpm: ibmvtpm: Wait for buffer to be set before
proceeding")
Reported-by: Hulk Robot
Signed-off-by: Wang Hai
Signed-off-by: Stefan Berger
---
Reviewed-by: Jarkko Sakkinen
Thanks! Should I add
Cc: sta...@vger.kernel.org to this?
Yes, that would be good! Thank you!
Stefan
/Jarkko
drivers/char/tpm/tpm_ibmvtpm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 994385bf37c0..813eb2cac0ce 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -687,6 +687,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
ibmvtpm->rtce_buf != NULL,
HZ)) {
dev_err(dev, "CRQ response timed out\n");
+ rc = -ETIMEDOUT;
goto init_irq_cleanup;
}
--
2.25.4
Re: [PATCH v2] tpm: ibmvtpm: fix error return code in tpm_ibmvtpm_probe()
On Mon, Jan 25, 2021 at 08:47:53PM -0500, Stefan Berger wrote:
> From: Stefan Berger
>
> Return error code -ETIMEDOUT rather than '0' when waiting for the
> rtce_buf to be set has timed out.
>
> Fixes: d8d74ea3c002 ("tpm: ibmvtpm: Wait for buffer to be set before
> proceeding")
> Reported-by: Hulk Robot
> Signed-off-by: Wang Hai
> Signed-off-by: Stefan Berger
> ---
Reviewed-by: Jarkko Sakkinen
Thanks! Should I add
Cc: sta...@vger.kernel.org to this?
/Jarkko
> drivers/char/tpm/tpm_ibmvtpm.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
> index 994385bf37c0..813eb2cac0ce 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.c
> +++ b/drivers/char/tpm/tpm_ibmvtpm.c
> @@ -687,6 +687,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
> ibmvtpm->rtce_buf != NULL,
> HZ)) {
> dev_err(dev, "CRQ response timed out\n");
> + rc = -ETIMEDOUT;
> goto init_irq_cleanup;
> }
>
> --
> 2.25.4
>
>
[PATCH AUTOSEL 4.4 2/2] scsi: ibmvfc: Set default timeout to avoid crash during migration
From: Brian King
[ Upstream commit 764907293edc1af7ac857389af9dc858944f53dc ]
While testing live partition mobility, we have observed occasional crashes
of the Linux partition. What we've seen is that during the live migration,
for specific configurations with large amounts of memory, slow network
links, and workloads that are changing memory a lot, the partition can end
up being suspended for 30 seconds or longer. This resulted in the following
scenario:
CPU 0 CPU 1
--- --
scsi_queue_rqmigration_store
-> blk_mq_start_request -> rtas_ibm_suspend_me
-> blk_add_timer -> on_each_cpu(rtas_percpu_suspend_me
___V
|
V
-> IPI from CPU 1
-> rtas_percpu_suspend_me
-> __rtas_suspend_last_cpu
-- Linux partition suspended for > 30 seconds --
-> for_each_online_cpu(cpu)
plpar_hcall_norets(H_PROD
-> scsi_dispatch_cmd
-> scsi_times_out
-> scsi_abort_command
-> queue_delayed_work
-> ibmvfc_queuecommand_lck
-> ibmvfc_send_event
-> ibmvfc_send_crq
- returns H_CLOSED
<- returns SCSI_MLQUEUE_HOST_BUSY
-> __blk_mq_requeue_request
-> scmd_eh_abort_handler
-> scsi_try_to_abort_cmd
- returns SUCCESS
-> scsi_queue_insert
Normally, the SCMD_STATE_COMPLETE bit would protect against the command
completion and the timeout, but that doesn't work here, since we don't
check that at all in the SCSI_MLQUEUE_HOST_BUSY path.
In this case we end up calling scsi_queue_insert on a request that has
already been queued, or possibly even freed, and we crash.
The patch below simply increases the default I/O timeout to avoid this race
condition. This is also the timeout value that nearly all IBM SAN storage
recommends setting as the default value.
Link:
https://lore.kernel.org/r/1610463998-19791-1-git-send-email-brk...@linux.vnet.ibm.com
Signed-off-by: Brian King
Signed-off-by: Martin K. Petersen
Signed-off-by: Sasha Levin
---
drivers/scsi/ibmvscsi/ibmvfc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index db80ab8335dfb..aa74f72e582ab 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -2883,8 +2883,10 @@ static int ibmvfc_slave_configure(struct scsi_device
*sdev)
unsigned long flags = 0;
spin_lock_irqsave(shost->host_lock, flags);
- if (sdev->type == TYPE_DISK)
+ if (sdev->type == TYPE_DISK) {
sdev->allow_restart = 1;
+ blk_queue_rq_timeout(sdev->request_queue, 120 * HZ);
+ }
spin_unlock_irqrestore(shost->host_lock, flags);
return 0;
}
--
2.27.0
[PATCH AUTOSEL 4.9 3/4] scsi: ibmvfc: Set default timeout to avoid crash during migration
From: Brian King
[ Upstream commit 764907293edc1af7ac857389af9dc858944f53dc ]
While testing live partition mobility, we have observed occasional crashes
of the Linux partition. What we've seen is that during the live migration,
for specific configurations with large amounts of memory, slow network
links, and workloads that are changing memory a lot, the partition can end
up being suspended for 30 seconds or longer. This resulted in the following
scenario:
CPU 0 CPU 1
--- --
scsi_queue_rqmigration_store
-> blk_mq_start_request -> rtas_ibm_suspend_me
-> blk_add_timer -> on_each_cpu(rtas_percpu_suspend_me
___V
|
V
-> IPI from CPU 1
-> rtas_percpu_suspend_me
-> __rtas_suspend_last_cpu
-- Linux partition suspended for > 30 seconds --
-> for_each_online_cpu(cpu)
plpar_hcall_norets(H_PROD
-> scsi_dispatch_cmd
-> scsi_times_out
-> scsi_abort_command
-> queue_delayed_work
-> ibmvfc_queuecommand_lck
-> ibmvfc_send_event
-> ibmvfc_send_crq
- returns H_CLOSED
<- returns SCSI_MLQUEUE_HOST_BUSY
-> __blk_mq_requeue_request
-> scmd_eh_abort_handler
-> scsi_try_to_abort_cmd
- returns SUCCESS
-> scsi_queue_insert
Normally, the SCMD_STATE_COMPLETE bit would protect against the command
completion and the timeout, but that doesn't work here, since we don't
check that at all in the SCSI_MLQUEUE_HOST_BUSY path.
In this case we end up calling scsi_queue_insert on a request that has
already been queued, or possibly even freed, and we crash.
The patch below simply increases the default I/O timeout to avoid this race
condition. This is also the timeout value that nearly all IBM SAN storage
recommends setting as the default value.
Link:
https://lore.kernel.org/r/1610463998-19791-1-git-send-email-brk...@linux.vnet.ibm.com
Signed-off-by: Brian King
Signed-off-by: Martin K. Petersen
Signed-off-by: Sasha Levin
---
drivers/scsi/ibmvscsi/ibmvfc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 04b3ac17531db..7865feb8e5e83 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -2891,8 +2891,10 @@ static int ibmvfc_slave_configure(struct scsi_device
*sdev)
unsigned long flags = 0;
spin_lock_irqsave(shost->host_lock, flags);
- if (sdev->type == TYPE_DISK)
+ if (sdev->type == TYPE_DISK) {
sdev->allow_restart = 1;
+ blk_queue_rq_timeout(sdev->request_queue, 120 * HZ);
+ }
spin_unlock_irqrestore(shost->host_lock, flags);
return 0;
}
--
2.27.0
[PATCH AUTOSEL 4.14 6/8] scsi: ibmvfc: Set default timeout to avoid crash during migration
From: Brian King
[ Upstream commit 764907293edc1af7ac857389af9dc858944f53dc ]
While testing live partition mobility, we have observed occasional crashes
of the Linux partition. What we've seen is that during the live migration,
for specific configurations with large amounts of memory, slow network
links, and workloads that are changing memory a lot, the partition can end
up being suspended for 30 seconds or longer. This resulted in the following
scenario:
CPU 0 CPU 1
--- --
scsi_queue_rqmigration_store
-> blk_mq_start_request -> rtas_ibm_suspend_me
-> blk_add_timer -> on_each_cpu(rtas_percpu_suspend_me
___V
|
V
-> IPI from CPU 1
-> rtas_percpu_suspend_me
-> __rtas_suspend_last_cpu
-- Linux partition suspended for > 30 seconds --
-> for_each_online_cpu(cpu)
plpar_hcall_norets(H_PROD
-> scsi_dispatch_cmd
-> scsi_times_out
-> scsi_abort_command
-> queue_delayed_work
-> ibmvfc_queuecommand_lck
-> ibmvfc_send_event
-> ibmvfc_send_crq
- returns H_CLOSED
<- returns SCSI_MLQUEUE_HOST_BUSY
-> __blk_mq_requeue_request
-> scmd_eh_abort_handler
-> scsi_try_to_abort_cmd
- returns SUCCESS
-> scsi_queue_insert
Normally, the SCMD_STATE_COMPLETE bit would protect against the command
completion and the timeout, but that doesn't work here, since we don't
check that at all in the SCSI_MLQUEUE_HOST_BUSY path.
In this case we end up calling scsi_queue_insert on a request that has
already been queued, or possibly even freed, and we crash.
The patch below simply increases the default I/O timeout to avoid this race
condition. This is also the timeout value that nearly all IBM SAN storage
recommends setting as the default value.
Link:
https://lore.kernel.org/r/1610463998-19791-1-git-send-email-brk...@linux.vnet.ibm.com
Signed-off-by: Brian King
Signed-off-by: Martin K. Petersen
Signed-off-by: Sasha Levin
---
drivers/scsi/ibmvscsi/ibmvfc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index dbacd9830d3df..460014ded14de 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -2891,8 +2891,10 @@ static int ibmvfc_slave_configure(struct scsi_device
*sdev)
unsigned long flags = 0;
spin_lock_irqsave(shost->host_lock, flags);
- if (sdev->type == TYPE_DISK)
+ if (sdev->type == TYPE_DISK) {
sdev->allow_restart = 1;
+ blk_queue_rq_timeout(sdev->request_queue, 120 * HZ);
+ }
spin_unlock_irqrestore(shost->host_lock, flags);
return 0;
}
--
2.27.0
[PATCH AUTOSEL 4.19 09/12] selftests/powerpc: Only test lwm/stmw on big endian
From: Michael Ellerman [ Upstream commit dd3a44c06f7b4f14e90065bf05d62c255b20005f ] Newer binutils (>= 2.36) refuse to assemble lmw/stmw when building in little endian mode. That breaks compilation of our alignment handler test: /tmp/cco4l14N.s: Assembler messages: /tmp/cco4l14N.s:1440: Error: `lmw' invalid when little-endian /tmp/cco4l14N.s:1814: Error: `stmw' invalid when little-endian make[2]: *** [../../lib.mk:139: /output/kselftest/powerpc/alignment/alignment_handler] Error 1 These tests do pass on little endian machines, as the kernel will still emulate those instructions even when running little endian (which is arguably a kernel bug). But we don't really need to test that case, so ifdef those instructions out to get the alignment test building again. Reported-by: Libor Pechacek Signed-off-by: Michael Ellerman Tested-by: Libor Pechacek Link: https://lore.kernel.org/r/20210119041800.3093047-1-...@ellerman.id.au Signed-off-by: Sasha Levin --- .../testing/selftests/powerpc/alignment/alignment_handler.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/powerpc/alignment/alignment_handler.c b/tools/testing/selftests/powerpc/alignment/alignment_handler.c index 169a8b9719fb9..4f8335e0c9858 100644 --- a/tools/testing/selftests/powerpc/alignment/alignment_handler.c +++ b/tools/testing/selftests/powerpc/alignment/alignment_handler.c @@ -384,7 +384,6 @@ int test_alignment_handler_integer(void) LOAD_DFORM_TEST(ldu); LOAD_XFORM_TEST(ldx); LOAD_XFORM_TEST(ldux); - LOAD_DFORM_TEST(lmw); STORE_DFORM_TEST(stb); STORE_XFORM_TEST(stbx); STORE_DFORM_TEST(stbu); @@ -403,7 +402,11 @@ int test_alignment_handler_integer(void) STORE_XFORM_TEST(stdx); STORE_DFORM_TEST(stdu); STORE_XFORM_TEST(stdux); + +#ifdef __BIG_ENDIAN__ + LOAD_DFORM_TEST(lmw); STORE_DFORM_TEST(stmw); +#endif return rc; } -- 2.27.0
[PATCH AUTOSEL 4.19 08/12] scsi: ibmvfc: Set default timeout to avoid crash during migration
From: Brian King
[ Upstream commit 764907293edc1af7ac857389af9dc858944f53dc ]
While testing live partition mobility, we have observed occasional crashes
of the Linux partition. What we've seen is that during the live migration,
for specific configurations with large amounts of memory, slow network
links, and workloads that are changing memory a lot, the partition can end
up being suspended for 30 seconds or longer. This resulted in the following
scenario:
CPU 0 CPU 1
--- --
scsi_queue_rqmigration_store
-> blk_mq_start_request -> rtas_ibm_suspend_me
-> blk_add_timer -> on_each_cpu(rtas_percpu_suspend_me
___V
|
V
-> IPI from CPU 1
-> rtas_percpu_suspend_me
-> __rtas_suspend_last_cpu
-- Linux partition suspended for > 30 seconds --
-> for_each_online_cpu(cpu)
plpar_hcall_norets(H_PROD
-> scsi_dispatch_cmd
-> scsi_times_out
-> scsi_abort_command
-> queue_delayed_work
-> ibmvfc_queuecommand_lck
-> ibmvfc_send_event
-> ibmvfc_send_crq
- returns H_CLOSED
<- returns SCSI_MLQUEUE_HOST_BUSY
-> __blk_mq_requeue_request
-> scmd_eh_abort_handler
-> scsi_try_to_abort_cmd
- returns SUCCESS
-> scsi_queue_insert
Normally, the SCMD_STATE_COMPLETE bit would protect against the command
completion and the timeout, but that doesn't work here, since we don't
check that at all in the SCSI_MLQUEUE_HOST_BUSY path.
In this case we end up calling scsi_queue_insert on a request that has
already been queued, or possibly even freed, and we crash.
The patch below simply increases the default I/O timeout to avoid this race
condition. This is also the timeout value that nearly all IBM SAN storage
recommends setting as the default value.
Link:
https://lore.kernel.org/r/1610463998-19791-1-git-send-email-brk...@linux.vnet.ibm.com
Signed-off-by: Brian King
Signed-off-by: Martin K. Petersen
Signed-off-by: Sasha Levin
---
drivers/scsi/ibmvscsi/ibmvfc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 090ab377f65e5..50078a199fea0 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -2890,8 +2890,10 @@ static int ibmvfc_slave_configure(struct scsi_device
*sdev)
unsigned long flags = 0;
spin_lock_irqsave(shost->host_lock, flags);
- if (sdev->type == TYPE_DISK)
+ if (sdev->type == TYPE_DISK) {
sdev->allow_restart = 1;
+ blk_queue_rq_timeout(sdev->request_queue, 120 * HZ);
+ }
spin_unlock_irqrestore(shost->host_lock, flags);
return 0;
}
--
2.27.0
[PATCH AUTOSEL 5.4 14/19] selftests/powerpc: Only test lwm/stmw on big endian
From: Michael Ellerman [ Upstream commit dd3a44c06f7b4f14e90065bf05d62c255b20005f ] Newer binutils (>= 2.36) refuse to assemble lmw/stmw when building in little endian mode. That breaks compilation of our alignment handler test: /tmp/cco4l14N.s: Assembler messages: /tmp/cco4l14N.s:1440: Error: `lmw' invalid when little-endian /tmp/cco4l14N.s:1814: Error: `stmw' invalid when little-endian make[2]: *** [../../lib.mk:139: /output/kselftest/powerpc/alignment/alignment_handler] Error 1 These tests do pass on little endian machines, as the kernel will still emulate those instructions even when running little endian (which is arguably a kernel bug). But we don't really need to test that case, so ifdef those instructions out to get the alignment test building again. Reported-by: Libor Pechacek Signed-off-by: Michael Ellerman Tested-by: Libor Pechacek Link: https://lore.kernel.org/r/20210119041800.3093047-1-...@ellerman.id.au Signed-off-by: Sasha Levin --- .../testing/selftests/powerpc/alignment/alignment_handler.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/powerpc/alignment/alignment_handler.c b/tools/testing/selftests/powerpc/alignment/alignment_handler.c index 0453c50c949cb..0725239bbd85c 100644 --- a/tools/testing/selftests/powerpc/alignment/alignment_handler.c +++ b/tools/testing/selftests/powerpc/alignment/alignment_handler.c @@ -380,7 +380,6 @@ int test_alignment_handler_integer(void) LOAD_DFORM_TEST(ldu); LOAD_XFORM_TEST(ldx); LOAD_XFORM_TEST(ldux); - LOAD_DFORM_TEST(lmw); STORE_DFORM_TEST(stb); STORE_XFORM_TEST(stbx); STORE_DFORM_TEST(stbu); @@ -399,7 +398,11 @@ int test_alignment_handler_integer(void) STORE_XFORM_TEST(stdx); STORE_DFORM_TEST(stdu); STORE_XFORM_TEST(stdux); + +#ifdef __BIG_ENDIAN__ + LOAD_DFORM_TEST(lmw); STORE_DFORM_TEST(stmw); +#endif return rc; } -- 2.27.0
[PATCH AUTOSEL 5.4 10/19] scsi: ibmvfc: Set default timeout to avoid crash during migration
From: Brian King
[ Upstream commit 764907293edc1af7ac857389af9dc858944f53dc ]
While testing live partition mobility, we have observed occasional crashes
of the Linux partition. What we've seen is that during the live migration,
for specific configurations with large amounts of memory, slow network
links, and workloads that are changing memory a lot, the partition can end
up being suspended for 30 seconds or longer. This resulted in the following
scenario:
CPU 0 CPU 1
--- --
scsi_queue_rqmigration_store
-> blk_mq_start_request -> rtas_ibm_suspend_me
-> blk_add_timer -> on_each_cpu(rtas_percpu_suspend_me
___V
|
V
-> IPI from CPU 1
-> rtas_percpu_suspend_me
-> __rtas_suspend_last_cpu
-- Linux partition suspended for > 30 seconds --
-> for_each_online_cpu(cpu)
plpar_hcall_norets(H_PROD
-> scsi_dispatch_cmd
-> scsi_times_out
-> scsi_abort_command
-> queue_delayed_work
-> ibmvfc_queuecommand_lck
-> ibmvfc_send_event
-> ibmvfc_send_crq
- returns H_CLOSED
<- returns SCSI_MLQUEUE_HOST_BUSY
-> __blk_mq_requeue_request
-> scmd_eh_abort_handler
-> scsi_try_to_abort_cmd
- returns SUCCESS
-> scsi_queue_insert
Normally, the SCMD_STATE_COMPLETE bit would protect against the command
completion and the timeout, but that doesn't work here, since we don't
check that at all in the SCSI_MLQUEUE_HOST_BUSY path.
In this case we end up calling scsi_queue_insert on a request that has
already been queued, or possibly even freed, and we crash.
The patch below simply increases the default I/O timeout to avoid this race
condition. This is also the timeout value that nearly all IBM SAN storage
recommends setting as the default value.
Link:
https://lore.kernel.org/r/1610463998-19791-1-git-send-email-brk...@linux.vnet.ibm.com
Signed-off-by: Brian King
Signed-off-by: Martin K. Petersen
Signed-off-by: Sasha Levin
---
drivers/scsi/ibmvscsi/ibmvfc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 8a76284b59b08..523809a8a2323 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -2881,8 +2881,10 @@ static int ibmvfc_slave_configure(struct scsi_device
*sdev)
unsigned long flags = 0;
spin_lock_irqsave(shost->host_lock, flags);
- if (sdev->type == TYPE_DISK)
+ if (sdev->type == TYPE_DISK) {
sdev->allow_restart = 1;
+ blk_queue_rq_timeout(sdev->request_queue, 120 * HZ);
+ }
spin_unlock_irqrestore(shost->host_lock, flags);
return 0;
}
--
2.27.0
[PATCH AUTOSEL 5.10 28/41] selftests/powerpc: Only test lwm/stmw on big endian
From: Michael Ellerman [ Upstream commit dd3a44c06f7b4f14e90065bf05d62c255b20005f ] Newer binutils (>= 2.36) refuse to assemble lmw/stmw when building in little endian mode. That breaks compilation of our alignment handler test: /tmp/cco4l14N.s: Assembler messages: /tmp/cco4l14N.s:1440: Error: `lmw' invalid when little-endian /tmp/cco4l14N.s:1814: Error: `stmw' invalid when little-endian make[2]: *** [../../lib.mk:139: /output/kselftest/powerpc/alignment/alignment_handler] Error 1 These tests do pass on little endian machines, as the kernel will still emulate those instructions even when running little endian (which is arguably a kernel bug). But we don't really need to test that case, so ifdef those instructions out to get the alignment test building again. Reported-by: Libor Pechacek Signed-off-by: Michael Ellerman Tested-by: Libor Pechacek Link: https://lore.kernel.org/r/20210119041800.3093047-1-...@ellerman.id.au Signed-off-by: Sasha Levin --- .../testing/selftests/powerpc/alignment/alignment_handler.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/powerpc/alignment/alignment_handler.c b/tools/testing/selftests/powerpc/alignment/alignment_handler.c index cb53a8b777e68..c25cf7cd45e9f 100644 --- a/tools/testing/selftests/powerpc/alignment/alignment_handler.c +++ b/tools/testing/selftests/powerpc/alignment/alignment_handler.c @@ -443,7 +443,6 @@ int test_alignment_handler_integer(void) LOAD_DFORM_TEST(ldu); LOAD_XFORM_TEST(ldx); LOAD_XFORM_TEST(ldux); - LOAD_DFORM_TEST(lmw); STORE_DFORM_TEST(stb); STORE_XFORM_TEST(stbx); STORE_DFORM_TEST(stbu); @@ -462,7 +461,11 @@ int test_alignment_handler_integer(void) STORE_XFORM_TEST(stdx); STORE_DFORM_TEST(stdu); STORE_XFORM_TEST(stdux); + +#ifdef __BIG_ENDIAN__ + LOAD_DFORM_TEST(lmw); STORE_DFORM_TEST(stmw); +#endif return rc; } -- 2.27.0
[PATCH AUTOSEL 5.10 18/41] scsi: ibmvfc: Set default timeout to avoid crash during migration
From: Brian King
[ Upstream commit 764907293edc1af7ac857389af9dc858944f53dc ]
While testing live partition mobility, we have observed occasional crashes
of the Linux partition. What we've seen is that during the live migration,
for specific configurations with large amounts of memory, slow network
links, and workloads that are changing memory a lot, the partition can end
up being suspended for 30 seconds or longer. This resulted in the following
scenario:
CPU 0 CPU 1
--- --
scsi_queue_rqmigration_store
-> blk_mq_start_request -> rtas_ibm_suspend_me
-> blk_add_timer -> on_each_cpu(rtas_percpu_suspend_me
___V
|
V
-> IPI from CPU 1
-> rtas_percpu_suspend_me
-> __rtas_suspend_last_cpu
-- Linux partition suspended for > 30 seconds --
-> for_each_online_cpu(cpu)
plpar_hcall_norets(H_PROD
-> scsi_dispatch_cmd
-> scsi_times_out
-> scsi_abort_command
-> queue_delayed_work
-> ibmvfc_queuecommand_lck
-> ibmvfc_send_event
-> ibmvfc_send_crq
- returns H_CLOSED
<- returns SCSI_MLQUEUE_HOST_BUSY
-> __blk_mq_requeue_request
-> scmd_eh_abort_handler
-> scsi_try_to_abort_cmd
- returns SUCCESS
-> scsi_queue_insert
Normally, the SCMD_STATE_COMPLETE bit would protect against the command
completion and the timeout, but that doesn't work here, since we don't
check that at all in the SCSI_MLQUEUE_HOST_BUSY path.
In this case we end up calling scsi_queue_insert on a request that has
already been queued, or possibly even freed, and we crash.
The patch below simply increases the default I/O timeout to avoid this race
condition. This is also the timeout value that nearly all IBM SAN storage
recommends setting as the default value.
Link:
https://lore.kernel.org/r/1610463998-19791-1-git-send-email-brk...@linux.vnet.ibm.com
Signed-off-by: Brian King
Signed-off-by: Martin K. Petersen
Signed-off-by: Sasha Levin
---
drivers/scsi/ibmvscsi/ibmvfc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 070cf516b98fe..57c9a71fa33a7 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -2957,8 +2957,10 @@ static int ibmvfc_slave_configure(struct scsi_device
*sdev)
unsigned long flags = 0;
spin_lock_irqsave(shost->host_lock, flags);
- if (sdev->type == TYPE_DISK)
+ if (sdev->type == TYPE_DISK) {
sdev->allow_restart = 1;
+ blk_queue_rq_timeout(sdev->request_queue, 120 * HZ);
+ }
spin_unlock_irqrestore(shost->host_lock, flags);
return 0;
}
--
2.27.0
Re: [PATCH 04/13] module: use RCU to synchronize find_module
On Thu, 28 Jan 2021, Christoph Hellwig wrote: > Allow for a RCU-sched critical section around find_module, following > the lower level find_module_all helper, and switch the two callers > outside of module.c to use such a RCU-sched critical section instead > of module_mutex. That's a nice idea. > @@ -57,7 +58,7 @@ static void klp_find_object_module(struct klp_object *obj) > if (!klp_is_module(obj)) > return; > > - mutex_lock(&module_mutex); > + rcu_read_lock_sched(); > /* >* We do not want to block removal of patched modules and therefore >* we do not take a reference here. The patches are removed by > @@ -74,7 +75,7 @@ static void klp_find_object_module(struct klp_object *obj) > if (mod && mod->klp_alive) RCU always baffles me a bit, so I'll ask. Don't we need rcu_dereference_sched() here? "mod" comes from a RCU-protected list, so I wonder. > obj->mod = mod; > > - mutex_unlock(&module_mutex); > + rcu_read_unlock_sched(); > } Thanks Miroslav
Re: [PATCH 03/13] module: unexport find_module and module_mutex
On Thu, 28 Jan 2021, Christoph Hellwig wrote: > find_module is not used by modular code any more, and random driver code > has no business calling it to start with. > > Signed-off-by: Christoph Hellwig Reviewed-by: Miroslav Benes M
Re: [PATCH 04/13] module: use RCU to synchronize find_module
On Thu 2021-01-28 19:14:12, Christoph Hellwig wrote: > Allow for a RCU-sched critical section around find_module, following > the lower level find_module_all helper, and switch the two callers > outside of module.c to use such a RCU-sched critical section instead > of module_mutex. > > Signed-off-by: Christoph Hellwig It looks good and safe. Reviewed-by: Petr Mladek Best Regards, Petr
Re: [PATCH 05/13] kallsyms: refactor {,module_}kallsyms_on_each_symbol
On Thu 2021-01-28 19:14:13, Christoph Hellwig wrote: > Require an explicit call to module_kallsyms_on_each_symbol to look > for symbols in modules instead of the call from kallsyms_on_each_symbol, > and acquire module_mutex inside of module_kallsyms_on_each_symbol instead > of leaving that up to the caller. > > Signed-off-by: Christoph Hellwig > --- > kernel/kallsyms.c | 6 +- > kernel/livepatch/core.c | 6 +- > kernel/module.c | 8 > 3 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c > index fe9de067771c34..a0d3f0865916f9 100644 > --- a/kernel/kallsyms.c > +++ b/kernel/kallsyms.c > @@ -177,6 +177,10 @@ unsigned long kallsyms_lookup_name(const char *name) > return module_kallsyms_lookup_name(name); > } > > +/* > + * Iterate over all symbols in vmlinux. For symbols from modules use > + * module_kallsyms_on_each_symbol instead. > + */ > int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *, > unsigned long), > void *data) > @@ -192,7 +196,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char > *, struct module *, > if (ret != 0) > return ret; > } > - return module_kallsyms_on_each_symbol(fn, data); > + return 0; > } > > static unsigned long get_symbol_pos(unsigned long addr, > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index 262cd9b003b9f0..f591dac5e86ef4 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -164,12 +164,8 @@ static int klp_find_object_symbol(const char *objname, > const char *name, > .pos = sympos, > }; > > - mutex_lock(&module_mutex); > - if (objname) > + if (objname || !kallsyms_on_each_symbol(klp_find_callback, &args)) > module_kallsyms_on_each_symbol(klp_find_callback, &args); > - else > - kallsyms_on_each_symbol(klp_find_callback, &args); > - mutex_unlock(&module_mutex); This change is not needed. (objname == NULL) means that we are interested only in symbols in "vmlinux". module_kallsyms_on_each_symbol(klp_find_callback, &args) will always fail when objname == NULL. Best Regards, Petr
Re: [PATCH] powerpc/fault: fix wrong KUAP fault for IO_URING
+Aneesh Le 29/01/2021 à 07:52, Zorro Lang a écrit : On Thu, Jan 28, 2021 at 03:44:21PM +0100, Christophe Leroy wrote: Le 28/01/2021 à 15:42, Jens Axboe a écrit : On 1/28/21 6:52 AM, Zorro Lang wrote: On Wed, Jan 27, 2021 at 08:06:37PM -0700, Jens Axboe wrote: On 1/27/21 8:13 PM, Zorro Lang wrote: On Thu, Jan 28, 2021 at 10:18:07AM +1000, Nicholas Piggin wrote: Excerpts from Jens Axboe's message of January 28, 2021 5:29 am: On 1/27/21 9:38 AM, Christophe Leroy wrote: Le 27/01/2021 à 15:56, Zorro Lang a écrit : On powerpc, io_uring test hit below KUAP fault on __do_page_fault. The fail source line is: if (unlikely(!is_user && bad_kernel_fault(regs, error_code, address, is_write))) return SIGSEGV; The is_user() is based on user_mod(regs) only. This's not suit for io_uring, where the helper thread can assume the user app identity and could perform this fault just fine. So turn to use mm to decide if this is valid or not. I don't understand why testing is_user would be an issue. KUAP purpose it to block any unallowed access from kernel to user memory (Equivalent to SMAP on x86). So it really must be based on MSR_PR bit, that is what is_user provides. If the kernel access is legitimate, kernel should have opened userspace access then you shouldn't get this "Bug: Read fault blocked by KUAP!". As far as I understand, the fault occurs in iov_iter_fault_in_readable() which calls fault_in_pages_readable() And fault_in_pages_readable() uses __get_user() so it is a legitimate access and you really should get a KUAP fault. So the problem is somewhere else, I think you proposed patch just hides the problem, it doesn't fix it. If we do kthread_use_mm(), can we agree that the user access is valid? Yeah the io uring code is fine, provided it uses the uaccess primitives like any other kernel code. It's looking more like a an arch/powerpc bug. We should be able to copy to/from user space, and including faults, if that's been done and the new mm assigned. Because it really should be. If SMAP was a problem on x86, we would have seen it long ago. I'm assuming this may be breakage related to the recent uaccess changes related to set_fs and friends? Or maybe recent changes on the powerpc side? Zorro, did 5.10 work? Would be interesting to know. Sure Nick and Jens, which 5.10 rc? version do you want to know ? Or any git commit(be the HEAD) in 5.10 phase? I forget which versions had what series of this, but 5.10 final - and if that fails, then 5.9 final. IIRC, 5.9 was pre any of these changes, and 5.10 definitely has them. I justed built linux v5.10 with same .config file, and gave it same test. v5.10 (HEAD=2c85ebc57b Linux 5.10) can't reproduce this bug: # ./check generic/013 generic/051 FSTYP -- xfs (non-debug) PLATFORM -- Linux/ppc64le ibm-p9z-xxx- 5.10.0 #3 SMP Thu Jan 28 04:12:14 EST 2021 MKFS_OPTIONS -- -f -m crc=1,finobt=1,reflink=1,rmapbt=1,bigtime=1,inobtcount=1 /dev/sda3 MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda3 /mnt/xfstests/scratch generic/013 138s ... 77s generic/051 103s ... 143s Ran: generic/013 generic/051 Passed all 2 tests Thanks for testing that, so I think it's safe to conclude that there's a regression in powerpc fault handling for kthreads that use kthread_use_mm in this release. A bisect would definitely find it, but might be pointless if Christophe or Nick already have an idea of what it is. I don't have any idea yet, but I'd be curious to see the vmlinux binary matching the reported Oops. I just upload the vmlinux and .config file, the vmlinux it too big, I have to upload it to my google store and share the link as below: config file: https://drive.google.com/file/d/1pMszboxdjbMPqSNXnMH-1UCZC-vtDnI9/view?usp=sharing vmlinux: https://drive.google.com/file/d/1s7g2eBPAFFV61aM2dO9bvVTERGQ9mLYk/view?usp=sharing I used latest upstream mainline linux, HEAD commit is: 76c057c84d (HEAD -> master, origin/master, origin/HEAD) Merge branch 'parisc-5.11-2' of git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux The test failed on this kernel as: # dmesg [ 96.200296] [ cut here ] [ 96.200304] Bug: Read fault blocked by KUAP! [ 96.200309] WARNING: CPU: 3 PID: 1876 at arch/powerpc/mm/fault.c:229 bad_kernel_fault+0x180/0x310 [ 96.200734] NIP [c0849424] fault_in_pages_readable+0x104/0x350 [ 96.200741] LR [c084952c] fault_in_pages_readable+0x20c/0x350 [ 96.200747] --- interrupt: 300 Problem happens in a section where userspace access is supposed to be granted, so the patch you proposed is definitely not the right fix. c0849408: 2c 01 00 4c isync c084940c: a6 03 3d 7d mtspr 29,r9 <== granting userspace access permission c0849410: 2c 01 00 4c isync c0849414: 00 00 36 e9 ld r9,0(r22) c0849418: 20 00 29 81 lwz r9,32(r9) c084941c:
Re: [PATCH] powerpc: remove unneeded semicolons
Chengyang Fan writes: > Remove superfluous semicolons after function definitions. Is there a good reason why? I realise they're superfluous, but they're also harmless as far as I'm aware. cheers > arch/powerpc/include/asm/book3s/32/mmu-hash.h | 2 +- > arch/powerpc/include/asm/book3s/64/mmu.h| 2 +- > arch/powerpc/include/asm/book3s/64/tlbflush-radix.h | 2 +- > arch/powerpc/include/asm/book3s/64/tlbflush.h | 2 +- > arch/powerpc/include/asm/firmware.h | 2 +- > arch/powerpc/include/asm/kvm_ppc.h | 6 +++--- > arch/powerpc/include/asm/paca.h | 6 +++--- > arch/powerpc/include/asm/rtas.h | 2 +- > arch/powerpc/include/asm/setup.h| 6 +++--- > arch/powerpc/include/asm/simple_spinlock.h | 4 ++-- > arch/powerpc/include/asm/smp.h | 2 +- > arch/powerpc/include/asm/xmon.h | 4 ++-- > arch/powerpc/kernel/prom.c | 2 +- > arch/powerpc/kernel/setup.h | 12 ++-- > arch/powerpc/platforms/powernv/subcore.h| 2 +- > arch/powerpc/platforms/pseries/pseries.h| 2 +- > 16 files changed, 29 insertions(+), 29 deletions(-)
Re: [PATCH 1/2] powerpc/vdso: fix unnecessary rebuilds of vgettimeofday.o
Masahiro Yamada writes:
> On Thu, Dec 24, 2020 at 2:12 AM Masahiro Yamada wrote:
>>
>> vgettimeofday.o is unnecessarily rebuilt. Adding it to 'targets' is not
>> enough to fix the issue. Kbuild is correctly rebuilding it because the
>> command line is changed.
>>
>> PowerPC builds each vdso directory twice; first in vdso_prepare to
>> generate vdso{32,64}-offsets.h, second as part of the ordinary build
>> process to embed vdso{32,64}.so.dbg into the kernel.
>>
>> The problem shows up when CONFIG_PPC_WERROR=y due to the following line
>> in arch/powerpc/Kbuild:
>>
>> subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
>>
>> In the preparation stage, Kbuild directly visits the vdso directories,
>> hence it does not inherit subdir-ccflags-y. In the second descend,
>> Kbuild adds -Werror, which results in the command line flipping
>> with/without -Werror.
>>
>> It implies a potential danger; if a more critical flag that would impact
>> the resulted vdso, the offsets recorded in the headers might be different
>> from real offsets in the embedded vdso images.
>>
>> Removing the unneeded second descend solves the problem.
>>
>> Link: https://lore.kernel.org/linuxppc-dev/87tuslxhry@mpe.ellerman.id.au/
>> Reported-by: Michael Ellerman
>> Signed-off-by: Masahiro Yamada
>> ---
>
>
> Michael, please take a look at this.
>
> The unneeded rebuild problem is still remaining.
Sorry missed those.
I guess I'll pick these up as fixes for v5.10.
cheers
Re: [PATCH 0/3] sched: Task priority related cleanups
On Thu, Jan 28, 2021 at 02:10:37PM +0100, Dietmar Eggemann wrote: > Dietmar Eggemann (3): > sched: Remove MAX_USER_RT_PRIO > sched: Remove USER_PRIO, TASK_USER_PRIO and MAX_USER_PRIO > sched/core: Update task_prio() function header Thanks!
Re: [PATCH v2] powerpc/sstep: Fix array out of bound warning
On 2021/01/29 12:47PM, Ravi Bangoria wrote:
> Compiling kernel with -Warray-bounds throws below warning:
>
> In function 'emulate_vsx_store':
> warning: array subscript is above array bounds [-Warray-bounds]
> buf.d[2] = byterev_8(reg->d[1]);
> ~^~~
> buf.d[3] = byterev_8(reg->d[0]);
> ~^~~
>
> Fix it by using temporary array variable 'union vsx_reg buf32[]' in
> that code block. Also, with element_size = 32, 'union vsx_reg *reg'
> is an array of size 2. So, use 'reg' as an array instead of pointer
> in the same code block.
>
> Fixes: af99da74333b ("powerpc/sstep: Support VSX vector paired storage access
> instructions")
> Suggested-by: Naveen N. Rao
> Signed-off-by: Ravi Bangoria
> ---
> v1:
> http://lore.kernel.org/r/20210115061620.692500-1-ravi.bango...@linux.ibm.com
> v1->v2:
> - Change code only in the affected block
I don't see the compiler warning with -Warray-bounds with this patch:
Tested-by: Naveen N. Rao
- Naveen
Re: [PATCH] cxl: Simplify bool conversion
On 29/01/2021 09:25, Yang Li wrote: Fix the following coccicheck warning: ./drivers/misc/cxl/sysfs.c:181:48-53: WARNING: conversion to bool not needed here Reported-by: Abaci Robot Signed-off-by: Yang Li --- Thanks! Acked-by: Frederic Barrat drivers/misc/cxl/sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c index d97a243..c173a5e 100644 --- a/drivers/misc/cxl/sysfs.c +++ b/drivers/misc/cxl/sysfs.c @@ -178,7 +178,7 @@ static ssize_t perst_reloads_same_image_store(struct device *device, if ((rc != 1) || !(val == 1 || val == 0)) return -EINVAL; - adapter->perst_same_image = (val == 1 ? true : false); + adapter->perst_same_image = (val == 1); return count; }
