On 3/31/23 13:48, James Bottomley wrote:
> On Thu, 2023-03-30 at 12:12 -0500, Mike Christie wrote:
>> On 3/30/23 11:49 AM, Tomas Henzl wrote:
>>> Set the state to deleted in sd_shutdown so that the attached LLD
>>> doesn't receive new I/O (can happen when in kexec) later after
>>> LLD's shutdown function has been called.
>>>
>>> Signed-off-by: Tomas Henzl <thenzl@xxxxxxxxxx>
>>> ---
>>> drivers/scsi/sd.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
>>> index 4f28dd617eca..8095f0302e66 100644
>>> --- a/drivers/scsi/sd.c
>>> +++ b/drivers/scsi/sd.c
>>> @@ -3694,10 +3694,13 @@ static int sd_start_stop_device(struct
>>> scsi_disk *sdkp, int start)
>>> static void sd_shutdown(struct device *dev)
>>> {
>>> struct scsi_disk *sdkp = dev_get_drvdata(dev);
>>> + struct scsi_device *sdp;
>>>
>>> if (!sdkp)
>>> return; /* this can happen */
>>>
>>> + sdp = sdkp->device;
>>> +
>>> if (pm_runtime_suspended(dev))
>>> return;
>>>
>>> @@ -3710,6 +3713,10 @@ static void sd_shutdown(struct device *dev)
>>> sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
>>> sd_start_stop_device(sdkp, 0);
>>> }
>>> +
>>> + mutex_lock(&sdp->state_mutex);
>>> + scsi_device_set_state(sdp, SDEV_DEL);
>>> + mutex_unlock(&sdp->state_mutex);
>>> }
>>
>> If this is run for device removal what state will be in here?
>>
>> Are we going to do:
>> 1. __scsi_remove_device sets the state to SDEV_CANCEL at the
>> beginning of the function
>
> It will also interfere with target and host device removal. They
> traverse their own lists and assume that anything in DEL is already
> being removed, which won't be the case here. So basically, after this
> happens it's impossible to clean the device trees. It also means any
> I/O to the root device wouldn't be allowed.
How will it interfere? After a return from sd_remove or via
device_unregister->__scsi_remove_device the device state is SDEV_DEL
regardless whether this patch has been added or not. Or is sd_shutdown
called directly?
> I assume the contention is that if we get here, we're either going for
> immediate shutdown or all the root device remounting to read only has
> already been done? If so, could you say that?
I can't say that, quite the opposite (see body of the mail). When the
system goes shutdown the individual device's .shutdown is called. Just
moments after sd shutdown the LLD shutdown is entered and the driver
stops any I/O immediately anyway. With this patch the I/O is stopped
before reaching LLD with a reasonable message and without error
correction mechanism in place.
I also assume that no I/O after sd_shutdown was projected when it was
written as there is a cache sync followed by a device power down.
Tomas
>
> James
>
