Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S941362AbcKOIhm (ORCPT ); Tue, 15 Nov 2016 03:37:42 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:35045 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935299AbcKOIhl (ORCPT ); Tue, 15 Nov 2016 03:37:41 -0500 Date: Tue, 15 Nov 2016 09:37:36 +0100 From: Ingo Molnar To: Greg KH Cc: Christoph Hellwig , Peter Zijlstra , keescook@chromium.org, will.deacon@arm.com, elena.reshetova@intel.com, arnd@arndb.de, tglx@linutronix.de, hpa@zytor.com, Linus Torvalds , dave@progbits.org, linux-kernel@vger.kernel.org Subject: [PATCH] printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' Message-ID: <20161115083736.GA15734@gmail.com> References: <20161114173946.501528675@infradead.org> <20161114174446.486581399@infradead.org> <20161114181655.GA21516@infradead.org> <20161115072855.GB28248@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161115072855.GB28248@kroah.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5489 Lines: 203 * Greg KH wrote: > On Mon, Nov 14, 2016 at 10:16:55AM -0800, Christoph Hellwig wrote: > > On Mon, Nov 14, 2016 at 06:39:48PM +0100, Peter Zijlstra wrote: > > > Since we need to change the implementation, stop exposing internals. > > > > > > Provide kref_read() to read the current reference count; typically > > > used for debug messages. > > > > Can we just provide a printk specifier for a kref value instead as > > that is the only valid use case for reading the value? > > Yeah, that would be great as no one should be doing anything > logic-related based on the kref value. Find below a patch that implements %pAk for 'struct kref' count printing and %pAr for atomic_t counter printing. This is against vanilla upstream. Thanks, Ingo ============================> Subject: printk, locking/atomics, kref: Introduce new %pAr and %pAk format string options for atomic_t and 'struct kref' From: Ingo Molnar Date: Tue Nov 15 08:53:14 CET 2016 A decade of kref internals exposed to driver writers has proven that exposing internals to them is a bad idea. Make the bad patterns a bit easier to detect and allow cleaner printouts by offering two new printk format string extensions: %pAr - print the atomic_t count in decimal %pAk - print the struct kref count in decimal Also add printf testcases: [ 0.353126] test_printf: all 268 tests passed Cc: Arnaldo Carvalho de Melo Cc: Greg Kroah-Hartman Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Signed-off-by: Ingo Molnar --- Documentation/printk-formats.txt | 10 +++++++++ lib/test_printf.c | 28 ++++++++++++++++++++++++++ lib/vsprintf.c | 42 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+) Index: tip/Documentation/printk-formats.txt =================================================================== --- tip.orig/Documentation/printk-formats.txt +++ tip/Documentation/printk-formats.txt @@ -316,6 +316,16 @@ Flags bitfields such as page flags, gfp_ Passed by reference. +atomic variables such atomic_t or struct kref: + + %pAr atomic_t count + %pAk struct kref count + + For printing the current count value of atomic variables. This is + preferred to accessing the counts directly. + + Passed by reference. + Network device features: %pNF 0x000000000000c000 Index: tip/lib/test_printf.c =================================================================== --- tip.orig/lib/test_printf.c +++ tip/lib/test_printf.c @@ -20,6 +20,8 @@ #include #include +#include + #define BUF_SIZE 256 #define PAD_SIZE 16 #define FILL_CHAR '$' @@ -462,6 +464,31 @@ flags(void) kfree(cmp_buffer); } +/* + * Testcases for %pAr (atomic_t) and %pAk (struct kref) count printing: + */ +static void __init test_atomics__atomic_t(void) +{ + atomic_t count = ATOMIC_INIT(1); + + test("1", "%pAr", &count); +} + +static void __init test_atomics__kref(void) +{ + struct kref kref; + + kref_init(&kref); + + test("1", "%pAk", &kref); +} + +static void __init test_atomics(void) +{ + test_atomics__atomic_t(); + test_atomics__kref(); +} + static void __init test_pointer(void) { @@ -481,6 +508,7 @@ test_pointer(void) bitmap(); netdev_features(); flags(); + test_atomics(); } static int __init Index: tip/lib/vsprintf.c =================================================================== --- tip.orig/lib/vsprintf.c +++ tip/lib/vsprintf.c @@ -38,6 +38,8 @@ #include "../mm/internal.h" /* For the trace_print_flags arrays */ +#include + #include /* for PAGE_SIZE */ #include /* for dereference_function_descriptor() */ #include /* cpu_to_le16 */ @@ -1470,6 +1472,40 @@ char *flags_string(char *buf, char *end, return format_flags(buf, end, flags, names); } +static noinline_for_stack +char *atomic_var(char *buf, char *end, void *atomic_ptr, const char *fmt) +{ + unsigned long num; + const struct printf_spec numspec = { + .flags = SPECIAL|SMALL, + .field_width = -1, + .precision = -1, + .base = 10, + }; + + switch (fmt[1]) { + case 'r': + { + atomic_t *count_p = (void *)atomic_ptr; + + num = atomic_read(count_p); + break; + } + case 'k': + { + struct kref *kref_p = (void *)atomic_ptr; + + num = atomic_read(&kref_p->refcount); + break; + } + default: + WARN_ONCE(1, "Unsupported atomics modifier: %c\n", fmt[1]); + return buf; + } + + return number(buf, end, num, numspec); +} + int kptr_restrict __read_mostly; /* @@ -1563,6 +1599,10 @@ int kptr_restrict __read_mostly; * p page flags (see struct page) given as pointer to unsigned long * g gfp flags (GFP_* and __GFP_*) given as pointer to gfp_t * v vma flags (VM_*) given as pointer to unsigned long + * - 'A' For the count of atomic variables to be printed. + * Supported flags given by option: + * r atomic_t ('r'aw count) + * k struct kref ('k'ref count) * * ** Please update also Documentation/printk-formats.txt when making changes ** * @@ -1718,6 +1758,8 @@ char *pointer(const char *fmt, char *buf case 'G': return flags_string(buf, end, ptr, fmt); + case 'A': + return atomic_var(buf, end, ptr, fmt); } spec.flags |= SMALL; if (spec.field_width == -1) {