mbox series

[00/14] scsi: Remove in_interrupt() usage.

Message ID 20201126132952.2287996-1-bigeasy@linutronix.de
Headers show
Series scsi: Remove in_interrupt() usage. | expand

Message

Sebastian Andrzej Siewior Nov. 26, 2020, 1:29 p.m. UTC
Folks,

in the discussion about preempt count consistency across kernel
configurations:

 https://lore.kernel.org/r/20200914204209.256266093@linutronix.de/

it was concluded that the usage of in_interrupt() and related context
checks should be removed from non-core code.

This includes allocation mode (GFP_*) decisions and avoidance of code paths
which might sleep.
In the long run, usage of 'preemptible, in_*irq etc.' should be banned from
driver code completely.

This series addresses most of the SCSI subsystem.
The first three patches have Fixes tags and address bugs were noticed during
review.

Sebastian

Comments

Jinpu Wang Nov. 26, 2020, 1:58 p.m. UTC | #1
On Thu, Nov 26, 2020 at 2:30 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> From: "Ahmed S. Darwish" <a.darwish@linutronix.de>
>
> hw_event_sas_phy_up() is used in hardirq/softirq context:
>
>  pm8001_interrupt_handler_msix() || pm8001_interrupt_handler_intx() || pm8001_tasklet
>    => PM8001_CHIP_DISP->isr() = pm80xx_chip_isr()
>      => process_oq() [spin_lock_irqsave(&pm8001_ha->lock,)]
>        => process_one_iomb()
>          => mpi_hw_event()
>            => hw_event_sas_phy_up()
>              => msleep(200)
>
> Revert the msleep() back to an mdelay() to avoid sleeping in atomic
> context.
>
> Fixes: 4daf1ef3c681 ("scsi: pm80xx: Convert 'long' mdelay to msleep")
> Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Cc: Vikram Auradkar <auradkar@google.com>
> Cc: Jack Wang <jinpu.wang@cloud.ionos.com>
Indeed, thx for the fix.
Acked-by: Jack Wang <jinpu.wang@cloud.ionos.com>> ---
>  drivers/scsi/pm8001/pm80xx_hwi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index 69f8244539e04..7d838e316657c 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -3296,7 +3296,7 @@ hw_event_sas_phy_up(struct pm8001_hba_info *pm8001_ha, void *piomb)
>         pm8001_get_attached_sas_addr(phy, phy->sas_phy.attached_sas_addr);
>         spin_unlock_irqrestore(&phy->sas_phy.frame_rcvd_lock, flags);
>         if (pm8001_ha->flags == PM8001F_RUN_TIME)
> -               msleep(200);/*delay a moment to wait disk to spinup*/
> +               mdelay(200);/*delay a moment to wait disk to spinup*/
>         pm8001_bytes_dmaed(pm8001_ha, phy_id);
>  }
>
> --
> 2.29.2
>
Daniel Wagner Nov. 30, 2020, 3:30 p.m. UTC | #2
On Thu, Nov 26, 2020 at 02:29:51PM +0100, Sebastian Andrzej Siewior wrote:
> From: Thomas Gleixner <tglx@linutronix.de>

> 

> in_interrupt() is referenced all over the place in these drivers. Most of

> these references are comments which are outdated and wrong.

> 

> Aside of that in_interrupt() is deprecated as it does not provide what the

> name suggests. It covers more than hard/soft interrupt servicing context

> and is semantically ill defined.

> 

> From reading the mpt_config() code and the history this is clearly a

> debug mechanism and should probably be replaced by might_sleep() or

> completely removed because such checks are already in the subsequent

> functions.

> 

> Remove the in_interrupt() references and replace the usage in

> mpt_config() with might_sleep().

> 

> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

> Cc: Sathya Prakash <sathya.prakash@broadcom.com>

> Cc: Sreekanth Reddy <sreekanth.reddy@broadcom.com>

> Cc: Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>

> Cc: MPT-FusionLinux.pdl@broadcom.com


Reviewed-by: Daniel Wagner <dwagner@suse.de>
Martin K. Petersen Dec. 1, 2020, 5:06 a.m. UTC | #3
Sebastian,

> This series addresses most of the SCSI subsystem.  The first three

> patches have Fixes tags and address bugs were noticed during review.


Applied series to 5.11/scsi-staging except for the NCR patch. Would like
to see where that lands first.

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering
Sebastian Andrzej Siewior Dec. 1, 2020, 9:08 a.m. UTC | #4
On 2020-12-01 00:06:55 [-0500], Martin K. Petersen wrote:
>

> Applied series to 5.11/scsi-staging except for the NCR patch. Would like

> to see where that lands first.


Thank you Martin.

> Thanks!


Sebastian
Martin K. Petersen Dec. 8, 2020, 4:51 a.m. UTC | #5
On Thu, 26 Nov 2020 14:29:38 +0100, Sebastian Andrzej Siewior wrote:

> Folks,

> 

> in the discussion about preempt count consistency across kernel

> configurations:

> 

>  https://lore.kernel.org/r/20200914204209.256266093@linutronix.de/

> 

> [...]


Applied to 5.11/scsi-queue, thanks!

[01/14] scsi: pm80xx: Do not sleep in atomic context.
        https://git.kernel.org/mkp/scsi/c/4ba9e516573e
[02/14] scsi: hisi_sas: Remove preemptible().
        https://git.kernel.org/mkp/scsi/c/18577cdcaeeb
[03/14] scsi: qla4xxx: qla4_82xx_crb_win_lock(): Remove in_interrupt().
        https://git.kernel.org/mkp/scsi/c/a93c38353198
[04/14] scsi: qla2xxx: qla82xx: Remove in_interrupt().
        https://git.kernel.org/mkp/scsi/c/8ac246bdd07a
[05/14] scsi: qla2xxx: tcm_qla2xxx: Remove BUG_ON(in_interrupt()).
        https://git.kernel.org/mkp/scsi/c/9fef41f25d60
[06/14] scsi: qla2xxx: init/os: Remove in_interrupt().
        https://git.kernel.org/mkp/scsi/c/4f6a57c23b1e
[07/14] scsi: qla4xxx: qla4_82xx_idc_lock(): Remove in_interrupt().
        https://git.kernel.org/mkp/scsi/c/3627668c2e2c
[08/14] scsi: qla4xxx: qla4_82xx_rom_lock(): Remove in_interrupt().
        https://git.kernel.org/mkp/scsi/c/014aced18aff
[09/14] scsi: mpt3sas: Remove in_interrupt().
        https://git.kernel.org/mkp/scsi/c/547c0d1aeb76
[10/14] scsi: myrb: Remove WARN_ON(in_interrupt()).
        https://git.kernel.org/mkp/scsi/c/3bc08b9545da
[11/14] scsi: myrs: Remove WARN_ON(in_interrupt()).
        https://git.kernel.org/mkp/scsi/c/ca6853693cbd
[12/14] scsi: NCR5380: Remove in_interrupt().
        (no commit info)
[13/14] scsi: message: fusion: Remove in_interrupt() usage in mpt_config().
        https://git.kernel.org/mkp/scsi/c/b8a5144370bc
[14/14] scsi: message: fusion: Remove in_interrupt() usage in mptsas_cleanup_fw_event_q().
        https://git.kernel.org/mkp/scsi/c/817a7c996786

-- 
Martin K. Petersen	Oracle Linux Engineering