Remove last BKL leftover: lock_depth
Subsystems (3)
I noticed the BKL task->lock_depth field was there with various users. But they all don't need it anymore because the BKL is gone. Fix up all the users and then finally remove the field.
This patchkit only removes code.
-Andi
PATCH 1/4 BKL: Remove BKL references in mutex code
From: Andi Kleen <[email protected]>
The mutex code still checks for the BKL which is already gone. Remove that code.
--- kernel/mutex.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
kernel/mutex.c +0 -7
@@ -163,13 +163,6 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,struct thread_info *owner;/* -* If we own the BKL, then don't spin. The owner of -* the mutex might be waiting on us to release the BKL. -*/ -if (unlikely (current->lock_depth >=0 )) -break ; - -/* * If there's an owner, wait for it to either * release the lock or go to sleep. */
Signature · 1 line
1.7.4.4
PATCH 2/4 SCHED: Remove BKL accounting from schedstats
In reply to Andi KleenFrom: Andi Kleen <[email protected]>
Remove the BKL accounting from schedstats.
I removed the field from the debug file too, in theory it could be kept as 0 for compatibility.
--- include/linux/sched.h | 4 ---- kernel/sched.c | 6 ------ kernel/sched_debug.c | 3 --- 3 files changed, 0 insertions(+), 13 deletions(-)
include/linux/sched.h +0 -4
@@ -731,10 +731,6 @@ struct sched_info {/* timestamps */ unsigned long long last_arrival,/* when we last ran on a cpu */ last_queued; /* when we were last queued to run */ -#ifdef CONFIG_SCHEDSTATS -/* BKL stats */ -unsigned int bkl_count; -#endif };#endif /* defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT) */
kernel/sched.c +0 -6
@@ -4025,12 +4025,6 @@ static inline void schedule_debug(struct task_struct *prev)profile_hit (SCHED_PROFILING ,__builtin_return_address (0 ));schedstat_inc (this_rq (), sched_count); -#ifdef CONFIG_SCHEDSTATS -if (unlikely (prev->lock_depth >=0 )) { -schedstat_inc (this_rq (), rq_sched_info.bkl_count); -schedstat_inc (prev, sched_info.bkl_count); - } -#endif }static void put_prev_task (struct rq *rq,struct task_struct *prev)
kernel/sched_debug.c +0 -3
@@ -296,9 +296,6 @@ static void print_cpu(struct seq_file *m, int cpu)P (ttwu_count);P (ttwu_local); -SEQ_printf (m," .%-30s: %d\n" ,"bkl_count" , - rq->rq_sched_info.bkl_count); -#undef P#undef P64#endif
Signature · 1 line
1.7.4.4
Re: [PATCH 2/4] SCHED: Remove BKL accounting from schedstats
Quoted 7 lines · Andi Kleen
From: Andi Kleen <[email protected]>
Remove the BKL accounting from schedstats.
I removed the field from the debug file too, in theory it could be kept as 0 for compatibility.
Quoted 6 lines
Signed-off-by Andi Kleen <[email protected]>--- include/linux/sched.h | 4 ---- kernel/sched.c | 6 ------ kernel/sched_debug.c | 3 --- 3 files changed, 0 insertions(+), 13 deletions(-)
The thing is, and this is rather amazing: not a *single* patch in your patch series gets the patch title right ...
Andi, how hard is it for you to type 'git log kernel/sched.c' (and 'git log kernel/mutex.c', etc.) and see what the commit title convention the affected kernel subsystem has, and follow that convention, so that the maintainer does not have to edit every single patch of your series?
You are not a kernel newbie, you've been a kernel developer for how many years?
You are showing an amazing level of inattention, which makes me very reluctant to even look at your patches, all of which touch tricky pieces of code.
You badly need to improve the quality of your patches. You could start that by feeding patches to a third party who knows how to submit patches upstream and who can sign off to them and forward them to the right maintainers.
Thanks,
Ingo
PATCH 3/4 SCHED: Don't use BKL count for idle preempt count initialization
In reply to Andi KleenFrom: Andi Kleen <[email protected]>
No need for that ever as far as I can see. Just set it to 0.
--- kernel/sched.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)
kernel/sched.c +1 -4
@@ -5776,11 +5776,8 @@ void __cpuinit init_idle(struct task_struct *idle, int cpu)raw_spin_unlock_irqrestore (&rq->lock, flags);/* Set the preempt count _outside_ the spinlocks! */ -#if defined(CONFIG_PREEMPT ) -task_thread_info (idle)->preempt_count = (idle->lock_depth >=0 ); -#else task_thread_info (idle)->preempt_count =0 ; -#endif +/* * The idle tasks have their own, simple scheduling class: */
Signature · 1 line
1.7.4.4
PATCH 4/4 Remove last BKL leftover: lock_depth
In reply to Andi KleenFrom: Andi Kleen <[email protected]>
This was the (hopefully) last BKL leftover: the per task lock_depth field.
After all users are gone remove the task_struct lock_depth field. Depending on the configuration this will not actually shrink task_struct unfortunately due to padding. It helps a bit on 32bit. And on 64bit it allows new bloat.
--- include/linux/init_task.h | 1 - include/linux/sched.h | 2 -- kernel/fork.c | 1 - 3 files changed, 0 insertions(+), 4 deletions(-)
include/linux/init_task.h +0 -1
@@ -134,7 +134,6 @@ extern struct cred init_cred; .stack = &init_thread_info, \ .usage = ATOMIC_INIT(2), \ .flags = PF_KTHREAD, \ - .lock_depth = -1, \ .prio = MAX_PRIO-20, \ .static_prio = MAX_PRIO-20, \ .normal_prio = MAX_PRIO-20, \
include/linux/sched.h +0 -2
@@ -1193,8 +1193,6 @@ struct task_struct {unsigned int flags;/* per process flags, defined below */ unsigned int ptrace; -int lock_depth;/* BKL lock depth */ -#ifdef CONFIG_SMP #ifdef __ARCH_WANT_UNLOCKED_CTXSWint oncpu;
kernel/fork.c +0 -1
@@ -1103,7 +1103,6 @@ static struct task_struct *copy_process(unsigned long clone_flags,posix_cpu_timers_init (p); - p->lock_depth =-1 ;/* -1 = no lock */ do_posix_clock_monotonic_gettime (&p->start_time); p->real_start_time = p->start_time;monotonic_to_bootbased (&p->real_start_time);
Signature · 1 line
1.7.4.4
Andrew Morton
maintainerQuoted 5 lines · On Fri, 6 May 2011 14:05:24 -0700 Andi Kleen <[email protected]> wrote:
I noticed the BKL
task->lock_depthfield was there with various users. But they all don't need it anymore because the BKL is gone. Fix up all the users and then finally remove the field.This patchkit only removes code.
Patches in Ingo's tree from Jon Corbet have done this removal.
Quoted 10 lines · Andrew Morton
Quoted 5 lines · On Fri, 6 May 2011 14:05:24 -0700 Andi Kleen <[email protected]> wrote:
I noticed the BKL
task->lock_depthfield was there with various users. But they all don't need it anymore because the BKL is gone. Fix up all the users and then finally remove the field.This patchkit only removes code.
Patches in Ingo's tree from Jon Corbet have done this removal.
Ok thanks. I was wondering why noone had done it.
-Andi
Signature · 1 line
[email protected] -- Speaking for myself only.