On 2024/8/19 12:17, Damien Le Moal wrote: > On 8/19/24 13:07, Yihang Li wrote: >> >> >> On 2024/8/19 7:55, Damien Le Moal wrote: >>> On 8/17/24 10:50, Yihang Li wrote: >>>> If formatting a suspended disk (such as formatting with different DIF >>>> type), the disk will be resuming first, and then the format command will >>>> submit to the disk through SG_IO ioctl. >>>> >>>> When the disk is processing the format command, the system does not submit >>>> other commands to the disk. Therefore, the system attempts to suspend the >>>> disk again and sends the SYNC CACHE command. However, the SYNC CACHE >>> >>> Why would the system try to suspend the disk with a request in flight ? Sounds >>> like there is a bug with PM reference counting, no ? >> >> According to my understand and test, the format command request is finished, >> so it is not in flight for the kernel. And the command need a few time to processing >> in the disk while no other commands are being sent. > > OK, fine. But I think that retrying SYNC CACHE if the drive is formatting makes > absolutely no sense at all because there is nothing to flush in that case. > So what about simply ignoring the error ? I.e. something like this: > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c > index 699f4f9674d9..1da267b8cd8a 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -1824,12 +1824,14 @@ static int sd_sync_cache(struct scsi_disk *sdkp) > /* this is no error here */ > return 0; > /* > - * This drive doesn't support sync and there's not much > - * we can do because this is called during shutdown > - * or suspend so just return success so those operations > - * can proceed. > + * If a format is in progress (asc = LOGICAL UNIT NOT > + * READY, ascq = FORMAT IN PROGRESS) or if the drive > + * does not support sync, there is not much we can do > + * because this is called during shutdown or suspend. So > + * just return success so those operations can proceed. > */ > - if (sshdr.sense_key == ILLEGAL_REQUEST) > + if ((sshdr.asc == 0x04 && sshdr.ascq == 0x04) || > + sshdr.sense_key == ILLEGAL_REQUEST) > return 0; > } > Thanks for your suggestion, it seems like good. I will send a new version based on this later. Thanks, Yihang.
