lkml.org 
[lkml]   [2017]   [Aug]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2 v2] sched/wait: Introduce lock breaker in wake_up_page_bit
On Mon, 28 Aug 2017 11:16:48 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:

> On Sun, 27 Aug 2017 16:12:19 -0700
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
>

> > diff --git a/mm/filemap.c b/mm/filemap.c
> > index baba290c276b..0b41c8cbeabc 100644
> > --- a/mm/filemap.c
> > +++ b/mm/filemap.c
> > @@ -986,10 +986,6 @@ static inline int
> > wait_on_page_bit_common(wait_queue_head_t *q,
> >
> > if (likely(test_bit(bit_nr, &page->flags))) {
> > io_schedule();
> > - if (unlikely(signal_pending_state(state, current))) {
> > - ret = -EINTR;
> > - break;
> > - }
> > }
> >
> > if (lock) {
> > @@ -999,6 +995,11 @@ static inline int
> > wait_on_page_bit_common(wait_queue_head_t *q,
> > if (!test_bit(bit_nr, &page->flags))
> > break;
> > }
> > +
> > + if (unlikely(signal_pending_state(state, current))) {
> > + ret = -EINTR;
> > + break;
> > + }
> > }
> >
> > finish_wait(q, wait);
> >
> > but maybe I'm missing something.
> >
> > Nick, comments?
>
> No I don't think you're missing something. We surely could lose our only
> wakeup in this window. So an exclusive waiter has to always make sure
> they propagate the wakeup (regardless of what they do with the contended
> resources itself).
>
> Seems like your fix should solve it. By the look of how wait_on_bit_lock
> is structured, the author probably did think about this case a little
> better than I did :\

BTW. since you are looking at this stuff, one other small problem I remember
with exclusive waiters is that losing to a concurrent locker puts them to
the back of the queue. I think that could be fixed with some small change to
the wait loops (first add to tail, then retries add to head). Thoughts?

Thanks,
Nick

\
 
 \ /
  Last update: 2017-08-28 03:30    [W:0.263 / U:0.984 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site