mbox series

[0/2] Fix runtime suspended device resume

Message ID 20231120073522.34180-1-dlemoal@kernel.org
Headers show
Series Fix runtime suspended device resume | expand

Message

Damien Le Moal Nov. 20, 2023, 7:35 a.m. UTC 
The first patch changes the use of the bool type back to the regular
unsigned:1 for the manage_xxx scsi device flags. This is marked as a fix
and CC-stable to avoid issues with later eventual fixes in this area.

The second patch addresses an issue with system resume with devices that
were runtime suspended. For ATA devices, this leads to a disk still
being reported as suspended while it is in fact spun up due to how ATA
resume is done (port reset).

Damien Le Moal (2):
  scsi: Change scsi device boolean fields to single bit flags
  scsi: sd: fix system start for ATA devices

 drivers/ata/libata-scsi.c  |  9 +++++++--
 drivers/firewire/sbp2.c    |  6 +++---
 drivers/scsi/sd.c          |  9 ++++++++-
 include/scsi/scsi_device.h | 12 +++++++++---
 4 files changed, 27 insertions(+), 9 deletions(-)

Comments

Bart Van Assche Nov. 20, 2023, 4:06 p.m. UTC | #1 
On 11/19/23 23:35, Damien Le Moal wrote:
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 10480eb582b2..1fb460dfca0c 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -167,19 +167,19 @@ struct scsi_device {
>   	 * power state for system suspend/resume (suspend to RAM and
>   	 * hibernation) operations.
>   	 */
> -	bool manage_system_start_stop;
> +	unsigned manage_system_start_stop:1;
>   
>   	/*
>   	 * If true, let the high-level device driver (sd) manage the device
>   	 * power state for runtime device suspand and resume operations.
>   	 */
> -	bool manage_runtime_start_stop;
> +	unsigned manage_runtime_start_stop:1;
>   
>   	/*
>   	 * If true, let the high-level device driver (sd) manage the device
>   	 * power state for system shutdown (power off) operations.
>   	 */
> -	bool manage_shutdown;
> +	unsigned manage_shutdown:1;
>   
>   	unsigned removable:1;
>   	unsigned changed:1;	/* Data invalid due to media change */

Is there any code that modifies the above flags from different
threads simultaneously? I'm wondering whether this patch introduces
one or more race conditions related to changing these flags.

Thanks,

Bart.