2019-11-05 06:17:10

by Charles Machalow

[permalink] [raw]
Subject: [PATCH] nvme: change nvme_passthru_cmd64 to explicitly mark rsvd

Changing nvme_passthru_cmd64 to add a field: rsvd2. This field is an explicit
marker for the padding space added on certain platforms as a result of the
enlargement of the result field from 32 bit to 64 bits in size.
---
include/uapi/linux/nvme_ioctl.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/nvme_ioctl.h b/include/uapi/linux/nvme_ioctl.h
index e168dc59e..d99b5a772 100644
--- a/include/uapi/linux/nvme_ioctl.h
+++ b/include/uapi/linux/nvme_ioctl.h
@@ -63,6 +63,7 @@ struct nvme_passthru_cmd64 {
__u32 cdw14;
__u32 cdw15;
__u32 timeout_ms;
+ __u32 rsvd2;
__u64 result;
};

--
2.17.1


2019-11-05 07:42:30

by Marta Rybczynska

[permalink] [raw]
Subject: Re: [PATCH] nvme: change nvme_passthru_cmd64 to explicitly mark rsvd



----- On 5 Nov, 2019, at 07:15, Charles Machalow [email protected] wrote:

> Changing nvme_passthru_cmd64 to add a field: rsvd2. This field is an explicit
> marker for the padding space added on certain platforms as a result of the
> enlargement of the result field from 32 bit to 64 bits in size.
> ---
> include/uapi/linux/nvme_ioctl.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/uapi/linux/nvme_ioctl.h b/include/uapi/linux/nvme_ioctl.h
> index e168dc59e..d99b5a772 100644
> --- a/include/uapi/linux/nvme_ioctl.h
> +++ b/include/uapi/linux/nvme_ioctl.h
> @@ -63,6 +63,7 @@ struct nvme_passthru_cmd64 {
> __u32 cdw14;
> __u32 cdw15;
> __u32 timeout_ms;
> + __u32 rsvd2;
> __u64 result;
> };
>

Looks good to me. However, please note that the new ioctl made it already to 5.3.8.

Regards,
Marta

2019-11-05 14:38:44

by Christoph Hellwig

[permalink] [raw]

2019-11-05 15:31:11

by Keith Busch

[permalink] [raw]
Subject: Re: [PATCH] nvme: change nvme_passthru_cmd64 to explicitly mark rsvd

On Mon, Nov 04, 2019 at 10:15:10PM -0800, Charles Machalow wrote:
> Changing nvme_passthru_cmd64 to add a field: rsvd2. This field is an explicit
> marker for the padding space added on certain platforms as a result of the
> enlargement of the result field from 32 bit to 64 bits in size.

Charles,
Could you reply with your "Signed-off-by" so I can apply this patch?
Thanks.

2019-11-05 15:32:58

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] nvme: change nvme_passthru_cmd64 to explicitly mark rsvd

On Tue, Nov 05, 2019 at 08:39:12AM +0100, Marta Rybczynska wrote:
> Looks good to me. However, please note that the new ioctl made it already to 5.3.8.

It wasn't in 5.3, but it seems like you are right and it somehow got
picked for the stable releases.

Sasha, can you please revert 76d609da9ed1cc0dc780e2b539d7b827ce28f182
in 5.3-stable ASAP and make sure crap like backporting new ABIs that
haven't seen a release yet is never ever going to happen again?

2019-11-05 20:58:13

by Charles Machalow

[permalink] [raw]
Subject: Re: [PATCH] nvme: change nvme_passthru_cmd64 to explicitly mark rsvd

Sorry, still new to this process.

Signed-off-by: Charles Machalow <[email protected]>

- Charlie Scott Machalow


On Tue, Nov 5, 2019 at 7:26 AM Keith Busch <[email protected]> wrote:
>
> On Mon, Nov 04, 2019 at 10:15:10PM -0800, Charles Machalow wrote:
> > Changing nvme_passthru_cmd64 to add a field: rsvd2. This field is an explicit
> > marker for the padding space added on certain platforms as a result of the
> > enlargement of the result field from 32 bit to 64 bits in size.
>
> Charles,
> Could you reply with your "Signed-off-by" so I can apply this patch?
> Thanks.

2019-11-05 21:23:05

by Keith Busch

[permalink] [raw]
Subject: Re: [PATCH] nvme: change nvme_passthru_cmd64 to explicitly mark rsvd

Thanks, applied for the next 5.4-rc pull with an updated changelog to
indicate the "Fixes" commit since not having the padding does indeed
break the compat ioctl.

2019-11-06 00:12:22

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH] nvme: change nvme_passthru_cmd64 to explicitly mark rsvd

On Tue, Nov 05, 2019 at 04:31:44PM +0100, Christoph Hellwig wrote:
>On Tue, Nov 05, 2019 at 08:39:12AM +0100, Marta Rybczynska wrote:
>> Looks good to me. However, please note that the new ioctl made it already to 5.3.8.
>
>It wasn't in 5.3, but it seems like you are right and it somehow got
>picked for the stable releases.
>
>Sasha, can you please revert 76d609da9ed1cc0dc780e2b539d7b827ce28f182
>in 5.3-stable ASAP and make sure crap like backporting new ABIs that
>haven't seen a release yet is never ever going to happen again?

Sure, I'll revert it. I guess I wasn't expecting to see something like
this in a -rc release. How did it make it into one if it's not a fix?

--
Thanks,
Sasha

2019-11-06 00:19:42

by Keith Busch

[permalink] [raw]
Subject: Re: [PATCH] nvme: change nvme_passthru_cmd64 to explicitly mark rsvd

On Tue, Nov 05, 2019 at 07:08:36PM -0500, Sasha Levin wrote:
> On Tue, Nov 05, 2019 at 04:31:44PM +0100, Christoph Hellwig wrote:
> > On Tue, Nov 05, 2019 at 08:39:12AM +0100, Marta Rybczynska wrote:
> > > Looks good to me. However, please note that the new ioctl made it already to 5.3.8.
> >
> > It wasn't in 5.3, but it seems like you are right and it somehow got
> > picked for the stable releases.
> >
> > Sasha, can you please revert 76d609da9ed1cc0dc780e2b539d7b827ce28f182
> > in 5.3-stable ASAP and make sure crap like backporting new ABIs that
> > haven't seen a release yet is never ever going to happen again?
>
> Sure, I'll revert it. I guess I wasn't expecting to see something like
> this in a -rc release. How did it make it into one if it's not a fix?

It is a fix, but it wasn't marked as such in the original changelog.
I've adjusted it for the pull request, currently staged here:

https://git.kernel.dk/cgit/linux-block/commit/?h=for-linus&id=0d6eeb1fd625272bd60d25f2d5e116cf582fc7dc

Still, a new ioctl seems pretty aggressive for stable bot, yeah?

2019-11-06 01:13:18

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] nvme: change nvme_passthru_cmd64 to explicitly mark rsvd

On Tue, Nov 05, 2019 at 07:08:36PM -0500, Sasha Levin wrote:
> On Tue, Nov 05, 2019 at 04:31:44PM +0100, Christoph Hellwig wrote:
>> On Tue, Nov 05, 2019 at 08:39:12AM +0100, Marta Rybczynska wrote:
>>> Looks good to me. However, please note that the new ioctl made it already to 5.3.8.
>>
>> It wasn't in 5.3, but it seems like you are right and it somehow got
>> picked for the stable releases.
>>
>> Sasha, can you please revert 76d609da9ed1cc0dc780e2b539d7b827ce28f182
>> in 5.3-stable ASAP and make sure crap like backporting new ABIs that
>> haven't seen a release yet is never ever going to happen again?
>
> Sure, I'll revert it. I guess I wasn't expecting to see something like
> this in a -rc release. How did it make it into one if it's not a fix?

76d609da9ed1cc0dc780e2b539d7b827ce28f182 is a backport of
65e68edce0db433aa0c2b26d7dc14fbbbeb89fbb, which went into 5.4-rc2 and
was not marked for stable. It might kinda bend the normal merge
window rules a little, but I don't see how it could be considered
something to backport.