Re: [linus:master] [do_pollfd()] 8935989798: will-it-scale.per_process_ops 11.7% regression

[Al Viro <viro@zeniv.linux.org.uk>]

On Sun, Jan 26, 2025 at 04:16:04PM +0800, kernel test robot wrote:
> 
> 
> Hello,
> 
> kernel test robot noticed a 11.7% regression of will-it-scale.per_process_ops on:
> 
> 
> commit: 89359897983825dbfc08578e7ee807aaf24d9911 ("do_pollfd(): convert to CLASS(fd)")
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
> 
> [test faield on linus/master      b46c89c08f4146e7987fc355941a93b12e2c03ef]
> [test failed on linux-next/master 5ffa57f6eecefababb8cbe327222ef171943b183]
> 
> testcase: will-it-scale
> config: x86_64-rhel-9.4
> compiler: gcc-12
> test machine: 104 threads 2 sockets (Skylake) with 192G memory
> parameters:
> 
> 	nr_task: 100%
> 	mode: process
> 	test: poll2
> 	cpufreq_governor: performance
> 
> 
> 
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@intel.com>
> | Closes: https://lore.kernel.org/oe-lkp/202501261509.b6b4260d-lkp@intel.com
> 
> 
> Details are as below:
> -------------------------------------------------------------------------------------------------->
> 
> 
> The kernel config and materials to reproduce are available at:
> https://download.01.org/0day-ci/archive/20250126/202501261509.b6b4260d-lkp@intel.com

Very interesting...  Looking at the generated asm, two things seem to
change in there- "we need an fput()" case in (now implicit) fdput() in
do_pollfd() is no longer out of line and slightly different spills are
done in do_poll().

Just to make sure it's not a geniune change of logics somewhere,
could you compare d000e073ca2a, 893598979838 and d000e073ca2a with the
delta below?  That delta provably is an equivalent transformation - all
exits from do_pollfd() go through the return in the end, so that just
shifts the last assignment in there into the caller.

diff --git a/fs/select.c b/fs/select.c
index b41e2d651cc1..e0c816fa4ec4 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -875,8 +875,6 @@ static inline __poll_t do_pollfd(struct pollfd *pollfd, poll_table *pwait,
 	fdput(f);
 
 out:
-	/* ... and so does ->revents */
-	pollfd->revents = mangle_poll(mask);
 	return mask;
 }
 
@@ -909,6 +907,7 @@ static int do_poll(struct poll_list *list, struct poll_wqueues *wait,
 			pfd = walk->entries;
 			pfd_end = pfd + walk->len;
 			for (; pfd != pfd_end; pfd++) {
+				__poll_t mask;
 				/*
 				 * Fish for events. If we found one, record it
 				 * and kill poll_table->_qproc, so we don't
@@ -916,8 +915,9 @@ static int do_poll(struct poll_list *list, struct poll_wqueues *wait,
 				 * this. They'll get immediately deregistered
 				 * when we break out and return.
 				 */
-				if (do_pollfd(pfd, pt, &can_busy_loop,
-					      busy_flag)) {
+				mask = do_pollfd(pfd, pt, &can_busy_loop, busy_flag);
+				pfd->revents = mangle_poll(mask);
+				if (mask) {
 					count++;
 					pt->_qproc = NULL;
 					/* found something, stop busy polling */