On Mon, May 02, 2022 at 05:43:21PM +0200, Lennart Poettering wrote:
> On Mo, 02.05.22 17:30, Jason A. Donenfeld ([email protected]) wrote:
>
> > Just wanted to double check with you that this change wouldn't break how
> > you're using it in systemd for /proc/sys/kernel/hostname:
> >
> > https://github.com/systemd/systemd/blob/39cd62c30c2e6bb5ec13ebc1ecf0d37ed015b1b8/src/journal/journald-server.c#L1832
> > https://github.com/systemd/systemd/blob/39cd62c30c2e6bb5ec13ebc1ecf0d37ed015b1b8/src/resolve/resolved-manager.c#L465
> >
> > I couldn't find anybody else actually polling on it. Interestingly, it
> > looks like sd_event_add_io uses epoll() inside, but you're not hitting
> > the bug that Jann pointed out (because I suppose you're not poll()ing on
> > an epoll fd).
>
> Well, if you made sure this still works, I am fine either way ;-)
Actually... ugh. It doesn't work. systemd uses uname() to read the host
name, and doesn't actually read() the file descriptor after receiving
the poll event on it. So I guess I'll forget this, and maybe we'll have
to live with sysctl's poll() being broken. :(
Jason
On Tue, May 03, 2022 at 01:27:44PM +0200, Jason A. Donenfeld wrote:
> On Mon, May 02, 2022 at 05:43:21PM +0200, Lennart Poettering wrote:
> > On Mo, 02.05.22 17:30, Jason A. Donenfeld ([email protected]) wrote:
> >
> > > Just wanted to double check with you that this change wouldn't break how
> > > you're using it in systemd for /proc/sys/kernel/hostname:
> > >
> > > https://github.com/systemd/systemd/blob/39cd62c30c2e6bb5ec13ebc1ecf0d37ed015b1b8/src/journal/journald-server.c#L1832
> > > https://github.com/systemd/systemd/blob/39cd62c30c2e6bb5ec13ebc1ecf0d37ed015b1b8/src/resolve/resolved-manager.c#L465
> > >
> > > I couldn't find anybody else actually polling on it. Interestingly, it
> > > looks like sd_event_add_io uses epoll() inside, but you're not hitting
> > > the bug that Jann pointed out (because I suppose you're not poll()ing on
> > > an epoll fd).
> >
> > Well, if you made sure this still works, I am fine either way ;-)
>
> Actually... ugh. It doesn't work. systemd uses uname() to read the host
> name, and doesn't actually read() the file descriptor after receiving
> the poll event on it. So I guess I'll forget this, and maybe we'll have
> to live with sysctl's poll() being broken. :(
A kconfig option may let you do what you want, and allow older kernels
to not break, however I am more curious how sysctl's approach to poll
went unnnoticed for so long. But also, I'm curious if it was based on
another poll implementation which may have been busted.
But more importantly, how do we avoid this in the future?
Luis
Luis Chamberlain <[email protected]> writes:
> On Tue, May 03, 2022 at 01:27:44PM +0200, Jason A. Donenfeld wrote:
>> On Mon, May 02, 2022 at 05:43:21PM +0200, Lennart Poettering wrote:
>> > On Mo, 02.05.22 17:30, Jason A. Donenfeld ([email protected]) wrote:
>> >
>> > > Just wanted to double check with you that this change wouldn't break how
>> > > you're using it in systemd for /proc/sys/kernel/hostname:
>> > >
>> > > https://github.com/systemd/systemd/blob/39cd62c30c2e6bb5ec13ebc1ecf0d37ed015b1b8/src/journal/journald-server.c#L1832
>> > > https://github.com/systemd/systemd/blob/39cd62c30c2e6bb5ec13ebc1ecf0d37ed015b1b8/src/resolve/resolved-manager.c#L465
>> > >
>> > > I couldn't find anybody else actually polling on it. Interestingly, it
>> > > looks like sd_event_add_io uses epoll() inside, but you're not hitting
>> > > the bug that Jann pointed out (because I suppose you're not poll()ing on
>> > > an epoll fd).
>> >
>> > Well, if you made sure this still works, I am fine either way ;-)
>>
>> Actually... ugh. It doesn't work. systemd uses uname() to read the host
>> name, and doesn't actually read() the file descriptor after receiving
>> the poll event on it. So I guess I'll forget this, and maybe we'll have
>> to live with sysctl's poll() being broken. :(
We should be able to modify calling uname() to act the same as reading
the file descriptor.
> A kconfig option may let you do what you want, and allow older kernels
> to not break, however I am more curious how sysctl's approach to poll
> went unnnoticed for so long. But also, I'm curious if it was based on
> another poll implementation which may have been busted.
>
> But more importantly, how do we avoid this in the future?
Poll on files is weird and generally doesn't work (because files are
always read to read or write). What did we do to make it work on these
sysctl files?
Eric