Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Wed, 2017-04-26 at 18:53 +, Bart Van Assche wrote: > On Tue, 2017-04-18 at 08:56 -0700, James Bottomley wrote: > > The priority has got to be the removal we've been ordered to do > > rather > > than waiting around to see if the transport comes back so we can > > send a > > final flush. > >

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Tue, 2017-04-18 at 08:56 -0700, James Bottomley wrote: > The priority has got to be the removal we've been ordered to do rather > than waiting around to see if the transport comes back so we can send a > final flush. > > How about this approach. It goes straight to DEL if the device is >

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Sun, 2017-04-23 at 12:28 -0500, James Bottomley wrote: > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -1250,6 +1250,12 @@ scsi_prep_state_check(struct scsi_device *sdev, struct > request *req) > break; > case SDEV_BLOCK: >

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Thu, 2017-04-20 at 15:13 -0700, James Bottomley wrote: > How is that possible? Once the device goes into the CANCEL state, it > no longer can be found by starget_for_each_device() because > scsi_device_get() returns NULL ... scsi_target_block() is not serialized against

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Thu, 2017-04-20 at 15:13 -0700, James Bottomley wrote: > On Thu, 2017-04-20 at 21:59 +, Bart Van Assche wrote: > > This approach cannot work. A scsi_target_block() call by the > > transport layer can happen concurrently with the > > __scsi_remove_device() call and hence can occur at any

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Thu, 2017-04-20 at 21:59 +, Bart Van Assche wrote: > On Tue, 2017-04-18 at 16:56 -0700, James Bottomley wrote: > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > > index e5a2d590a104..31171204cfd1 100644 > > --- a/drivers/scsi/scsi_lib.c > > +++ b/drivers/scsi/scsi_lib.c >

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Tue, 2017-04-18 at 16:56 -0700, James Bottomley wrote: > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index e5a2d590a104..31171204cfd1 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -2611,7 +2611,6 @@ scsi_device_set_state(struct scsi_device

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Tue, 2017-04-18 at 16:56 -0700, James Bottomley wrote: > This means the combined 1/3 3/3 patch looks like this: > [ ... ] Hello James, The two attached patches pass my tests. How would you like to proceed with patch 1/2? Would you like to submit it yourself or is it OK for you if I mention

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Wed, 2017-04-19 at 00:02 +, Bart Van Assche wrote: > On Tue, 2017-04-18 at 16:56 -0700, James Bottomley wrote: > > Actually, I think 3/3 is wrong. You need to start the queue as > > soon as we see it's blocked and the device has gone into DEL. This > > is safe because DEL ensures

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Tue, 2017-04-18 at 16:56 -0700, James Bottomley wrote: > Actually, I think 3/3 is wrong. You need to start the queue as soon as > we see it's blocked and the device has gone into DEL. This is safe > because DEL ensures nothing ever gets from the generic request function > to queuecommand, so

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Tue, 2017-04-18 at 23:47 +, Bart Van Assche wrote: > On Tue, 2017-04-18 at 08:56 -0700, James Bottomley wrote: > > How about this approach. It goes straight to DEL if the device is > > blocked (skipping CANCEL). This means that all the commands issued > > in > > ->shutdown will error in

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Tue, 2017-04-18 at 08:56 -0700, James Bottomley wrote: > How about this approach. It goes straight to DEL if the device is > blocked (skipping CANCEL). This means that all the commands issued in > ->shutdown will error in the mid-layer, thus making the removal proceed > without being

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Tue, 2017-04-18 at 08:56 -0700, James Bottomley wrote: > How about this approach. It goes straight to DEL if the device is > blocked (skipping CANCEL). This means that all the commands issued in > ->shutdown will error in the mid-layer, thus making the removal proceed > without being

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Tue, 2017-04-18 at 16:44 +0200, Benjamin Block wrote: > Hej Bart, > > sry for the late'ish reply, had a long weekend. > > On Thu, Apr 13, 2017 at 12:28:54AM +, Bart Van Assche wrote: > > On Wed, 2017-04-12 at 16:41 +0200, Benjamin Block wrote: > > > On Mon, Apr 10, 2017 at 10:54:01AM

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

On Tue, 2017-04-18 at 16:44 +0200, Benjamin Block wrote: > I still wonder why we try 'so hard' scheduling a command for a dead > device, but as that seems to be the status quo, and only lacks in the > case where the LLD is already half-way gone, its ok for me too. I mean, > the order is a bit

Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

Hej Bart, sry for the late'ish reply, had a long weekend. On Thu, Apr 13, 2017 at 12:28:54AM +, Bart Van Assche wrote: > On Wed, 2017-04-12 at 16:41 +0200, Benjamin Block wrote: > > On Mon, Apr 10, 2017 at 10:54:01AM -0700, Bart Van Assche wrote: > > > [ ... ] > > OK, so I take it the

[PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous

This patch avoids that sd_shutdown() hangs on the SYNCHRONIZE CACHE command if the block layer queue has been stopped by scsi_target_block(). Signed-off-by: Bart Van Assche Cc: Israel Rukshin Cc: Max Gurtovoy Cc: Hannes