Messages in this thread Patch in this message | | | Date | Fri, 25 Nov 2005 15:28:29 -0800 | From | "Paul E. McKenney" <> | Subject | Re: BUG: spinlock recursion on 2.6.14-mm2 when oprofiling |
| |
On Fri, Nov 25, 2005 at 02:01:17PM -0800, Paul E. McKenney wrote: > One concern on the attached patch is the possible effect on latency. > > John, any reason why the dead_tasks list cannot be spliced onto a > local list, then freed outside of the task_mortuary lock? Any reason > why the dying_tasks list cannot be spliced onto the dead_tasks list > (an O(1) operation)?
And here is an alternative patch that assumes that the answer to both questions above is "no". It is shorter, though mostly due to use of the list_splice_init() and list_for_each_entry_safe() primitives.
Thanx, Paul
Signed-off-by: <paulmck@us.ibm.com>
buffer_sync.c | 28 +++++++++++++--------------- 1 files changed, 13 insertions(+), 15 deletions(-)
diff -urpNa -X dontdiff linux-2.6.14-mm2/drivers/oprofile/buffer_sync.c linux-2.6.14-mm2-fixmortuary/drivers/oprofile/buffer_sync.c --- linux-2.6.14-mm2/drivers/oprofile/buffer_sync.c 2005-10-27 17:02:08.000000000 -0700 +++ linux-2.6.14-mm2-fixmortuary/drivers/oprofile/buffer_sync.c 2005-11-25 14:51:26.000000000 -0800 @@ -46,10 +46,11 @@ static void process_task_mortuary(void); */ static int task_free_notify(struct notifier_block * self, unsigned long val, void * data) { + unsigned long flags; struct task_struct * task = data; - spin_lock(&task_mortuary); + spin_lock_irqsave(&task_mortuary, flags); list_add(&task->tasks, &dying_tasks); - spin_unlock(&task_mortuary); + spin_unlock_irqrestore(&task_mortuary, flags); return NOTIFY_OK; } @@ -431,25 +432,22 @@ static void increment_tail(struct oprofi */ static void process_task_mortuary(void) { - struct list_head * pos; - struct list_head * pos2; + unsigned long flags; + LIST_HEAD(local_dead_tasks); struct task_struct * task; + struct task_struct * ttask; - spin_lock(&task_mortuary); + spin_lock_irqsave(&task_mortuary, flags); - list_for_each_safe(pos, pos2, &dead_tasks) { - task = list_entry(pos, struct task_struct, tasks); - list_del(&task->tasks); - free_task(task); - } + list_splice_init(&dead_tasks, &local_dead_tasks); + list_splice_init(&dying_tasks, &dead_tasks); - list_for_each_safe(pos, pos2, &dying_tasks) { - task = list_entry(pos, struct task_struct, tasks); + spin_unlock_irqrestore(&task_mortuary, flags); + + list_for_each_entry_safe(task, ttask, &local_dead_tasks, tasks) { list_del(&task->tasks); - list_add_tail(&task->tasks, &dead_tasks); + free_task(task); } - - spin_unlock(&task_mortuary); } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |