2024-04-16 14:28:46

by Lennart Poettering

[permalink] [raw]
Subject: Re: API break, sysfs "capability" file

On Di, 16.04.24 08:22, Jens Axboe ([email protected]) wrote:

> On 4/16/24 8:18 AM, Lennart Poettering wrote:
> > On Di, 09.04.24 09:17, Jens Axboe ([email protected]) wrote:
> >
> >> On 4/9/24 8:15 AM, Christoph Hellwig wrote:
> >>> On Tue, Apr 09, 2024 at 10:19:09AM +0200, Lennart Poettering wrote:
> >>>> All I am looking for is a very simple test that returns me a boolean:
> >>>> is there kernel-level partition scanning enabled on this device or
> >>>> not.
> >>>
> >>> And we can add a trivial sysfs attribute for that.
> >>
> >> And I think we should. I don't know what was being smoked adding a sysfs
> >> interface that exposed internal flag values - and honestly what was
> >> being smoked to rely on that, but I think it's fair to say that the
> >> majority of the fuckup here is on the kernel side.
> >
> > Yeah, it's a shitty interface, the kernel is rich in that. But it was
> > excessively well documented, better in fact than almost all other
> > kernel interfaces:
> >
> > ? https://www.kernel.org/doc/html/v5.16/block/capability.html ?
> >
> > If you document something on so much detail in the API docs, how do
> > you expect this *not* to be relied on by userspace.
>
> This is _internal_ documentation, not user ABI documentation. The fact
> that it's talking about internal flag values should make that clear,
> though I can definitely see how that's just badly exposed along with
> other things that document things that users/admins could care about.

The text begins with:

"This file documents the sysfs file block/<disk>/capability."

So it makes very clear this is about the sysfs interface.

Are you saying that sysfs of the block layer should be considered an
*internal* kernel API? That's a wild definition, if I may say so.

Lennart

--
Lennart Poettering, Berlin


2024-04-16 14:33:54

by Jens Axboe

[permalink] [raw]
Subject: Re: API break, sysfs "capability" file

On 4/16/24 8:25 AM, Lennart Poettering wrote:
> On Di, 16.04.24 08:22, Jens Axboe ([email protected]) wrote:
>
>> On 4/16/24 8:18 AM, Lennart Poettering wrote:
>>> On Di, 09.04.24 09:17, Jens Axboe ([email protected]) wrote:
>>>
>>>> On 4/9/24 8:15 AM, Christoph Hellwig wrote:
>>>>> On Tue, Apr 09, 2024 at 10:19:09AM +0200, Lennart Poettering wrote:
>>>>>> All I am looking for is a very simple test that returns me a boolean:
>>>>>> is there kernel-level partition scanning enabled on this device or
>>>>>> not.
>>>>>
>>>>> And we can add a trivial sysfs attribute for that.
>>>>
>>>> And I think we should. I don't know what was being smoked adding a sysfs
>>>> interface that exposed internal flag values - and honestly what was
>>>> being smoked to rely on that, but I think it's fair to say that the
>>>> majority of the fuckup here is on the kernel side.
>>>
>>> Yeah, it's a shitty interface, the kernel is rich in that. But it was
>>> excessively well documented, better in fact than almost all other
>>> kernel interfaces:
>>>
>>> ? https://www.kernel.org/doc/html/v5.16/block/capability.html ?
>>>
>>> If you document something on so much detail in the API docs, how do
>>> you expect this *not* to be relied on by userspace.
>>
>> This is _internal_ documentation, not user ABI documentation. The fact
>> that it's talking about internal flag values should make that clear,
>> though I can definitely see how that's just badly exposed along with
>> other things that document things that users/admins could care about.
>
> The text begins with:
>
> "This file documents the sysfs file block/<disk>/capability."
>
> So it makes very clear this is about the sysfs interface.
>
> Are you saying that sysfs of the block layer should be considered an
> *internal* kernel API? That's a wild definition, if I may say so.

No I missed that - to me it's clearly internal documentation as it's
talking about the flags, but yeah you are right it's being presented as
sysfs documentation for the 'capability' file. That should never have
gone into the tree as ABI documentation.

Doesn't really change my conclusion from earlier. As mentioned, this is
clearly a kernel fuckup, and honestly since it's being presented as ABI,
we definitely need to rectify this and provide an alternative. Even
though I'm not a huge fan of it, might just be best to re-introduce
'capability' and just have conversions of the flags so we retain the
user side of it the same. That can then also go into stable, so we'll
end up with something that at least looks cohesive on the user side.

--
Jens Axboe


Subject: Re: API break, sysfs "capability" file

On 16.04.24 16:33, Jens Axboe wrote:
> On 4/16/24 8:25 AM, Lennart Poettering wrote:
>> On Di, 16.04.24 08:22, Jens Axboe ([email protected]) wrote:
>>> On 4/16/24 8:18 AM, Lennart Poettering wrote:
>>>> On Di, 09.04.24 09:17, Jens Axboe ([email protected]) wrote:
>>>>
>>>>> On 4/9/24 8:15 AM, Christoph Hellwig wrote:
>>>>>> On Tue, Apr 09, 2024 at 10:19:09AM +0200, Lennart Poettering wrote:
>>>>>>> All I am looking for is a very simple test that returns me a boolean:
>>>>>>> is there kernel-level partition scanning enabled on this device or
>>>>>>> not.
>>>>>> And we can add a trivial sysfs attribute for that.
>>>>>
>>>>> And I think we should. I don't know what was being smoked adding a sysfs
>>>>> interface that exposed internal flag values - and honestly what was
>>>>> being smoked to rely on that, but I think it's fair to say that the
>>>>> majority of the fuckup here is on the kernel side.
> [...]
> Doesn't really change my conclusion from earlier. As mentioned, this is
> clearly a kernel fuckup, and honestly since it's being presented as ABI,
> we definitely need to rectify this and provide an alternative. Even
> though I'm not a huge fan of it, might just be best to re-introduce
> 'capability' and just have conversions of the flags so we retain the
> user side of it the same. That can then also go into stable, so we'll
> end up with something that at least looks cohesive on the user side.

Jens, quick question: what's the plan forward here and who will realize
what you outlined?

I'm asking, as afaics nothing happened for a week (hope I didn't miss
anything!). Sure, it's not a regression from the last cycle or so, so
it's not that urgent. But hch's "It is not a regression at all" last
week made me worry that this in the end might not be solved unless
somebody has it on the todo list. Normally I would have CCed Linus at
that point anyway to get his stance, but from your statements it sounds
like this is unnecessary here.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

2024-04-25 13:32:17

by Christoph Hellwig

[permalink] [raw]
Subject: Re: API break, sysfs "capability" file

On Wed, Apr 24, 2024 at 10:09:35AM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> Jens, quick question: what's the plan forward here and who will realize
> what you outlined?
>
> I'm asking, as afaics nothing happened for a week (hope I didn't miss
> anything!). Sure, it's not a regression from the last cycle or so, so
> it's not that urgent. But hch's "It is not a regression at all" last
> week made me worry that this in the end might not be solved unless
> somebody has it on the todo list. Normally I would have CCed Linus at
> that point anyway to get his stance, but from your statements it sounds
> like this is unnecessary here.

I'll get to adding a real interface in a bit. Sorry, I've been
a little too busy the last days.