Messages in this thread | | | Date | Tue, 23 Jun 2020 12:45:29 -0700 | From | Andrei Vagin <> | Subject | Re: [RFC PATCH 1/3 v2] futex: introduce FUTEX_SWAP operation |
| |
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?
> > > > > > + } > > > + if (ret < 0) > > > + return ret; > > > + > > > + return futex_wait(uaddr, flags, val, abs_time, bitset, next); > > > +} > >
| |