RE: [x86] d55564cfc2: will-it-scale.per_thread_ops -5.8% regression

From: David Laight
Date: Fri Jan 08 2021 - 04:31:41 EST


From: Al Viro
> Sent: 07 January 2021 19:58
>
> On Thu, Jan 07, 2021 at 11:33:36AM -0800, Linus Torvalds wrote:
>
> > In fact, even some threaded app that does what I suspect it could do
> > would likely be ok with it 99% of the time. Because the situation
> > where you change the fd in the poll array is likely not the common
> > case, and even if some -1 file descriptor gets overwritten by a valid
> > one by the poll() system call again, it probably ends up being very
> > hard to see a failure.
> >
> > Which just makes me even more nervous.
>
> Hmm... But anything like that will have another problem - we do
> copyin only once. And we repeat fdget() on each iteration of
> do_poll() loop. Sure, we don't actually put anything on the
> queues after the first time around, and __pollwait() keeps the
> ones we are actually waiting for pinned, but... If another
> thread stores -1 to ->fd, then closes what used to be there
> and moves on, what will it see? ->poll() calls will be done
> for whatever file we'd reused the descriptor for. Sure,
> the kernel won't break, but the caller of poll() would need
> to be very careful about what it sees...
>
> Frankly, I'd consider seeing that kind of games in the userland
> as a big red flag; I'm not saying it's OK to break the suckers
> even worse than they are now, but I'm curious whether anything
> in the userland does it *and* how many bugs does it have around
> those uses of poll()...

It is much more likely that an application will change the 'events'
field - in particular enabling POLLOUT if a write() returned EAGAIN.

It could also change the fd, but defer doing the actual close() until
much later - that needs to be synchronised between the application
threads.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)