nvme reservation commands during controller reset

nvme reservation commands during controller reset

[Engel, Amit]

Hello, 
We test nvme reservation commands while controller reset and encounter a problem
The reservation cmd during controller reset fails on the host side with error code 0x370 (NVME_SC_HOST_PATH_ERROR)

based on the host code, nvme cli/ioctl commands are marked for failfast (no retry)
This is also the case for block layer user space API for persistent reservations (for example: IOC_PR_RESERVE)
host/core.c 'nvme_alloc_request' set the flag
req->cmd_flags |= REQ_FAILFAST_DRIVER

In case of path failover, the host will fail the cmd with status NVME_SC_HOST_PATH_ERROR
host/fabrics.c:

 blk_status_t nvmf_fail_nonready_command(struct nvme_ctrl *ctrl,
 >------->-------struct request *rq)
 {
 >-------if (ctrl->state != NVME_CTRL_DELETING &&
 >-------    ctrl->state != NVME_CTRL_DEAD &&
 >-------    !blk_noretry_request(rq) && !(rq->cmd_flags & REQ_NVME_MPATH))
 >------->-------return BLK_STS_RESOURCE;
 
  >-------nvme_req(rq)->status = NVME_SC_HOST_PATH_ERROR;
 >-------blk_mq_start_request(rq);
 >-------nvme_complete_rq(rq);
 >-------return BLK_STS_OK;
 }

What is the reason for setting REQ_FAILFAST_DRIVER flag in nvme_alloc_request ?
The original intention was to mark only sync cmds as failfast ?

Thanks
Amit Engel


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Keith Busch]

On Wed, Aug 05, 2020 at 07:14:22AM +0000, Engel, Amit wrote:
> What is the reason for setting REQ_FAILFAST_DRIVER flag in nvme_alloc_request ?
> The original intention was to mark only sync cmds as failfast ?

Special requests don't get generic error handling. The caller gets to decide
how to handle it.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Christoph Hellwig]

On Sun, Aug 09, 2020 at 10:10:16AM -0600, Keith Busch wrote:
> On Wed, Aug 05, 2020 at 07:14:22AM +0000, Engel, Amit wrote:
> > What is the reason for setting REQ_FAILFAST_DRIVER flag in nvme_alloc_request ?
> > The original intention was to mark only sync cmds as failfast ?
> 
> Special requests don't get generic error handling. The caller gets to decide
> how to handle it.

Although I wonder if we want to allow to opt in into the normal retries,
as for some cases that might actually make sense.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

RE: nvme reservation commands during controller reset

[Engel, Amit]

We saw that in case of path error, nvme reservation cmds does not get error handling.
This is true via the blk layer as well as nvme-cli.

We are working on implementing nvmeof stack and 
this current behavior is blocking our host from running nvme reservation cmds during path error.

-----Original Message-----
From: Christoph Hellwig <hch@infradead.org> 
Sent: Monday, August 10, 2020 3:41 PM
To: Keith Busch
Cc: Engel, Amit; Anner, Ran; linux-nvme@lists.infradead.org
Subject: Re: nvme reservation commands during controller reset


[EXTERNAL EMAIL] 

On Sun, Aug 09, 2020 at 10:10:16AM -0600, Keith Busch wrote:
> On Wed, Aug 05, 2020 at 07:14:22AM +0000, Engel, Amit wrote:
> > What is the reason for setting REQ_FAILFAST_DRIVER flag in nvme_alloc_request ?
> > The original intention was to mark only sync cmds as failfast ?
> 
> Special requests don't get generic error handling. The caller gets to 
> decide how to handle it.

Although I wonder if we want to allow to opt in into the normal retries, as for some cases that might actually make sense.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Keith Busch]

On Mon, Aug 10, 2020 at 03:13:16PM +0000, Engel, Amit wrote:
> We saw that in case of path error, nvme reservation cmds does not get error handling.
> This is true via the blk layer as well as nvme-cli.
> 
> We are working on implementing nvmeof stack and 
> this current behavior is blocking our host from running nvme reservation cmds during path error.

What's blocking you from retrying this command in your host software?

If we truly want to provide a way to hide intermittent errors on
passthrough commands via automatic retry, it may be a little longer
before such an interface hits a released kernel.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Sagi Grimberg]

>> We saw that in case of path error, nvme reservation cmds does not get error handling.
>> This is true via the blk layer as well as nvme-cli.
>>
>> We are working on implementing nvmeof stack and
>> this current behavior is blocking our host from running nvme reservation cmds during path error.
> 
> What's blocking you from retrying this command in your host software?
> 
> If we truly want to provide a way to hide intermittent errors on
> passthrough commands via automatic retry, it may be a little longer
> before such an interface hits a released kernel.

I think we need to keep the passthru commands as they are, but
reservations coming from the pr_ops maybe we can failover these
in nvme_pr_command?

What is the semantics in scsi btw? Didn't see any failover functionality
for pr_ops...

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

RE: nvme reservation commands during controller reset

[Engel, Amit]

In nvme,
cmd_flags is set with REQ_FAILFAST_DRIVER as part of nvme_alloc_request
while in scsi,
sd_pr_command is called with flags = 0

-----Original Message-----
From: Sagi Grimberg <sagi@grimberg.me> 
Sent: Tuesday, August 11, 2020 11:48 PM
To: Keith Busch; Engel, Amit
Cc: Christoph Hellwig; Anner, Ran; linux-nvme@lists.infradead.org
Subject: Re: nvme reservation commands during controller reset


[EXTERNAL EMAIL] 


>> We saw that in case of path error, nvme reservation cmds does not get error handling.
>> This is true via the blk layer as well as nvme-cli.
>>
>> We are working on implementing nvmeof stack and this current behavior 
>> is blocking our host from running nvme reservation cmds during path error.
> 
> What's blocking you from retrying this command in your host software?
> 
> If we truly want to provide a way to hide intermittent errors on 
> passthrough commands via automatic retry, it may be a little longer 
> before such an interface hits a released kernel.

I think we need to keep the passthru commands as they are, but reservations coming from the pr_ops maybe we can failover these in nvme_pr_command?

What is the semantics in scsi btw? Didn't see any failover functionality for pr_ops...
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Christoph Hellwig]

On Tue, Aug 11, 2020 at 01:48:20PM -0700, Sagi Grimberg wrote:
> 
> > > We saw that in case of path error, nvme reservation cmds does not get error handling.
> > > This is true via the blk layer as well as nvme-cli.
> > > 
> > > We are working on implementing nvmeof stack and
> > > this current behavior is blocking our host from running nvme reservation cmds during path error.
> > 
> > What's blocking you from retrying this command in your host software?
> > 
> > If we truly want to provide a way to hide intermittent errors on
> > passthrough commands via automatic retry, it may be a little longer
> > before such an interface hits a released kernel.
> 
> I think we need to keep the passthru commands as they are, but
> reservations coming from the pr_ops maybe we can failover these
> in nvme_pr_command?

Yes, if issued from the multipath device we should try another path.

> 
> What is the semantics in scsi btw? Didn't see any failover functionality
> for pr_ops...

SCSI has no native failover support, but dm-multipath should support
this.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Christoph Hellwig]

Amit,

can you try the branch below?  Pretty much hot off the press, but I
think this should address your problem:

http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/nvme-pr-fix

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Keith Busch]

On Fri, Aug 14, 2020 at 11:09:11AM +0100, Christoph Hellwig wrote:
> Amit,
> 
> can you try the branch below?  Pretty much hot off the press, but I
> think this should address your problem:
> 
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/nvme-pr-fix

Couldn't you just clear the REQ_DRIVER_FAILFAST flag to get the existing
retry mechanisms working in this path?

And while it looks fine for fabrics, PCIe doesn't have the implied HOST
ID connection that makes reservations do the right thing across
different paths. Automatic path failover retries for PCIe may have the
wrong result.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Sagi Grimberg]

> Amit,
> 
> can you try the branch below?  Pretty much hot off the press, but I
> think this should address your problem:
> 
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/nvme-pr-fix

I like the approach, but I think we'll need a bit more path awareness
like we check for available paths if we requeue or fail..

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Christoph Hellwig]

On Fri, Aug 14, 2020 at 09:02:20AM -0700, Keith Busch wrote:
> Couldn't you just clear the REQ_DRIVER_FAILFAST flag to get the existing
> retry mechanisms working in this path?

That would only retry locally and thus not really help.

> 
> And while it looks fine for fabrics, PCIe doesn't have the implied HOST
> ID connection that makes reservations do the right thing across
> different paths. Automatic path failover retries for PCIe may have the
> wrong result.

You need a host id for reservations to work properly, as without that
they won't actually be persistent.  Unfortunasly NVMe allows
reservations without a hostid with some rather broken semantics instead
of just failing them, so applications that do use reservations needs
to ensure a host id is set.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Christoph Hellwig]

On Fri, Aug 14, 2020 at 11:29:57AM -0700, Sagi Grimberg wrote:
> 
> > Amit,
> > 
> > can you try the branch below?  Pretty much hot off the press, but I
> > think this should address your problem:
> > 
> > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/nvme-pr-fix
> 
> I like the approach, but I think we'll need a bit more path awareness
> like we check for available paths if we requeue or fail..

If there is no available path, nvme_find_path will fail and thus we'll
error it.  This is the same mechanism as used by
nvme_ns_head_submit_bio.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Sagi Grimberg]

>>> Amit,
>>>
>>> can you try the branch below?  Pretty much hot off the press, but I
>>> think this should address your problem:
>>>
>>> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/nvme-pr-fix
>>
>> I like the approach, but I think we'll need a bit more path awareness
>> like we check for available paths if we requeue or fail..
> 
> If there is no available path, nvme_find_path will fail and thus we'll
> error it.  This is the same mechanism as used by
> nvme_ns_head_submit_bio.

nvme_find_path will return ns=NULL even if none of the paths is LIVE,
but we want to requeue if we have an available path (e.g. ANA state is
inaccessible temporarily or ctrl state is RESETTING/CONNECTING).

Only if no path is available for request execution we are failing the
request.

If we are providing multipathing for reservations, we should give
the same efforts as we do for normal I/O. This could mean waiting
for some indication on the ns path states (wait for a completion in
nvme_submit_sync_cmd_disk and wake it up in nvme_mpath_set_live).

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Christoph Hellwig]

On Mon, Aug 17, 2020 at 12:56:47AM -0700, Sagi Grimberg wrote:
> 
> > > > Amit,
> > > > 
> > > > can you try the branch below?  Pretty much hot off the press, but I
> > > > think this should address your problem:
> > > > 
> > > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/nvme-pr-fix
> > > 
> > > I like the approach, but I think we'll need a bit more path awareness
> > > like we check for available paths if we requeue or fail..
> > 
> > If there is no available path, nvme_find_path will fail and thus we'll
> > error it.  This is the same mechanism as used by
> > nvme_ns_head_submit_bio.
> 
> nvme_find_path will return ns=NULL even if none of the paths is LIVE,
> but we want to requeue if we have an available path (e.g. ANA state is
> inaccessible temporarily or ctrl state is RESETTING/CONNECTING).

nvme_find_path will return a namespace if there is a namespace with
an optimized or non-optimized state, and which does not have the
NVME_NS_ANA_PENDING or NVME_NS_REMOVING flags set on a controller that is
in the live or deleting states.

And that is exactly what nvme_ns_head_submit_bio relies on.

> Only if no path is available for request execution we are failing the
> request.
> 
> If we are providing multipathing for reservations, we should give
> the same efforts as we do for normal I/O. This could mean waiting
> for some indication on the ns path states (wait for a completion in
> nvme_submit_sync_cmd_disk and wake it up in nvme_mpath_set_live).

The only difference in nvme_ns_head_submit_bio is that the bio is
queued up if the controller is in a suitable state.  But that isn't
something we can implement for the passthrough path as we don't have
an inidividual bio that we can queue up.  Note that not other retries
are handled by the pr path either, so there is the need for some
amount of retrying in the caller anyway.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Sagi Grimberg]

>>>>> Amit,
>>>>>
>>>>> can you try the branch below?  Pretty much hot off the press, but I
>>>>> think this should address your problem:
>>>>>
>>>>> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/nvme-pr-fix
>>>>
>>>> I like the approach, but I think we'll need a bit more path awareness
>>>> like we check for available paths if we requeue or fail..
>>>
>>> If there is no available path, nvme_find_path will fail and thus we'll
>>> error it.  This is the same mechanism as used by
>>> nvme_ns_head_submit_bio.
>>
>> nvme_find_path will return ns=NULL even if none of the paths is LIVE,
>> but we want to requeue if we have an available path (e.g. ANA state is
>> inaccessible temporarily or ctrl state is RESETTING/CONNECTING).
> 
> nvme_find_path will return a namespace if there is a namespace with
> an optimized or non-optimized state, and which does not have the
> NVME_NS_ANA_PENDING or NVME_NS_REMOVING flags set on a controller that is
> in the live or deleting states.
> 
> And that is exactly what nvme_ns_head_submit_bio relies on.

Agree.

>> Only if no path is available for request execution we are failing the
>> request.
>>
>> If we are providing multipathing for reservations, we should give
>> the same efforts as we do for normal I/O. This could mean waiting
>> for some indication on the ns path states (wait for a completion in
>> nvme_submit_sync_cmd_disk and wake it up in nvme_mpath_set_live).
> 
> The only difference in nvme_ns_head_submit_bio is that the bio is
> queued up if the controller is in a suitable state.

These are details, let's first discuss the semantics we should be
providing.

   But that isn't
> something we can implement for the passthrough path as we don't have
> an inidividual bio that we can queue up.  Note that not other retries
> are handled by the pr path either, so there is the need for some
> amount of retrying in the caller anyway.

So what is the semantics we want to provide? a best-effort one? not very
different than what we have today. We could, if we choose to, have the
pr context to wait for a path_map_changed completion and have
nvme_mpath_set_live and nvme_mpath_check_last_path do complete_all on
this completion.

But the question is what the semantics we want to provide...

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Christoph Hellwig]

On Mon, Aug 17, 2020 at 12:29:49PM -0700, Sagi Grimberg wrote:
> So what is the semantics we want to provide? a best-effort one? not very
> different than what we have today. We could, if we choose to, have the
> pr context to wait for a path_map_changed completion and have
> nvme_mpath_set_live and nvme_mpath_check_last_path do complete_all on
> this completion.
> 
> But the question is what the semantics we want to provide...

But it might never change..  We could give it the default max_retries
based retries for any reasons if we want to be fancy.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: nvme reservation commands during controller reset

[Sagi Grimberg]

>> So what is the semantics we want to provide? a best-effort one? not very
>> different than what we have today. We could, if we choose to, have the
>> pr context to wait for a path_map_changed completion and have
>> nvme_mpath_set_live and nvme_mpath_check_last_path do complete_all on
>> this completion.
>>
>> But the question is what the semantics we want to provide...
> 
> But it might never change..  We could give it the default max_retries
> based retries for any reasons if we want to be fancy.

Is there a difference between sync read and pr ioctl on the mpath device 
node?

Currently the read will block until it absolutely cannot issue the
request, but the ioctl will block until there is no available path.

I think we should have the same semantics for both. Unless convinced
otherwise..

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme