On Fri, Jun 29, 2018 at 6:29 AM Christoph Hellwig wrote:
> No need for poll_table_entry, we just need a wait_queue_head.
> poll_table_entry is an select.c internal (except for two nasty driver) -
> neither epoll nor most in-kernel callers use it.
Well, you need the poll_table for the "poll_wait()
On Thu, Jun 28, 2018 at 10:30:27PM +0100, Al Viro wrote:
> > Because I think that what it can do is simply to do the ->poll() calls
> > outside the iocb locks, and then just attach the poll table to the
> > kioctx afterwards.
>
> I'd do a bit more - embed the first poll_table_entry into poll iocb
On Thu, Jun 28, 2018 at 02:11:17PM -0700, Linus Torvalds wrote:
> Christoph, do you have a test program for IOCB_CMD_POLL and what it's
> actually supposed to do?
https://pagure.io/libaio/c/9c6935e81854d1585bbfa48c35b185849d746864?branch=aio-poll
is the actual test in libaio. In addition to that
On Thu, Jun 28, 2018 at 4:37 PM Al Viro wrote:
>
> You underestimate the nastiness of that thing (and for the record, I'm
> a lot *less* fond of AIO than you are, what with having had to read that
> nest of horrors lately). It does not "copy the data to userland"; what it
> does instead is copyin
On Thu, Jun 28, 2018 at 03:55:35PM -0700, Linus Torvalds wrote:
> > You are misreading that mess. What he's trying to do (other than surviving
> > the awful clusterfuck around cancels) is to handle the decision what to
> > report to userland right in the wakeup callback. *That* is what really
> >
On Thu, Jun 28, 2018 at 3:49 PM Al Viro wrote:
>
> aio_poll() is not a problem. It's wakeup callback that is one.
No, it's not a problem either.
The only thing the wakup callback wants to do is to remove the wait queue entry.
And *that* doesn't need to sleep, and it has absolutely nothing to d
On Thu, Jun 28, 2018 at 03:35:03PM -0700, Linus Torvalds wrote:
> Yes, the AIO poll implementation did it under the spinlock.
>
> But there's no good *reason* for that. The "aio_poll()" function
> itself is called in perfectly fine blocking context.
aio_poll() is not a problem. It's wakeup cal
On Thu, Jun 28, 2018 at 3:20 PM Al Viro wrote:
>
> The rules for drivers change only in one respect - if your ->poll() is going
> to
> need to block, check poll_requested_events(pt) & EPOLL_ATOMIC and return
> EPOLLNVAL
> in such case.
OI still don't even understand why you care.
Yes, the AIO
On Thu, Jun 28, 2018 at 10:30:27PM +0100, Al Viro wrote:
> I'm not saying that blocking on other things is a bug; some of such *are*
> bogus,
> but a lot aren't really broken. What I said is that in a lot of cases we
> really
> have hard "no blocking other than in callback" (and on subsequent p
On Thu, Jun 28, 2018 at 2:30 PM Al Viro wrote:
>
> > Again, locking is permitted. It's not great, but it's not against the rules.
>
> Me: a *LOT* of ->poll() instances only block in __pollwait() called
> (indirectly)
> on the first pass.
>
> You: They are *all* supposed to do it.
>
> Me:
Oh, I
On Thu, Jun 28, 2018 at 02:11:17PM -0700, Linus Torvalds wrote:
> On Thu, Jun 28, 2018 at 1:28 PM Al Viro wrote:
> >
> >
> > Sure, but...
> >
> > static __poll_t binder_poll(struct file *filp,
> > struct poll_table_struct *wait)
> > {
> > struct binder_proc
On Thu, Jun 28, 2018 at 1:37 PM Al Viro wrote:
>
>
> Speaking of obvious bogosities (a lot more so than a blocking allocation
> several calls down into helper):
>
> static __poll_t ca8210_test_int_poll(
> struct file *filp,
> struct poll_table_struct *ptable
> )
Ok, that's just ga
On Thu, Jun 28, 2018 at 1:28 PM Al Viro wrote:
>
>
> Sure, but...
>
> static __poll_t binder_poll(struct file *filp,
> struct poll_table_struct *wait)
> {
> struct binder_proc *proc = filp->private_data;
> struct binder_thread *thread = NULL;
>
On Thu, Jun 28, 2018 at 09:28:37PM +0100, Al Viro wrote:
> Sure. Unfortunately, adding yourself to the poll table is not the only
> way to block. And a plenty of instances in e.g. drivers/media (where
> the bulk of ->poll() instances lives) are very free with grabbing mutexes
> as they go.
Spea
On Thu, Jun 28, 2018 at 12:31:14PM -0700, Linus Torvalds wrote:
> > * a *LOT* of ->poll() instances only block in __pollwait()
> > called (indirectly) on the first pass.
>
> They are *all* supposed to do it.
Sure, but...
static __poll_t binder_poll(struct file *filp,
On Thu, Jun 28, 2018 at 11:17 AM Al Viro wrote:
>
> As for what can be salvaged out of the whole mess,
> * probably the most serious lesson is that INDIRECT CALLS ARE
> COSTLY NOW and shouldn't be used lightly.
Note that this has always been true, it's just _way_ more obvious now.
Indire
On Thu, Jun 28, 2018 at 09:40:21AM -0700, Linus Torvalds wrote:
> On Thu, Jun 28, 2018 at 7:21 AM Christoph Hellwig wrote:
> >
> > Note that for this removes the possibility of actually returning an
> > error before waiting in poll. We could still do this with an ERR_PTR
> > in f_poll_head with a
On Thu, Jun 28, 2018 at 7:21 AM Christoph Hellwig wrote:
>
> Note that for this removes the possibility of actually returning an
> error before waiting in poll. We could still do this with an ERR_PTR
> in f_poll_head with a little bit of WRITE_ONCE/READ_ONCE magic, but
> I'd like to defer that un
The doubling of the indirect calls caused a too big regression for some
benchmarks in our post-spectre world.
With some of the nastiness in the networking code moved out of the way
we can instead stick a pointer to a waitqueue into struct file and
avoid one of the indirect calls. This actually ha
19 matches
Mail list logo
