Messages in this thread | | | Date | Mon, 28 Aug 2017 11:29:59 +1000 | From | Nicholas Piggin <> | Subject | Re: [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
| |