Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751792Ab3EEJSw (ORCPT ); Sun, 5 May 2013 05:18:52 -0400 Received: from mail-ee0-f54.google.com ([74.125.83.54]:47753 "EHLO mail-ee0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751597Ab3EEJSt (ORCPT ); Sun, 5 May 2013 05:18:49 -0400 Date: Sun, 5 May 2013 11:18:44 +0200 From: Ingo Molnar To: Pavel Machek Cc: Colin Cross , Andrew Morton , lkml , Trond Myklebust , Len Brown , "Rafael J. Wysocki" , Peter Zijlstra , Ingo Molnar , "J. Bruce Fields" , "David S. Miller" , Andrew Morton , Mandeep Singh Baines , Paul Walmsley , Al Viro , "Eric W. Biederman" , Oleg Nesterov , linux-nfs@vger.kernel.org, Linux PM list , netdev , Linus Torvalds , Tejun Heo , Ben Chan Subject: Re: [PATCH 2/2] lockdep: check that no locks held at freeze time Message-ID: <20130505091844.GC22239@gmail.com> References: <1367615050-3894-1-git-send-email-ccross@android.com> <1367615050-3894-2-git-send-email-ccross@android.com> <20130504130440.GC13770@amd.pavel.ucw.cz> <20130504225715.GB24276@amd.pavel.ucw.cz> <20130505000528.GA25454@amd.pavel.ucw.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130505000528.GA25454@amd.pavel.ucw.cz> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2382 Lines: 73 * Pavel Machek wrote: > Hi! > > > >> >> --- a/kernel/exit.c > > >> >> +++ b/kernel/exit.c > > >> >> @@ -835,7 +835,7 @@ void do_exit(long code) > > >> >> /* > > >> >> * Make sure we are holding no locks: > > >> >> */ > > >> >> - debug_check_no_locks_held(tsk); > > >> >> + debug_check_no_locks_held(); > > >> > > > >> > Is task guaranteed == current? > > >> > > >> Yes, the first line of do_exit is: > > >> struct task_struct *tsk = current; > > > > > > Aha, I understand it now. > > > > > > Accessing current is slower than local variable. So your "new" code > > > will work but will be slower. Please revert this part. > > > > Using current instead of passing in tsk was done at Andrew Morton's > > suggestion, and makes no difference from the freezer's perspective > > since it would have to use current to get the task to pass in, so I'm > > going to leave it as is. > > Well, current is: > > static inline struct thread_info *current_thread_info(void) > { > register unsigned long sp asm ("sp"); > return (struct thread_info *)(sp & ~(THREAD_SIZE - 1)); > } That's the old 32-bit x86 trick to compute 'current' from the kernel stack pointer. It can be done better - for example on platforms with optimized percpu variables (x86-64) it looks like this: static inline struct thread_info *current_thread_info(void) { struct thread_info *ti; ti = (void *)(this_cpu_read_stable(kernel_stack) + KERNEL_STACK_OFFSET - THREAD_SIZE); return ti; } Which turns the computation of 'current' into a single instruction. For example, to access current->pid [which fields is at offset 0x2a4], we get: 3ad0: 65 48 8b 04 25 00 00 mov %gs:0x0,%rax 3ad7: 00 00 3ad9: 8b 80 a4 02 00 00 mov 0x2a4(%rax),%eax I also agree with the removal of the 'tsk' parameter because the function itself internally assumes that tsk == current. [ We could perhaps rename the function to debug_check_no_locks_held_curr(), to make it clear it operates on the current task. ] Thanks, Ingo -- 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/