diff mbox series

[next,1/2] sched: Init new task's vruntime after select cpu

Message ID 20221031125113.72980-2-zhangqiao22@huawei.com
State New
Headers
Series sched: sched_fork() optimizations |

Commit Message

Zhang Qiao Oct. 31, 2022, 12:51 p.m. UTC 
  When create a new task, we initialize vruntime of the new task
at sched_cgroup_fork(). However, this action is executed too
early and may be incorrect, because it use current cpu to
init the vruntime, but the new task actually runs on the
cpu assigned at wake_up_new_task().

So the patch call task_fork() after select fork cpu and use
the ready cpu(the child will run on it) init the new task.

Signed-off-by: Zhang Qiao <zhangqiao22@huawei.com>
---
 kernel/sched/core.c | 7 ++++++-
 kernel/sched/fair.c | 7 +------
 2 files changed, 7 insertions(+), 7 deletions(-)

Comments

kernel test robot Oct. 31, 2022, 2:23 p.m. UTC | #1 
Hi Zhang,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20221031]

url:    https://github.com/intel-lab-lkp/linux/commits/Zhang-Qiao/sched-sched_fork-optimizations/20221031-202427
patch link:    https://lore.kernel.org/r/20221031125113.72980-2-zhangqiao22%40huawei.com
patch subject: [PATCH next 1/2] sched: Init new task's vruntime after select cpu
config: i386-tinyconfig
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/dd85b689329a85b7c34067d160a67569c82327c6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Zhang-Qiao/sched-sched_fork-optimizations/20221031-202427
        git checkout dd85b689329a85b7c34067d160a67569c82327c6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash kernel/sched/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> kernel/sched/core.c:4654:6: warning: no previous prototype for 'sched_task_fork' [-Wmissing-prototypes]
    4654 | void sched_task_fork(struct task_struct *p)
         |      ^~~~~~~~~~~~~~~


vim +/sched_task_fork +4654 kernel/sched/core.c

  4653	
> 4654	void sched_task_fork(struct task_struct *p)
  4655	{
  4656		if (p->sched_class->task_fork)
  4657			p->sched_class->task_fork(p);
  4658	}
  4659
kernel test robot Nov. 1, 2022, 10:46 a.m. UTC | #2 
Hi Zhang,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20221031]

url:    https://github.com/intel-lab-lkp/linux/commits/Zhang-Qiao/sched-sched_fork-optimizations/20221031-202427
patch link:    https://lore.kernel.org/r/20221031125113.72980-2-zhangqiao22%40huawei.com
patch subject: [PATCH next 1/2] sched: Init new task's vruntime after select cpu
config: hexagon-randconfig-r006-20221101
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/dd85b689329a85b7c34067d160a67569c82327c6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Zhang-Qiao/sched-sched_fork-optimizations/20221031-202427
        git checkout dd85b689329a85b7c34067d160a67569c82327c6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash kernel/sched/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from kernel/sched/core.c:9:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from kernel/sched/core.c:9:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from kernel/sched/core.c:9:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
>> kernel/sched/core.c:4654:6: warning: no previous prototype for function 'sched_task_fork' [-Wmissing-prototypes]
   void sched_task_fork(struct task_struct *p)
        ^
   kernel/sched/core.c:4654:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void sched_task_fork(struct task_struct *p)
   ^
   static 
   kernel/sched/core.c:3579:20: warning: unused function 'rq_has_pinned_tasks' [-Wunused-function]
   static inline bool rq_has_pinned_tasks(struct rq *rq)
                      ^
   kernel/sched/core.c:5645:20: warning: unused function 'sched_tick_start' [-Wunused-function]
   static inline void sched_tick_start(int cpu) { }
                      ^
   kernel/sched/core.c:5646:20: warning: unused function 'sched_tick_stop' [-Wunused-function]
   static inline void sched_tick_stop(int cpu) { }
                      ^
   kernel/sched/core.c:6340:20: warning: unused function 'sched_core_cpu_starting' [-Wunused-function]
   static inline void sched_core_cpu_starting(unsigned int cpu) {}
                      ^
   kernel/sched/core.c:6341:20: warning: unused function 'sched_core_cpu_deactivate' [-Wunused-function]
   static inline void sched_core_cpu_deactivate(unsigned int cpu) {}
                      ^
   kernel/sched/core.c:6342:20: warning: unused function 'sched_core_cpu_dying' [-Wunused-function]
   static inline void sched_core_cpu_dying(unsigned int cpu) {}
                      ^
   13 warnings generated.


vim +/sched_task_fork +4654 kernel/sched/core.c

  4653	
> 4654	void sched_task_fork(struct task_struct *p)
  4655	{
  4656		if (p->sched_class->task_fork)
  4657			p->sched_class->task_fork(p);
  4658	}
  4659
diff mbox series

Patch

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index e4ce124ec701..ca5677206efd 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4627,9 +4627,13 @@  void sched_cgroup_fork(struct task_struct *p, struct kernel_clone_args *kargs)
 	 * so use __set_task_cpu().
 	 */
 	__set_task_cpu(p, smp_processor_id());
+	raw_spin_unlock_irqrestore(&p->pi_lock, flags);
+}
+
+void sched_task_fork(struct task_struct *p)
+{
 	if (p->sched_class->task_fork)
 		p->sched_class->task_fork(p);
-	raw_spin_unlock_irqrestore(&p->pi_lock, flags);
 }
 
 void sched_post_fork(struct task_struct *p)
@@ -4682,6 +4686,7 @@  void wake_up_new_task(struct task_struct *p)
 #endif
 	rq = __task_rq_lock(p, &rf);
 	update_rq_clock(rq);
+	sched_task_fork(p);
 	post_init_entity_util_avg(p);
 
 	activate_task(rq, p, ENQUEUE_NOCLOCK);
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index e4a0b8bd941c..34845d425180 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -11603,12 +11603,8 @@  static void task_fork_fair(struct task_struct *p)
 	struct cfs_rq *cfs_rq;
 	struct sched_entity *se = &p->se, *curr;
 	struct rq *rq = this_rq();
-	struct rq_flags rf;
 
-	rq_lock(rq, &rf);
-	update_rq_clock(rq);
-
-	cfs_rq = task_cfs_rq(current);
+	cfs_rq = task_cfs_rq(p);
 	curr = cfs_rq->curr;
 	if (curr) {
 		update_curr(cfs_rq);
@@ -11626,7 +11622,6 @@  static void task_fork_fair(struct task_struct *p)
 	}
 
 	se->vruntime -= cfs_rq->min_vruntime;
-	rq_unlock(rq, &rf);
 }
 
 /*