On Tue, Aug 06, 2019 at 02:26:38AM -0400, Gabriel Krisman Bertazi wrote: > Peter Zijlstra <pet...@infradead.org> writes: > > > > >> +static int futex_wait_multiple(u32 __user *uaddr, unsigned int flags, > >> + u32 count, ktime_t *abs_time) > >> +{ > >> + struct futex_wait_block *wb; > >> + struct restart_block *restart; > >> + int ret; > >> + > >> + if (!count) > >> + return -EINVAL; > >> + > >> + wb = kcalloc(count, sizeof(struct futex_wait_block), GFP_KERNEL); > >> + if (!wb) > >> + return -ENOMEM; > >> + > >> + if (copy_from_user(wb, uaddr, > >> + count * sizeof(struct futex_wait_block))) { > >> + ret = -EFAULT; > >> + goto out; > >> + } > > > > I'm thinking we can do away with this giant copy and do it one at a time > > from the other function, just extend the storage allocated there to > > store whatever values are still required later.
> I'm not sure I get the suggestion here. If I understand the code > correctly, once we do it one at a time, we need to queue_me() each futex > and then drop the hb lock, before going to the next one. So the idea is to reduce to a single allocation; like Thomas also said. And afaict, we've not yet taken any locks the first time we iterate -- that only does get_futex_key(), the whole __futex_wait_setup() and queue_me(), comes later, in the second iteration.