[PATCH] nvme: translate zns errors to blk_status_t

[PATCH] nvme: translate zns errors to blk_status_t

[Keith Busch]

EBUSY is the appropriate errno for commands that complete with zone
resource errors.

Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index ea1fa41fbba8..aff556837115 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -249,6 +249,9 @@ static blk_status_t nvme_error_status(u16 status)
 		return BLK_STS_NEXUS;
 	case NVME_SC_HOST_PATH_ERROR:
 		return BLK_STS_TRANSPORT;
+	case NVME_SC_ZONE_TOO_MANY_ACTIVE:
+	case NVME_SC_ZONE_TOO_MANY_OPEN:
+		return BLK_STS_DEV_RESOURCE;
 	default:
 		return BLK_STS_IOERR;
 	}
-- 
2.24.1


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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Damien Le Moal]

On 2020/09/10 5:38, Keith Busch wrote:
> EBUSY is the appropriate errno for commands that complete with zone
> resource errors.
> 
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>  drivers/nvme/host/core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index ea1fa41fbba8..aff556837115 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -249,6 +249,9 @@ static blk_status_t nvme_error_status(u16 status)
>  		return BLK_STS_NEXUS;
>  	case NVME_SC_HOST_PATH_ERROR:
>  		return BLK_STS_TRANSPORT;
> +	case NVME_SC_ZONE_TOO_MANY_ACTIVE:
> +	case NVME_SC_ZONE_TOO_MANY_OPEN:
> +		return BLK_STS_DEV_RESOURCE;
>  	default:
>  		return BLK_STS_IOERR;
>  	}
> 

Perfect ! That fits nicely with what Johannes is working on with zonefs explicit
zone open. I will check the scsi side to make sure the same is returned for too
many open zones case.

Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>

-- 
Damien Le Moal
Western Digital Research

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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Christoph Hellwig]

On Wed, Sep 09, 2020 at 01:33:24PM -0700, Keith Busch wrote:
> EBUSY is the appropriate errno for commands that complete with zone
> resource errors.
> 
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>  drivers/nvme/host/core.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index ea1fa41fbba8..aff556837115 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -249,6 +249,9 @@ static blk_status_t nvme_error_status(u16 status)
>  		return BLK_STS_NEXUS;
>  	case NVME_SC_HOST_PATH_ERROR:
>  		return BLK_STS_TRANSPORT;
> +	case NVME_SC_ZONE_TOO_MANY_ACTIVE:
> +	case NVME_SC_ZONE_TOO_MANY_OPEN:
> +		return BLK_STS_DEV_RESOURCE;

I'm not sure this is the best idea, we probably need specific error codes
if we want file systems to be aware of the limit.

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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Keith Busch]

On Thu, Sep 10, 2020 at 07:26:43AM +0200, Christoph Hellwig wrote:
> On Wed, Sep 09, 2020 at 01:33:24PM -0700, Keith Busch wrote:
> > EBUSY is the appropriate errno for commands that complete with zone
> > resource errors.
> > 
> > Signed-off-by: Keith Busch <kbusch@kernel.org>
> > ---
> >  drivers/nvme/host/core.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index ea1fa41fbba8..aff556837115 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -249,6 +249,9 @@ static blk_status_t nvme_error_status(u16 status)
> >  		return BLK_STS_NEXUS;
> >  	case NVME_SC_HOST_PATH_ERROR:
> >  		return BLK_STS_TRANSPORT;
> > +	case NVME_SC_ZONE_TOO_MANY_ACTIVE:
> > +	case NVME_SC_ZONE_TOO_MANY_OPEN:
> > +		return BLK_STS_DEV_RESOURCE;
> 
> I'm not sure this is the best idea, we probably need specific error codes
> if we want file systems to be aware of the limit.

I was more considering the errno the user sees going direct to the
device.

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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Damien Le Moal]

On 2020/09/10 14:34, Christoph Hellwig wrote:
> On Wed, Sep 09, 2020 at 01:33:24PM -0700, Keith Busch wrote:
>> EBUSY is the appropriate errno for commands that complete with zone
>> resource errors.
>>
>> Signed-off-by: Keith Busch <kbusch@kernel.org>
>> ---
>>  drivers/nvme/host/core.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index ea1fa41fbba8..aff556837115 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -249,6 +249,9 @@ static blk_status_t nvme_error_status(u16 status)
>>  		return BLK_STS_NEXUS;
>>  	case NVME_SC_HOST_PATH_ERROR:
>>  		return BLK_STS_TRANSPORT;
>> +	case NVME_SC_ZONE_TOO_MANY_ACTIVE:
>> +	case NVME_SC_ZONE_TOO_MANY_OPEN:
>> +		return BLK_STS_DEV_RESOURCE;
> 
> I'm not sure this is the best idea, we probably need specific error codes
> if we want file systems to be aware of the limit.

Do you mean something else than -EBUSY being returned to the user by the block
layer ? Or a different/specific BLK_STS_XXX code which translates into -EBUSY in
blk_status_to_errno() ?



-- 
Damien Le Moal
Western Digital Research

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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Christoph Hellwig]

On Thu, Sep 10, 2020 at 10:25:24PM +0000, Damien Le Moal wrote:
> > I'm not sure this is the best idea, we probably need specific error codes
> > if we want file systems to be aware of the limit.
> 
> Do you mean something else than -EBUSY being returned to the user by the block
> layer ? Or a different/specific BLK_STS_XXX code which translates into -EBUSY in
> blk_status_to_errno() ?

My primary aim is a different BLK_STS_ code.  But given that I think
that we should not map different BLK_STS_ codes to the same errno if
we can avoid it, we should probably also use a different errno.

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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Damien Le Moal]

On 2020/09/11 15:10, Christoph Hellwig wrote:
> On Thu, Sep 10, 2020 at 10:25:24PM +0000, Damien Le Moal wrote:
>>> I'm not sure this is the best idea, we probably need specific error codes
>>> if we want file systems to be aware of the limit.
>>
>> Do you mean something else than -EBUSY being returned to the user by the block
>> layer ? Or a different/specific BLK_STS_XXX code which translates into -EBUSY in
>> blk_status_to_errno() ?
> 
> My primary aim is a different BLK_STS_ code.  But given that I think
> that we should not map different BLK_STS_ codes to the same errno if
> we can avoid it, we should probably also use a different errno.
> 

Hmm. Beside EBUSY, I do not see a nice existing errno that we can reuse. May be
ENOSR (out of stream resources) ? Or do we define a new one ? EZBUSY ?

And indeed if we define a new BLK_STS_ZONE_RESOURCE and map that to EBUSY,
errno_to_blkstatus() will get confused.


-- 
Damien Le Moal
Western Digital Research

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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Keith Busch]

On Fri, Sep 11, 2020 at 09:35:31AM +0000, Damien Le Moal wrote:
> On 2020/09/11 15:10, Christoph Hellwig wrote:
> > On Thu, Sep 10, 2020 at 10:25:24PM +0000, Damien Le Moal wrote:
> >>> I'm not sure this is the best idea, we probably need specific error codes
> >>> if we want file systems to be aware of the limit.
> >>
> >> Do you mean something else than -EBUSY being returned to the user by the block
> >> layer ? Or a different/specific BLK_STS_XXX code which translates into -EBUSY in
> >> blk_status_to_errno() ?
> > 
> > My primary aim is a different BLK_STS_ code.  But given that I think
> > that we should not map different BLK_STS_ codes to the same errno if
> > we can avoid it, we should probably also use a different errno.
> > 
> 
> Hmm. Beside EBUSY, I do not see a nice existing errno that we can reuse. May be
> ENOSR (out of stream resources) ? Or do we define a new one ? EZBUSY ?
> 
> And indeed if we define a new BLK_STS_ZONE_RESOURCE and map that to EBUSY,
> errno_to_blkstatus() will get confused.

It's not often I get to help define a new errno, so I'm a bit cautious
of that route. But if that's the recommendation, then sure, sounds fun.

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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Christoph Hellwig]

On Fri, Sep 11, 2020 at 09:35:31AM +0000, Damien Le Moal wrote:
> On 2020/09/11 15:10, Christoph Hellwig wrote:
> > On Thu, Sep 10, 2020 at 10:25:24PM +0000, Damien Le Moal wrote:
> >>> I'm not sure this is the best idea, we probably need specific error codes
> >>> if we want file systems to be aware of the limit.
> >>
> >> Do you mean something else than -EBUSY being returned to the user by the block
> >> layer ? Or a different/specific BLK_STS_XXX code which translates into -EBUSY in
> >> blk_status_to_errno() ?
> > 
> > My primary aim is a different BLK_STS_ code.  But given that I think
> > that we should not map different BLK_STS_ codes to the same errno if
> > we can avoid it, we should probably also use a different errno.
> > 
> 
> Hmm. Beside EBUSY, I do not see a nice existing errno that we can reuse. May be
> ENOSR (out of stream resources) ? Or do we define a new one ? EZBUSY ?
> 
> And indeed if we define a new BLK_STS_ZONE_RESOURCE and map that to EBUSY,
> errno_to_blkstatus() will get confused.

Yes, you need to pick a new error.  I don't really care which one,
most of the mappings are pretty odd anyway.

> 
> 
> -- 
> Damien Le Moal
> Western Digital Research
---end quoted text---

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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Keith Busch]

On Sat, Sep 12, 2020 at 08:41:04AM +0200, Christoph Hellwig wrote:
> On Fri, Sep 11, 2020 at 09:35:31AM +0000, Damien Le Moal wrote:
> > On 2020/09/11 15:10, Christoph Hellwig wrote:
> > > On Thu, Sep 10, 2020 at 10:25:24PM +0000, Damien Le Moal wrote:
> > >>> I'm not sure this is the best idea, we probably need specific error codes
> > >>> if we want file systems to be aware of the limit.
> > >>
> > >> Do you mean something else than -EBUSY being returned to the user by the block
> > >> layer ? Or a different/specific BLK_STS_XXX code which translates into -EBUSY in
> > >> blk_status_to_errno() ?
> > > 
> > > My primary aim is a different BLK_STS_ code.  But given that I think
> > > that we should not map different BLK_STS_ codes to the same errno if
> > > we can avoid it, we should probably also use a different errno.
> > > 
> > 
> > Hmm. Beside EBUSY, I do not see a nice existing errno that we can reuse. May be
> > ENOSR (out of stream resources) ? Or do we define a new one ? EZBUSY ?
> > 
> > And indeed if we define a new BLK_STS_ZONE_RESOURCE and map that to EBUSY,
> > errno_to_blkstatus() will get confused.
> 
> Yes, you need to pick a new error.  I don't really care which one,
> most of the mappings are pretty odd anyway.

Selecting an existing unused errno feels a bit arbitrary for a status
that is currently specific to NVMe ZNS. Could you provide a little more
guidance on what you're looking for?

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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Damien Le Moal]

On 2020/09/15 4:44, Keith Busch wrote:
> On Sat, Sep 12, 2020 at 08:41:04AM +0200, Christoph Hellwig wrote:
>> On Fri, Sep 11, 2020 at 09:35:31AM +0000, Damien Le Moal wrote:
>>> On 2020/09/11 15:10, Christoph Hellwig wrote:
>>>> On Thu, Sep 10, 2020 at 10:25:24PM +0000, Damien Le Moal wrote:
>>>>>> I'm not sure this is the best idea, we probably need specific error codes
>>>>>> if we want file systems to be aware of the limit.
>>>>>
>>>>> Do you mean something else than -EBUSY being returned to the user by the block
>>>>> layer ? Or a different/specific BLK_STS_XXX code which translates into -EBUSY in
>>>>> blk_status_to_errno() ?
>>>>
>>>> My primary aim is a different BLK_STS_ code.  But given that I think
>>>> that we should not map different BLK_STS_ codes to the same errno if
>>>> we can avoid it, we should probably also use a different errno.
>>>>
>>>
>>> Hmm. Beside EBUSY, I do not see a nice existing errno that we can reuse. May be
>>> ENOSR (out of stream resources) ? Or do we define a new one ? EZBUSY ?
>>>
>>> And indeed if we define a new BLK_STS_ZONE_RESOURCE and map that to EBUSY,
>>> errno_to_blkstatus() will get confused.
>>
>> Yes, you need to pick a new error.  I don't really care which one,
>> most of the mappings are pretty odd anyway.
> 
> Selecting an existing unused errno feels a bit arbitrary for a status
> that is currently specific to NVMe ZNS. Could you provide a little more
> guidance on what you're looking for?

Not just NVMe ZNS. SCSI/ZBC and ATA/ZAC too (both sd driver) :)



-- 
Damien Le Moal
Western Digital Research

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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Keith Busch]

On Mon, Sep 14, 2020 at 11:25:08PM +0000, Damien Le Moal wrote:
> On 2020/09/15 4:44, Keith Busch wrote:
> > Selecting an existing unused errno feels a bit arbitrary for a status
> > that is currently specific to NVMe ZNS. Could you provide a little more
> > guidance on what you're looking for?
> 
> Not just NVMe ZNS. SCSI/ZBC and ATA/ZAC too (both sd driver) :)

Gotcha, I did not realize those may have open zone limits too.

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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Damien Le Moal]

On 2020/09/15 11:55, Keith Busch wrote:
> On Mon, Sep 14, 2020 at 11:25:08PM +0000, Damien Le Moal wrote:
>> On 2020/09/15 4:44, Keith Busch wrote:
>>> Selecting an existing unused errno feels a bit arbitrary for a status
>>> that is currently specific to NVMe ZNS. Could you provide a little more
>>> guidance on what you're looking for?
>>
>> Not just NVMe ZNS. SCSI/ZBC and ATA/ZAC too (both sd driver) :)
> 
> Gotcha, I did not realize those may have open zone limits too.

ZBC/ZAC both define a max open zone limit too, which is semantically equivalent
to ZNS definition. ZBC and ZAC do not define the active zone concept though, so
no limit set for max_active_zones.


-- 
Damien Le Moal
Western Digital Research

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

Re: [PATCH] nvme: translate zns errors to blk_status_t

[Christoph Hellwig]

On Tue, Sep 15, 2020 at 04:44:21AM +0900, Keith Busch wrote:
> > Yes, you need to pick a new error.  I don't really care which one,
> > most of the mappings are pretty odd anyway.
> 
> Selecting an existing unused errno feels a bit arbitrary for a status
> that is currently specific to NVMe ZNS. Could you provide a little more
> guidance on what you're looking for?

As said before: most of the errno mappins are pretty arbitrary, e.g.
take a look at the -ENOLINK, -EREMOTEIO, -EBADE, -ENODATA, -EILSEQ,
or -EREMCHG mappings.

So I think just picking a value like ECHRNG or EBADSLT or EUSERS
should be perfectly fine.

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