Messages in this thread | | | From | Peter Oskolkov <> | Date | Tue, 23 Jun 2020 13:59:22 -0700 | Subject | Re: [RFC PATCH 1/3 v2] futex: introduce FUTEX_SWAP operation |
| |
On Tue, Jun 23, 2020 at 12:45 PM Andrei Vagin <avagin@gmail.com> wrote: > > On Tue, Jun 23, 2020 at 11:30:30AM -0700, Peter Oskolkov wrote: > ... > > > > /** > > > > +static int futex_swap(u32 __user *uaddr, unsigned int flags, u32 > > > > val, > > > > + ktime_t *abs_time, u32 __user *uaddr2) > > > > +{ > > > > + u32 bitset = FUTEX_BITSET_MATCH_ANY; > > > > + struct task_struct *next = NULL; > > > > + DEFINE_WAKE_Q(wake_q); > > > > + int ret; > > > > + > > > > + ret = prepare_wake_q(uaddr2, flags, 1, bitset, &wake_q); > > > > + if (!wake_q_empty(&wake_q)) { > > > > + /* Pull the first wakee out of the queue to swap into. > > > > */ > > > > + next = container_of(wake_q.first, struct task_struct, > > > > wake_q); > > > > + wake_q.first = wake_q.first->next; > > > > + next->wake_q.next = NULL; > > > > + /* > > > > + * Note that wake_up_q does not touch wake_q.last, so > > > > we > > > > + * do not bother with it here. > > > > + */ > > > > + wake_up_q(&wake_q); > > > > > > wake_up_q() doesn't seem to serve any purpose in that the above > > > assignment of wake_q.first shall make it an empty queue now? > > > Also, I don't see a need to touch wake_q.first either so I think we > > > can > > > get rid of wake_q altogether here. > > > > The futex at uaddr2 may have more than one waiter, so we cannot assume > > that wake_q will be empty when we remove the first element. > > The third argument of prepare_wake_q is nr_wake which is one in this > case, so we can be sure that wake_q will be empty, can't we?
Right, sorry. In an early draft it was possible to wake more than one waiter, and the code carried over from then/there in this form. I'll remove wake_up_q() if/when the API will be deemed acceptable by the maintainers.
> > > > > > > > > > + } > > > > + if (ret < 0) > > > > + return ret; > > > > + > > > > + return futex_wait(uaddr, flags, val, abs_time, bitset, next); > > > > +} > > >
| |