Messages in this thread | | | Date | Thu, 7 Dec 2017 01:49:15 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH 1/2] epoll: use the waitqueue lock to protect ep->wq |
| |
* Christoph Hellwig <hch@lst.de> wrote:
> The eoll code currently always uses the unlocked waitqueue helpers for
s/eoll /epoll
> ep->wq, but instead of holding the lock inside the waitqueue around these > calls, as expected by the epoll code uses its own lock.
Hm, that reads a bit weirdly. How about:
The epoll code currently uses the unlocked waitqueue helpers for managing ep->wq, but instead of holding the waitqueue lock around these calls, it uses its own ep->lock spinlock.
> Given that the > waitqueue is not exposed to the rest of the kernel this actually works > ok at the moment, but prevents the epoll locking rules from being > enforced using lockdep. Remove ep->lock and use the waitqueue lock > to not only reduce the size of struct eventpoll but also make sure we > can assert locking invariations in the waitqueue code.
s/but also make sure but also to make sure
s/invariations /invariants
> Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/eventpoll.c | 46 ++++++++++++++++++++++------------------------ > 1 file changed, 22 insertions(+), 24 deletions(-) > > diff --git a/fs/eventpoll.c b/fs/eventpoll.c > index afd548ebc328..2b2c5ac80e26 100644 > --- a/fs/eventpoll.c > +++ b/fs/eventpoll.c > @@ -182,11 +182,10 @@ struct epitem { > * This structure is stored inside the "private_data" member of the file > * structure and represents the main data structure for the eventpoll > * interface. > + * > + * Access to it is protected by the lock inside wq. > */ > struct eventpoll { > - /* Protect the access to this structure */ > - spinlock_t lock; > - > /* > * This mutex is used to ensure that files are not removed > * while epoll is using them. This is held during the event > @@ -210,7 +209,7 @@ struct eventpoll { > /* > * This is a single linked list that chains all the "struct epitem" that > * happened while transferring ready events to userspace w/out > - * holding ->lock. > + * holding ->wq.lock. > */
Neat trick!
This exposes some waitqueue internals, but AFAICS the FUSE code already does a similar trick with fiq->waitq.lock so there's precedent.
Peter, what do you think?
Thanks,
Ingo
| |