* 2016 Changes to nvme.h
@ 2019-01-24 16:59 Wilson, Ellis
2019-01-24 17:21 ` Keith Busch
0 siblings, 1 reply; 4+ messages in thread
From: Wilson, Ellis @ 2019-01-24 16:59 UTC (permalink / raw)
Hi all,
I am looking for further insight into the change, which just caused some
issues with code I'm working with when we moved to a new kernel. It is
identified on this page:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9d99a8dda154f38307d43d9c9aa504bd3703d596
In essence this pulls 90+% of the structs/enums/etc into the kernel
sources, and leaves only the most basic interface in nvme_uapi.h. This
forces one to privately re-implement the structs/enums/etc that
previously existed in nvme.h, which previously was available in the
normal include path. Fortunately all such data structures are described
clearly in the NVMe specs, but it still feels like work without much
reason. This is especially the case since the author calls out that the
only public user of the old header file (nvme cli) already had a copy of
it in their sources locally, so it wouldn't break them. This points out
the reliance/expectation of external users on the structs expected to be
passed through the uapi.
Is there rationale behind this that's not obvious to me? Any insight is
appreciated. I admit I don't live in kernel code on a daily basis so
it's possible I'm being naive about something -- apologies in advance if
that's the case.
Thanks!
ellis
^ permalink raw reply [flat|nested] 4+ messages in thread
* 2016 Changes to nvme.h
2019-01-24 16:59 2016 Changes to nvme.h Wilson, Ellis
@ 2019-01-24 17:21 ` Keith Busch
2019-01-24 21:38 ` Wilson, Ellis
0 siblings, 1 reply; 4+ messages in thread
From: Keith Busch @ 2019-01-24 17:21 UTC (permalink / raw)
On Thu, Jan 24, 2019@04:59:58PM +0000, Wilson, Ellis wrote:
> Hi all,
>
> I am looking for further insight into the change, which just caused some
> issues with code I'm working with when we moved to a new kernel. It is
> identified on this page:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9d99a8dda154f38307d43d9c9aa504bd3703d596
>
> In essence this pulls 90+% of the structs/enums/etc into the kernel
> sources, and leaves only the most basic interface in nvme_uapi.h. This
> forces one to privately re-implement the structs/enums/etc that
> previously existed in nvme.h, which previously was available in the
> normal include path. Fortunately all such data structures are described
> clearly in the NVMe specs, but it still feels like work without much
> reason. This is especially the case since the author calls out that the
> only public user of the old header file (nvme cli) already had a copy of
> it in their sources locally, so it wouldn't break them. This points out
> the reliance/expectation of external users on the structs expected to be
> passed through the uapi.
>
> Is there rationale behind this that's not obvious to me? Any insight is
> appreciated. I admit I don't live in kernel code on a daily basis so
> it's possible I'm being naive about something -- apologies in advance if
> that's the case.
As the spec defines new fields and user tooling wants to user them,
it is just not a good idea to rely on the kernel to own updating the
structures when the kernel has no internal need for it. It's better
for user space to control its own needs independent from the kernel's.
This could really be a user space library. I had a request to separate
nvme-cli parts into a library that included the spec structures, but I
admit I dropped the ball on that...
^ permalink raw reply [flat|nested] 4+ messages in thread
* 2016 Changes to nvme.h
2019-01-24 17:21 ` Keith Busch
@ 2019-01-24 21:38 ` Wilson, Ellis
2019-01-24 22:21 ` Keith Busch
0 siblings, 1 reply; 4+ messages in thread
From: Wilson, Ellis @ 2019-01-24 21:38 UTC (permalink / raw)
On 1/24/19 12:21 PM, Keith Busch wrote:
> On Thu, Jan 24, 2019@04:59:58PM +0000, Wilson, Ellis wrote:
>> Hi all,
>>
>> I am looking for further insight into the change, which just caused some
>> issues with code I'm working with when we moved to a new kernel. It is
>> identified on this page:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9d99a8dda154f38307d43d9c9aa504bd3703d596
>>
>> In essence this pulls 90+% of the structs/enums/etc into the kernel
>> sources, and leaves only the most basic interface in nvme_uapi.h. This
>> forces one to privately re-implement the structs/enums/etc that
>> previously existed in nvme.h, which previously was available in the
>> normal include path. Fortunately all such data structures are described
>> clearly in the NVMe specs, but it still feels like work without much
>> reason. This is especially the case since the author calls out that the
>> only public user of the old header file (nvme cli) already had a copy of
>> it in their sources locally, so it wouldn't break them. This points out
>> the reliance/expectation of external users on the structs expected to be
>> passed through the uapi.
>>
>> Is there rationale behind this that's not obvious to me? Any insight is
>> appreciated. I admit I don't live in kernel code on a daily basis so
>> it's possible I'm being naive about something -- apologies in advance if
>> that's the case.
>
> As the spec defines new fields and user tooling wants to user them,
> it is just not a good idea to rely on the kernel to own updating the
> structures when the kernel has no internal need for it. It's better
> for user space to control its own needs independent from the kernel's.
>
> This could really be a user space library. I had a request to separate
> nvme-cli parts into a library that included the spec structures, but I
> admit I dropped the ball on that...
Hi Keith,
Thank you for the response. I think I understand the intent now. Your
comment about a userspace library makes total sense. I still disagree
with one thing. You say, "it is just not a good idea to rely on the
kernel to own updating the structures when the kernel has no internal
need for it." And yet, those very structures were moved further
/inside/, rather than being deleted wholesale, so the action doesn't
quite match up with your explanation from what I can tell.
I dug into the code and I see pci.c and scsi.c both rely on many of
these structs and enums, which explains why it was pulled in rather than
being removed entirely. I think I follow now, and agree it makes sense
to disentangle the userspace code from the kernel code to prevent
forcing everybody to move forward in lock-step in the face of changing
spec structures. Both need and rely on these structures, but can be
off-version with regard to one another without mishap.
Thanks for your time,
ellis
^ permalink raw reply [flat|nested] 4+ messages in thread
* 2016 Changes to nvme.h
2019-01-24 21:38 ` Wilson, Ellis
@ 2019-01-24 22:21 ` Keith Busch
0 siblings, 0 replies; 4+ messages in thread
From: Keith Busch @ 2019-01-24 22:21 UTC (permalink / raw)
On Thu, Jan 24, 2019@09:38:11PM +0000, Wilson, Ellis wrote:
> Thank you for the response. I think I understand the intent now. Your
> comment about a userspace library makes total sense. I still disagree
> with one thing. You say, "it is just not a good idea to rely on the
> kernel to own updating the structures when the kernel has no internal
> need for it." And yet, those very structures were moved further
> /inside/, rather than being deleted wholesale, so the action doesn't
> quite match up with your explanation from what I can tell.
We do have in-kernel uses for nvme structures, hence the relocation to
an internal definition. We just don't want to export it for user space,
which would (and did) create undue maintenance to non-kernel usage.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-01-24 22:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-24 16:59 2016 Changes to nvme.h Wilson, Ellis
2019-01-24 17:21 ` Keith Busch
2019-01-24 21:38 ` Wilson, Ellis
2019-01-24 22:21 ` Keith Busch
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.