Commit ef0ba055 authored by Linus Torvalds's avatar Linus Torvalds

poll: fix performance regression due to out-of-line __put_user()

The kernel test robot reported a -5.8% performance regression on the
"poll2" test of will-it-scale, and bisected it to commit d55564cf
("x86: Make __put_user() generate an out-of-line call").

I didn't expect an out-of-line __put_user() to matter, because no normal
core code should use that non-checking legacy version of user access any
more.  But I had overlooked the very odd poll() usage, which does a
__put_user() to update the 'revents' values of the poll array.

Now, Al Viro correctly points out that instead of updating just the
'revents' field, it would be much simpler to just copy the _whole_
pollfd entry, and then we could just use "copy_to_user()" on the whole
array of entries, the same way we use "copy_from_user()" a few lines
earlier to get the original values.

But that is not what we've traditionally done, and I worry that threaded
applications might be concurrently modifying the other fields of the
pollfd array.  So while Al's suggestion is simpler - and perhaps worth
trying in the future - this instead keeps the "just update revents"
model.

To fix the performance regression, use the modern "unsafe_put_user()"
instead of __put_user(), with the proper "user_write_access_begin()"
guarding in place. This improves code generation enormously.

Link: https://lore.kernel.org/lkml/20210107134723.GA28532@xsang-OptiPlex-9020/Reported-by: default avatarkernel test robot <oliver.sang@intel.com>
Tested-by: default avatarOliver Sang <oliver.sang@intel.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: David Laight <David.Laight@aculab.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent a91bd622
...@@ -1011,14 +1011,17 @@ static int do_sys_poll(struct pollfd __user *ufds, unsigned int nfds, ...@@ -1011,14 +1011,17 @@ static int do_sys_poll(struct pollfd __user *ufds, unsigned int nfds,
fdcount = do_poll(head, &table, end_time); fdcount = do_poll(head, &table, end_time);
poll_freewait(&table); poll_freewait(&table);
if (!user_write_access_begin(ufds, nfds * sizeof(*ufds)))
goto out_fds;
for (walk = head; walk; walk = walk->next) { for (walk = head; walk; walk = walk->next) {
struct pollfd *fds = walk->entries; struct pollfd *fds = walk->entries;
int j; int j;
for (j = 0; j < walk->len; j++, ufds++) for (j = walk->len; j; fds++, ufds++, j--)
if (__put_user(fds[j].revents, &ufds->revents)) unsafe_put_user(fds->revents, &ufds->revents, Efault);
goto out_fds;
} }
user_write_access_end();
err = fdcount; err = fdcount;
out_fds: out_fds:
...@@ -1030,6 +1033,11 @@ static int do_sys_poll(struct pollfd __user *ufds, unsigned int nfds, ...@@ -1030,6 +1033,11 @@ static int do_sys_poll(struct pollfd __user *ufds, unsigned int nfds,
} }
return err; return err;
Efault:
user_write_access_end();
err = -EFAULT;
goto out_fds;
} }
static long do_restart_poll(struct restart_block *restart_block) static long do_restart_poll(struct restart_block *restart_block)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment