From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 08686C433E0 for ; Thu, 25 Feb 2021 22:01:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BD73E614A7 for ; Thu, 25 Feb 2021 22:01:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232770AbhBYWAf (ORCPT ); Thu, 25 Feb 2021 17:00:35 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37494 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230174AbhBYWAB (ORCPT ); Thu, 25 Feb 2021 17:00:01 -0500 Received: from mail-pg1-x536.google.com (mail-pg1-x536.google.com [IPv6:2607:f8b0:4864:20::536]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A578FC061756 for ; Thu, 25 Feb 2021 13:59:21 -0800 (PST) Received: by mail-pg1-x536.google.com with SMTP id t26so4752356pgv.3 for ; Thu, 25 Feb 2021 13:59:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Lr+EAj1L3SvhS0sgLJSE5BurhCAU0L6vbZIwL4lD+n0=; b=Z+d9Z01pJ/XoTC1j3PIYh8Ln2Le+IRissEssJVg6YNao725xaEF8bjpXlUlTDafXw3 +X4+UGpVNnopdZH4n08e6Z42aULD6jxU7PhmbZXrGT0TvxJjtU2/1TJ8w+t26hmbItnN JuimIMyxjua8zK/cjQEM3tlvZOF35MWYInuI8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Lr+EAj1L3SvhS0sgLJSE5BurhCAU0L6vbZIwL4lD+n0=; b=Q7k9syJGnr1qvA69NtMSONXb7bTcy0n3k8d/VmaiAVxPZ9PnuT5aqvVVAVQvQQKzbI LUhe5q/pI4WF+MCh14E3bbSPJANGntDXX+00oUdzYKugCRN3BLUiyYZRfpz93VeiWP8G xp1544zX8EFYm38Xe2+UKlyReROEh0Zt3txfouBm/k5FxSGLechXITxX4Ye3GoRm98lg Pk8xmrWpSV340Z2j3Y8+Mz4sRalPkPKVgpAFGsdK1qBqnnjqcnVHF9o5UJxik1RN6YQ8 HY0WDAAh5zRMfErshzYcutpeLrNKjEE1RDE/GUeCBxGGpYeutXsqmo6E9D4AswKW1V5m ixIw== X-Gm-Message-State: AOAM533IVxQny/L1EOgRabqVFNOtdanfEEB06AnVwd3NqGiRu1HhNneE kPXG3PDyz07f1BZowAf3+tDYxQ== X-Google-Smtp-Source: ABdhPJwl2Ti+XW6IUIm1JffdsYaS5HUTaKwMNdPcSB7kzRn/dqv2nPmmkKujDxOZzCXsD3O1WddSZQ== X-Received: by 2002:a63:a401:: with SMTP id c1mr13477pgf.60.1614290360980; Thu, 25 Feb 2021 13:59:20 -0800 (PST) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id i8sm7302055pgn.94.2021.02.25.13.59.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Feb 2021 13:59:20 -0800 (PST) Date: Thu, 25 Feb 2021 13:59:19 -0800 From: Kees Cook To: John Ogness Cc: Petr Mladek , Sergey Senozhatsky , Sergey Senozhatsky , Steven Rostedt , Thomas Gleixner , linux-kernel@vger.kernel.org, Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , Jeff Dike , Richard Weinberger , Anton Ivanov , "K. Y. Srinivasan" , Haiyang Zhang , Stephen Hemminger , Wei Liu , Miquel Raynal , Vignesh Raghavendra , Anton Vorontsov , Colin Cross , Tony Luck , Jason Wessel , Daniel Thompson , Douglas Anderson , Pavel Tatashin , Joel Stanley , Christophe Leroy , Jordan Niethe , Alistair Popple , Ravi Bangoria , Nicholas Piggin , Mike Rapoport , Madhavan Srinivasan , Thomas Meyer , Andy Shevchenko , Davidlohr Bueso , Oleg Nesterov , Wei Li , Michael Kelley , linuxppc-dev@lists.ozlabs.org, linux-um@lists.infradead.org, linux-hyperv@vger.kernel.org, linux-mtd@lists.infradead.org, kgdb-bugreport@lists.sourceforge.net Subject: Re: [PATCH next v3 12/15] printk: introduce a kmsg_dump iterator Message-ID: <202102251358.60700B3FFA@keescook> References: <20210225202438.28985-1-john.ogness@linutronix.de> <20210225202438.28985-13-john.ogness@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210225202438.28985-13-john.ogness@linutronix.de> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 25, 2021 at 09:24:35PM +0100, John Ogness wrote: > Rather than storing the iterator information in the registered > kmsg_dumper structure, create a separate iterator structure. The > kmsg_dump_iter structure can reside on the stack of the caller, thus > allowing lockless use of the kmsg_dump functions. > > This change also means that the kmsg_dumper dump() callback no > longer needs to pass in the kmsg_dumper as an argument. If > kmsg_dumpers want to access the kernel logs, they can use the new > iterator. > > Update the kmsg_dumper callback prototype. Update code that accesses > the kernel logs using the kmsg_dumper structure to use the new > kmsg_dump_iter structure. For kmsg_dumpers, this also means adding a > call to kmsg_dump_rewind() to initialize the iterator. > > All this is in preparation for removal of @logbuf_lock. > > Signed-off-by: John Ogness > --- > arch/powerpc/kernel/nvram_64.c | 14 +++--- > arch/powerpc/platforms/powernv/opal-kmsg.c | 3 +- > arch/powerpc/xmon/xmon.c | 6 +-- > arch/um/kernel/kmsg_dump.c | 8 +-- > drivers/hv/vmbus_drv.c | 7 +-- > drivers/mtd/mtdoops.c | 8 +-- > fs/pstore/platform.c | 8 +-- Reviewed-by: Kees Cook # pstore -Kees > include/linux/kmsg_dump.h | 38 ++++++++------- > kernel/debug/kdb/kdb_main.c | 10 ++-- > kernel/printk/printk.c | 57 ++++++++++------------ > 10 files changed, 81 insertions(+), 78 deletions(-) > > diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c > index 532f22637783..5a64b24a91c2 100644 > --- a/arch/powerpc/kernel/nvram_64.c > +++ b/arch/powerpc/kernel/nvram_64.c > @@ -72,8 +72,7 @@ static const char *nvram_os_partitions[] = { > NULL > }; > > -static void oops_to_nvram(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason); > +static void oops_to_nvram(enum kmsg_dump_reason reason); > > static struct kmsg_dumper nvram_kmsg_dumper = { > .dump = oops_to_nvram > @@ -642,11 +641,11 @@ void __init nvram_init_oops_partition(int rtas_partition_exists) > * that we think will compress sufficiently to fit in the lnx,oops-log > * partition. If that's too much, go back and capture uncompressed text. > */ > -static void oops_to_nvram(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void oops_to_nvram(enum kmsg_dump_reason reason) > { > struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf; > static unsigned int oops_count = 0; > + static struct kmsg_dump_iter iter; > static bool panicking = false; > static DEFINE_SPINLOCK(lock); > unsigned long flags; > @@ -681,13 +680,14 @@ static void oops_to_nvram(struct kmsg_dumper *dumper, > return; > > if (big_oops_buf) { > - kmsg_dump_get_buffer(dumper, false, > + kmsg_dump_rewind(&iter); > + kmsg_dump_get_buffer(&iter, false, > big_oops_buf, big_oops_buf_sz, &text_len); > rc = zip_oops(text_len); > } > if (rc != 0) { > - kmsg_dump_rewind(dumper); > - kmsg_dump_get_buffer(dumper, false, > + kmsg_dump_rewind(&iter); > + kmsg_dump_get_buffer(&iter, false, > oops_data, oops_data_sz, &text_len); > err_type = ERR_TYPE_KERNEL_PANIC; > oops_hdr->version = cpu_to_be16(OOPS_HDR_VERSION); > diff --git a/arch/powerpc/platforms/powernv/opal-kmsg.c b/arch/powerpc/platforms/powernv/opal-kmsg.c > index 6c3bc4b4da98..a7bd6ac681f4 100644 > --- a/arch/powerpc/platforms/powernv/opal-kmsg.c > +++ b/arch/powerpc/platforms/powernv/opal-kmsg.c > @@ -19,8 +19,7 @@ > * may not be completely printed. This function does not actually dump the > * message, it just ensures that OPAL completely flushes the console buffer. > */ > -static void kmsg_dump_opal_console_flush(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void kmsg_dump_opal_console_flush(enum kmsg_dump_reason reason) > { > /* > * Outside of a panic context the pollers will continue to run, > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c > index 80ed3e1becf9..5978b90a885f 100644 > --- a/arch/powerpc/xmon/xmon.c > +++ b/arch/powerpc/xmon/xmon.c > @@ -3001,7 +3001,7 @@ print_address(unsigned long addr) > static void > dump_log_buf(void) > { > - struct kmsg_dumper dumper; > + struct kmsg_dump_iter iter; > unsigned char buf[128]; > size_t len; > > @@ -3013,9 +3013,9 @@ dump_log_buf(void) > catch_memory_errors = 1; > sync(); > > - kmsg_dump_rewind_nolock(&dumper); > + kmsg_dump_rewind_nolock(&iter); > xmon_start_pagination(); > - while (kmsg_dump_get_line_nolock(&dumper, false, buf, sizeof(buf), &len)) { > + while (kmsg_dump_get_line_nolock(&iter, false, buf, sizeof(buf), &len)) { > buf[len] = '\0'; > printf("%s", buf); > } > diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c > index 4869e2cc787c..9fbc5e5b1023 100644 > --- a/arch/um/kernel/kmsg_dump.c > +++ b/arch/um/kernel/kmsg_dump.c > @@ -7,9 +7,9 @@ > #include > #include > > -static void kmsg_dumper_stdout(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void kmsg_dumper_stdout(enum kmsg_dump_reason reason) > { > + static struct kmsg_dump_iter iter; > static DEFINE_SPINLOCK(lock); > static char line[1024]; > struct console *con; > @@ -34,8 +34,10 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper, > if (!spin_trylock(&lock)) > return; > > + kmsg_dump_rewind(&iter); > + > printf("kmsg_dump:\n"); > - while (kmsg_dump_get_line(dumper, true, line, sizeof(line), &len)) { > + while (kmsg_dump_get_line(&iter, true, line, sizeof(line), &len)) { > line[len] = '\0'; > printf("%s", line); > } > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index 10dce9f91216..1b858f280e22 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -1388,9 +1388,9 @@ static void vmbus_isr(void) > * Callback from kmsg_dump. Grab as much as possible from the end of the kmsg > * buffer and call into Hyper-V to transfer the data. > */ > -static void hv_kmsg_dump(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void hv_kmsg_dump(enum kmsg_dump_reason reason) > { > + struct kmsg_dump_iter iter; > size_t bytes_written; > phys_addr_t panic_pa; > > @@ -1404,7 +1404,8 @@ static void hv_kmsg_dump(struct kmsg_dumper *dumper, > * Write dump contents to the page. No need to synchronize; panic should > * be single-threaded. > */ > - kmsg_dump_get_buffer(dumper, false, hv_panic_page, HV_HYP_PAGE_SIZE, > + kmsg_dump_rewind(&iter); > + kmsg_dump_get_buffer(&iter, false, hv_panic_page, HV_HYP_PAGE_SIZE, > &bytes_written); > if (bytes_written) > hyperv_report_panic_msg(panic_pa, bytes_written); > diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c > index 8bbfba40a554..d179b726a1c9 100644 > --- a/drivers/mtd/mtdoops.c > +++ b/drivers/mtd/mtdoops.c > @@ -272,19 +272,21 @@ static void find_next_position(struct mtdoops_context *cxt) > mtdoops_inc_counter(cxt); > } > > -static void mtdoops_do_dump(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void mtdoops_do_dump(enum kmsg_dump_reason reason) > { > struct mtdoops_context *cxt = container_of(dumper, > struct mtdoops_context, dump); > + struct kmsg_dump_iter iter; > > /* Only dump oopses if dump_oops is set */ > if (reason == KMSG_DUMP_OOPS && !dump_oops) > return; > > + kmsg_dump_rewind(&iter); > + > if (test_and_set_bit(0, &cxt->oops_buf_busy)) > return; > - kmsg_dump_get_buffer(dumper, true, cxt->oops_buf + MTDOOPS_HEADER_SIZE, > + kmsg_dump_get_buffer(&iter, true, cxt->oops_buf + MTDOOPS_HEADER_SIZE, > record_size - MTDOOPS_HEADER_SIZE, NULL); > clear_bit(0, &cxt->oops_buf_busy); > > diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c > index d963ae7902f9..edfc9504e024 100644 > --- a/fs/pstore/platform.c > +++ b/fs/pstore/platform.c > @@ -382,9 +382,9 @@ void pstore_record_init(struct pstore_record *record, > * callback from kmsg_dump. Save as much as we can (up to kmsg_bytes) from the > * end of the buffer. > */ > -static void pstore_dump(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void pstore_dump(enum kmsg_dump_reason reason) > { > + struct kmsg_dump_iter iter; > unsigned long total = 0; > const char *why; > unsigned int part = 1; > @@ -405,6 +405,8 @@ static void pstore_dump(struct kmsg_dumper *dumper, > } > } > > + kmsg_dump_rewind(&iter); > + > oopscount++; > while (total < kmsg_bytes) { > char *dst; > @@ -435,7 +437,7 @@ static void pstore_dump(struct kmsg_dumper *dumper, > dst_size -= header_size; > > /* Write dump contents. */ > - if (!kmsg_dump_get_buffer(dumper, true, dst + header_size, > + if (!kmsg_dump_get_buffer(&iter, true, dst + header_size, > dst_size, &dump_size)) > break; > > diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h > index 84eaa2090efa..5d3bf20f9f0a 100644 > --- a/include/linux/kmsg_dump.h > +++ b/include/linux/kmsg_dump.h > @@ -29,6 +29,16 @@ enum kmsg_dump_reason { > KMSG_DUMP_MAX > }; > > +/** > + * struct kmsg_dump_iter - iterator for retrieving kernel messages > + * @cur_seq: Points to the oldest message to dump > + * @next_seq: Points after the newest message to dump > + */ > +struct kmsg_dump_iter { > + u64 cur_seq; > + u64 next_seq; > +}; > + > /** > * struct kmsg_dumper - kernel crash message dumper structure > * @list: Entry in the dumper list (private) > @@ -36,35 +46,29 @@ enum kmsg_dump_reason { > * through the record iterator > * @max_reason: filter for highest reason number that should be dumped > * @registered: Flag that specifies if this is already registered > - * @cur_seq: Points to the oldest message to dump > - * @next_seq: Points after the newest message to dump > */ > struct kmsg_dumper { > struct list_head list; > - void (*dump)(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason); > + void (*dump)(enum kmsg_dump_reason reason); > enum kmsg_dump_reason max_reason; > bool registered; > - > - /* private state of the kmsg iterator */ > - u64 cur_seq; > - u64 next_seq; > }; > > #ifdef CONFIG_PRINTK > void kmsg_dump(enum kmsg_dump_reason reason); > > -bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog, > char *line, size_t size, size_t *len); > > -bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog, > char *line, size_t size, size_t *len); > > -bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog, > char *buf, size_t size, size_t *len_out); > > -void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper); > +void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter); > > -void kmsg_dump_rewind(struct kmsg_dumper *dumper); > +void kmsg_dump_rewind(struct kmsg_dump_iter *iter); > > int kmsg_dump_register(struct kmsg_dumper *dumper); > > @@ -76,30 +80,30 @@ static inline void kmsg_dump(enum kmsg_dump_reason reason) > { > } > > -static inline bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, > +static inline bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, > bool syslog, const char *line, > size_t size, size_t *len) > { > return false; > } > > -static inline bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog, > +static inline bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog, > const char *line, size_t size, size_t *len) > { > return false; > } > > -static inline bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > +static inline bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog, > char *buf, size_t size, size_t *len) > { > return false; > } > > -static inline void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper) > +static inline void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter) > { > } > > -static inline void kmsg_dump_rewind(struct kmsg_dumper *dumper) > +static inline void kmsg_dump_rewind(struct kmsg_dump_iter *iter) > { > } > > diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c > index 315169d5e119..8544d7a55a57 100644 > --- a/kernel/debug/kdb/kdb_main.c > +++ b/kernel/debug/kdb/kdb_main.c > @@ -2101,7 +2101,7 @@ static int kdb_dmesg(int argc, const char **argv) > int adjust = 0; > int n = 0; > int skip = 0; > - struct kmsg_dumper dumper; > + struct kmsg_dump_iter iter; > size_t len; > char buf[201]; > > @@ -2126,8 +2126,8 @@ static int kdb_dmesg(int argc, const char **argv) > kdb_set(2, setargs); > } > > - kmsg_dump_rewind_nolock(&dumper); > - while (kmsg_dump_get_line_nolock(&dumper, 1, NULL, 0, NULL)) > + kmsg_dump_rewind_nolock(&iter); > + while (kmsg_dump_get_line_nolock(&iter, 1, NULL, 0, NULL)) > n++; > > if (lines < 0) { > @@ -2159,8 +2159,8 @@ static int kdb_dmesg(int argc, const char **argv) > if (skip >= n || skip < 0) > return 0; > > - kmsg_dump_rewind_nolock(&dumper); > - while (kmsg_dump_get_line_nolock(&dumper, 1, buf, sizeof(buf), &len)) { > + kmsg_dump_rewind_nolock(&iter); > + while (kmsg_dump_get_line_nolock(&iter, 1, buf, sizeof(buf), &len)) { > if (skip) { > skip--; > continue; > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index 45cb3e9c62c5..e58ccc368348 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -3390,7 +3390,6 @@ EXPORT_SYMBOL_GPL(kmsg_dump_reason_str); > void kmsg_dump(enum kmsg_dump_reason reason) > { > struct kmsg_dumper *dumper; > - unsigned long flags; > > rcu_read_lock(); > list_for_each_entry_rcu(dumper, &dump_list, list) { > @@ -3407,21 +3406,15 @@ void kmsg_dump(enum kmsg_dump_reason reason) > if (reason > max_reason) > continue; > > - /* initialize iterator with data about the stored records */ > - logbuf_lock_irqsave(flags); > - dumper->cur_seq = latched_seq_read_nolock(&clear_seq); > - dumper->next_seq = prb_next_seq(prb); > - logbuf_unlock_irqrestore(flags); > - > /* invoke dumper which will iterate over records */ > - dumper->dump(dumper, reason); > + dumper->dump(reason); > } > rcu_read_unlock(); > } > > /** > * kmsg_dump_get_line_nolock - retrieve one kmsg log line (unlocked version) > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * @syslog: include the "<4>" prefixes > * @line: buffer to copy the line to > * @size: maximum size of the buffer > @@ -3438,7 +3431,7 @@ void kmsg_dump(enum kmsg_dump_reason reason) > * > * The function is similar to kmsg_dump_get_line(), but grabs no locks. > */ > -bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog, > char *line, size_t size, size_t *len) > { > struct printk_info info; > @@ -3451,11 +3444,11 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > > /* Read text or count text lines? */ > if (line) { > - if (!prb_read_valid(prb, dumper->cur_seq, &r)) > + if (!prb_read_valid(prb, iter->cur_seq, &r)) > goto out; > l = record_print_text(&r, syslog, printk_time); > } else { > - if (!prb_read_valid_info(prb, dumper->cur_seq, > + if (!prb_read_valid_info(prb, iter->cur_seq, > &info, &line_count)) { > goto out; > } > @@ -3464,7 +3457,7 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > > } > > - dumper->cur_seq = r.info->seq + 1; > + iter->cur_seq = r.info->seq + 1; > ret = true; > out: > if (len) > @@ -3474,7 +3467,7 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > > /** > * kmsg_dump_get_line - retrieve one kmsg log line > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * @syslog: include the "<4>" prefixes > * @line: buffer to copy the line to > * @size: maximum size of the buffer > @@ -3489,14 +3482,14 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > * A return value of FALSE indicates that there are no more records to > * read. > */ > -bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog, > char *line, size_t size, size_t *len) > { > unsigned long flags; > bool ret; > > logbuf_lock_irqsave(flags); > - ret = kmsg_dump_get_line_nolock(dumper, syslog, line, size, len); > + ret = kmsg_dump_get_line_nolock(iter, syslog, line, size, len); > logbuf_unlock_irqrestore(flags); > > return ret; > @@ -3505,7 +3498,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_line); > > /** > * kmsg_dump_get_buffer - copy kmsg log lines > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * @syslog: include the "<4>" prefixes > * @buf: buffer to copy the line to > * @size: maximum size of the buffer > @@ -3522,7 +3515,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_line); > * A return value of FALSE indicates that there are no more records to > * read. > */ > -bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog, > char *buf, size_t size, size_t *len_out) > { > struct printk_info info; > @@ -3538,15 +3531,15 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > goto out; > > logbuf_lock_irqsave(flags); > - if (prb_read_valid_info(prb, dumper->cur_seq, &info, NULL)) { > - if (info.seq != dumper->cur_seq) { > + if (prb_read_valid_info(prb, iter->cur_seq, &info, NULL)) { > + if (info.seq != iter->cur_seq) { > /* messages are gone, move to first available one */ > - dumper->cur_seq = info.seq; > + iter->cur_seq = info.seq; > } > } > > /* last entry */ > - if (dumper->cur_seq >= dumper->next_seq) { > + if (iter->cur_seq >= iter->next_seq) { > logbuf_unlock_irqrestore(flags); > goto out; > } > @@ -3557,7 +3550,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > * because this function (by way of record_print_text()) will > * not write more than size-1 bytes of text into @buf. > */ > - seq = find_first_fitting_seq(dumper->cur_seq, dumper->next_seq, > + seq = find_first_fitting_seq(iter->cur_seq, iter->next_seq, > size - 1, syslog, time); > > /* > @@ -3570,7 +3563,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > > len = 0; > prb_for_each_record(seq, prb, seq, &r) { > - if (r.info->seq >= dumper->next_seq) > + if (r.info->seq >= iter->next_seq) > break; > > len += record_print_text(&r, syslog, time); > @@ -3579,7 +3572,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > prb_rec_init_rd(&r, &info, buf + len, size - len); > } > > - dumper->next_seq = next_seq; > + iter->next_seq = next_seq; > ret = true; > logbuf_unlock_irqrestore(flags); > out: > @@ -3591,7 +3584,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer); > > /** > * kmsg_dump_rewind_nolock - reset the iterator (unlocked version) > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * > * Reset the dumper's iterator so that kmsg_dump_get_line() and > * kmsg_dump_get_buffer() can be called again and used multiple > @@ -3599,26 +3592,26 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer); > * > * The function is similar to kmsg_dump_rewind(), but grabs no locks. > */ > -void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper) > +void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter) > { > - dumper->cur_seq = latched_seq_read_nolock(&clear_seq); > - dumper->next_seq = prb_next_seq(prb); > + iter->cur_seq = latched_seq_read_nolock(&clear_seq); > + iter->next_seq = prb_next_seq(prb); > } > > /** > * kmsg_dump_rewind - reset the iterator > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * > * Reset the dumper's iterator so that kmsg_dump_get_line() and > * kmsg_dump_get_buffer() can be called again and used multiple > * times within the same dumper.dump() callback. > */ > -void kmsg_dump_rewind(struct kmsg_dumper *dumper) > +void kmsg_dump_rewind(struct kmsg_dump_iter *iter) > { > unsigned long flags; > > logbuf_lock_irqsave(flags); > - kmsg_dump_rewind_nolock(dumper); > + kmsg_dump_rewind_nolock(iter); > logbuf_unlock_irqrestore(flags); > } > EXPORT_SYMBOL_GPL(kmsg_dump_rewind); > -- > 2.20.1 > -- Kees Cook From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3CE9BC433DB for ; Thu, 25 Feb 2021 21:59:54 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9D01E614A7 for ; Thu, 25 Feb 2021 21:59:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9D01E614A7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4Dmmt00xGYz3cxT for ; Fri, 26 Feb 2021 08:59:52 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.a=rsa-sha256 header.s=google header.b=Z+d9Z01p; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=chromium.org (client-ip=2607:f8b0:4864:20::436; helo=mail-pf1-x436.google.com; envelope-from=keescook@chromium.org; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.a=rsa-sha256 header.s=google header.b=Z+d9Z01p; dkim-atps=neutral Received: from mail-pf1-x436.google.com (mail-pf1-x436.google.com [IPv6:2607:f8b0:4864:20::436]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4DmmsS6sZsz30Jn for ; Fri, 26 Feb 2021 08:59:23 +1100 (AEDT) Received: by mail-pf1-x436.google.com with SMTP id q204so3485361pfq.10 for ; Thu, 25 Feb 2021 13:59:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Lr+EAj1L3SvhS0sgLJSE5BurhCAU0L6vbZIwL4lD+n0=; b=Z+d9Z01pJ/XoTC1j3PIYh8Ln2Le+IRissEssJVg6YNao725xaEF8bjpXlUlTDafXw3 +X4+UGpVNnopdZH4n08e6Z42aULD6jxU7PhmbZXrGT0TvxJjtU2/1TJ8w+t26hmbItnN JuimIMyxjua8zK/cjQEM3tlvZOF35MWYInuI8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Lr+EAj1L3SvhS0sgLJSE5BurhCAU0L6vbZIwL4lD+n0=; b=Hha3bQ97Z2+NufSfyx2otC3e8xGARFdHMrcT6me3shqmHZHaxsHapvuNmT/MKZdJVc Yl7lh6CDjQBdQgZPUjK6wrey5YVZAye1fyz5MyHadrZ/wNwL/zVoI4zvzrIqk+LHTvJS XDUatM7Av6koh55DmGKUzbk71i0en52tBH21HMt0b6BMKtVeFiIt/C8zSuNCRzSHOEF2 3b3mTbXfOrLrLww/NpaUcACaedQCQSxHNResCHvipBsQ/3sIKcJRf5YXOqLutGd8H9/D oGPpM5m5e/+oOt/nnN2pLUN1dFLegSjLGdCnX+ag4Sp3f9QNNODVExZ+xRJjBcSf2oy7 MBoA== X-Gm-Message-State: AOAM531YTIyNuD30pp75Qs33uuLsx4Po3QqtEd18ZfQv/9RHGsd1DvfU W8MouzDtUnN5IG1e36gFKuaScg== X-Google-Smtp-Source: ABdhPJwl2Ti+XW6IUIm1JffdsYaS5HUTaKwMNdPcSB7kzRn/dqv2nPmmkKujDxOZzCXsD3O1WddSZQ== X-Received: by 2002:a63:a401:: with SMTP id c1mr13477pgf.60.1614290360980; Thu, 25 Feb 2021 13:59:20 -0800 (PST) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id i8sm7302055pgn.94.2021.02.25.13.59.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Feb 2021 13:59:20 -0800 (PST) Date: Thu, 25 Feb 2021 13:59:19 -0800 From: Kees Cook To: John Ogness Subject: Re: [PATCH next v3 12/15] printk: introduce a kmsg_dump iterator Message-ID: <202102251358.60700B3FFA@keescook> References: <20210225202438.28985-1-john.ogness@linutronix.de> <20210225202438.28985-13-john.ogness@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210225202438.28985-13-john.ogness@linutronix.de> X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-hyperv@vger.kernel.org, Sergey Senozhatsky , Ravi Bangoria , Douglas Anderson , Paul Mackerras , Miquel Raynal , "K. Y. Srinivasan" , Thomas Meyer , Vignesh Raghavendra , Wei Liu , Madhavan Srinivasan , Stephen Hemminger , Anton Vorontsov , Joel Stanley , Jason Wessel , Anton Ivanov , Wei Li , Haiyang Zhang , Petr Mladek , Pavel Tatashin , Alistair Popple , Jeff Dike , Colin Cross , linux-um@lists.infradead.org, Daniel Thompson , Steven Rostedt , Davidlohr Bueso , Nicholas Piggin , Oleg Nesterov , Thomas Gleixner , Andy Shevchenko , Jordan Niethe , Michael Kelley , Christophe Leroy , Tony Luck , linux-kernel@vger.kernel.org, Sergey Senozhatsky , Richard Weinberger , kgdb-bugreport@lists.sourceforge.net, linux-mtd@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, Mike Rapoport Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Thu, Feb 25, 2021 at 09:24:35PM +0100, John Ogness wrote: > Rather than storing the iterator information in the registered > kmsg_dumper structure, create a separate iterator structure. The > kmsg_dump_iter structure can reside on the stack of the caller, thus > allowing lockless use of the kmsg_dump functions. > > This change also means that the kmsg_dumper dump() callback no > longer needs to pass in the kmsg_dumper as an argument. If > kmsg_dumpers want to access the kernel logs, they can use the new > iterator. > > Update the kmsg_dumper callback prototype. Update code that accesses > the kernel logs using the kmsg_dumper structure to use the new > kmsg_dump_iter structure. For kmsg_dumpers, this also means adding a > call to kmsg_dump_rewind() to initialize the iterator. > > All this is in preparation for removal of @logbuf_lock. > > Signed-off-by: John Ogness > --- > arch/powerpc/kernel/nvram_64.c | 14 +++--- > arch/powerpc/platforms/powernv/opal-kmsg.c | 3 +- > arch/powerpc/xmon/xmon.c | 6 +-- > arch/um/kernel/kmsg_dump.c | 8 +-- > drivers/hv/vmbus_drv.c | 7 +-- > drivers/mtd/mtdoops.c | 8 +-- > fs/pstore/platform.c | 8 +-- Reviewed-by: Kees Cook # pstore -Kees > include/linux/kmsg_dump.h | 38 ++++++++------- > kernel/debug/kdb/kdb_main.c | 10 ++-- > kernel/printk/printk.c | 57 ++++++++++------------ > 10 files changed, 81 insertions(+), 78 deletions(-) > > diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c > index 532f22637783..5a64b24a91c2 100644 > --- a/arch/powerpc/kernel/nvram_64.c > +++ b/arch/powerpc/kernel/nvram_64.c > @@ -72,8 +72,7 @@ static const char *nvram_os_partitions[] = { > NULL > }; > > -static void oops_to_nvram(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason); > +static void oops_to_nvram(enum kmsg_dump_reason reason); > > static struct kmsg_dumper nvram_kmsg_dumper = { > .dump = oops_to_nvram > @@ -642,11 +641,11 @@ void __init nvram_init_oops_partition(int rtas_partition_exists) > * that we think will compress sufficiently to fit in the lnx,oops-log > * partition. If that's too much, go back and capture uncompressed text. > */ > -static void oops_to_nvram(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void oops_to_nvram(enum kmsg_dump_reason reason) > { > struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf; > static unsigned int oops_count = 0; > + static struct kmsg_dump_iter iter; > static bool panicking = false; > static DEFINE_SPINLOCK(lock); > unsigned long flags; > @@ -681,13 +680,14 @@ static void oops_to_nvram(struct kmsg_dumper *dumper, > return; > > if (big_oops_buf) { > - kmsg_dump_get_buffer(dumper, false, > + kmsg_dump_rewind(&iter); > + kmsg_dump_get_buffer(&iter, false, > big_oops_buf, big_oops_buf_sz, &text_len); > rc = zip_oops(text_len); > } > if (rc != 0) { > - kmsg_dump_rewind(dumper); > - kmsg_dump_get_buffer(dumper, false, > + kmsg_dump_rewind(&iter); > + kmsg_dump_get_buffer(&iter, false, > oops_data, oops_data_sz, &text_len); > err_type = ERR_TYPE_KERNEL_PANIC; > oops_hdr->version = cpu_to_be16(OOPS_HDR_VERSION); > diff --git a/arch/powerpc/platforms/powernv/opal-kmsg.c b/arch/powerpc/platforms/powernv/opal-kmsg.c > index 6c3bc4b4da98..a7bd6ac681f4 100644 > --- a/arch/powerpc/platforms/powernv/opal-kmsg.c > +++ b/arch/powerpc/platforms/powernv/opal-kmsg.c > @@ -19,8 +19,7 @@ > * may not be completely printed. This function does not actually dump the > * message, it just ensures that OPAL completely flushes the console buffer. > */ > -static void kmsg_dump_opal_console_flush(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void kmsg_dump_opal_console_flush(enum kmsg_dump_reason reason) > { > /* > * Outside of a panic context the pollers will continue to run, > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c > index 80ed3e1becf9..5978b90a885f 100644 > --- a/arch/powerpc/xmon/xmon.c > +++ b/arch/powerpc/xmon/xmon.c > @@ -3001,7 +3001,7 @@ print_address(unsigned long addr) > static void > dump_log_buf(void) > { > - struct kmsg_dumper dumper; > + struct kmsg_dump_iter iter; > unsigned char buf[128]; > size_t len; > > @@ -3013,9 +3013,9 @@ dump_log_buf(void) > catch_memory_errors = 1; > sync(); > > - kmsg_dump_rewind_nolock(&dumper); > + kmsg_dump_rewind_nolock(&iter); > xmon_start_pagination(); > - while (kmsg_dump_get_line_nolock(&dumper, false, buf, sizeof(buf), &len)) { > + while (kmsg_dump_get_line_nolock(&iter, false, buf, sizeof(buf), &len)) { > buf[len] = '\0'; > printf("%s", buf); > } > diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c > index 4869e2cc787c..9fbc5e5b1023 100644 > --- a/arch/um/kernel/kmsg_dump.c > +++ b/arch/um/kernel/kmsg_dump.c > @@ -7,9 +7,9 @@ > #include > #include > > -static void kmsg_dumper_stdout(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void kmsg_dumper_stdout(enum kmsg_dump_reason reason) > { > + static struct kmsg_dump_iter iter; > static DEFINE_SPINLOCK(lock); > static char line[1024]; > struct console *con; > @@ -34,8 +34,10 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper, > if (!spin_trylock(&lock)) > return; > > + kmsg_dump_rewind(&iter); > + > printf("kmsg_dump:\n"); > - while (kmsg_dump_get_line(dumper, true, line, sizeof(line), &len)) { > + while (kmsg_dump_get_line(&iter, true, line, sizeof(line), &len)) { > line[len] = '\0'; > printf("%s", line); > } > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index 10dce9f91216..1b858f280e22 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -1388,9 +1388,9 @@ static void vmbus_isr(void) > * Callback from kmsg_dump. Grab as much as possible from the end of the kmsg > * buffer and call into Hyper-V to transfer the data. > */ > -static void hv_kmsg_dump(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void hv_kmsg_dump(enum kmsg_dump_reason reason) > { > + struct kmsg_dump_iter iter; > size_t bytes_written; > phys_addr_t panic_pa; > > @@ -1404,7 +1404,8 @@ static void hv_kmsg_dump(struct kmsg_dumper *dumper, > * Write dump contents to the page. No need to synchronize; panic should > * be single-threaded. > */ > - kmsg_dump_get_buffer(dumper, false, hv_panic_page, HV_HYP_PAGE_SIZE, > + kmsg_dump_rewind(&iter); > + kmsg_dump_get_buffer(&iter, false, hv_panic_page, HV_HYP_PAGE_SIZE, > &bytes_written); > if (bytes_written) > hyperv_report_panic_msg(panic_pa, bytes_written); > diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c > index 8bbfba40a554..d179b726a1c9 100644 > --- a/drivers/mtd/mtdoops.c > +++ b/drivers/mtd/mtdoops.c > @@ -272,19 +272,21 @@ static void find_next_position(struct mtdoops_context *cxt) > mtdoops_inc_counter(cxt); > } > > -static void mtdoops_do_dump(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void mtdoops_do_dump(enum kmsg_dump_reason reason) > { > struct mtdoops_context *cxt = container_of(dumper, > struct mtdoops_context, dump); > + struct kmsg_dump_iter iter; > > /* Only dump oopses if dump_oops is set */ > if (reason == KMSG_DUMP_OOPS && !dump_oops) > return; > > + kmsg_dump_rewind(&iter); > + > if (test_and_set_bit(0, &cxt->oops_buf_busy)) > return; > - kmsg_dump_get_buffer(dumper, true, cxt->oops_buf + MTDOOPS_HEADER_SIZE, > + kmsg_dump_get_buffer(&iter, true, cxt->oops_buf + MTDOOPS_HEADER_SIZE, > record_size - MTDOOPS_HEADER_SIZE, NULL); > clear_bit(0, &cxt->oops_buf_busy); > > diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c > index d963ae7902f9..edfc9504e024 100644 > --- a/fs/pstore/platform.c > +++ b/fs/pstore/platform.c > @@ -382,9 +382,9 @@ void pstore_record_init(struct pstore_record *record, > * callback from kmsg_dump. Save as much as we can (up to kmsg_bytes) from the > * end of the buffer. > */ > -static void pstore_dump(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void pstore_dump(enum kmsg_dump_reason reason) > { > + struct kmsg_dump_iter iter; > unsigned long total = 0; > const char *why; > unsigned int part = 1; > @@ -405,6 +405,8 @@ static void pstore_dump(struct kmsg_dumper *dumper, > } > } > > + kmsg_dump_rewind(&iter); > + > oopscount++; > while (total < kmsg_bytes) { > char *dst; > @@ -435,7 +437,7 @@ static void pstore_dump(struct kmsg_dumper *dumper, > dst_size -= header_size; > > /* Write dump contents. */ > - if (!kmsg_dump_get_buffer(dumper, true, dst + header_size, > + if (!kmsg_dump_get_buffer(&iter, true, dst + header_size, > dst_size, &dump_size)) > break; > > diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h > index 84eaa2090efa..5d3bf20f9f0a 100644 > --- a/include/linux/kmsg_dump.h > +++ b/include/linux/kmsg_dump.h > @@ -29,6 +29,16 @@ enum kmsg_dump_reason { > KMSG_DUMP_MAX > }; > > +/** > + * struct kmsg_dump_iter - iterator for retrieving kernel messages > + * @cur_seq: Points to the oldest message to dump > + * @next_seq: Points after the newest message to dump > + */ > +struct kmsg_dump_iter { > + u64 cur_seq; > + u64 next_seq; > +}; > + > /** > * struct kmsg_dumper - kernel crash message dumper structure > * @list: Entry in the dumper list (private) > @@ -36,35 +46,29 @@ enum kmsg_dump_reason { > * through the record iterator > * @max_reason: filter for highest reason number that should be dumped > * @registered: Flag that specifies if this is already registered > - * @cur_seq: Points to the oldest message to dump > - * @next_seq: Points after the newest message to dump > */ > struct kmsg_dumper { > struct list_head list; > - void (*dump)(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason); > + void (*dump)(enum kmsg_dump_reason reason); > enum kmsg_dump_reason max_reason; > bool registered; > - > - /* private state of the kmsg iterator */ > - u64 cur_seq; > - u64 next_seq; > }; > > #ifdef CONFIG_PRINTK > void kmsg_dump(enum kmsg_dump_reason reason); > > -bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog, > char *line, size_t size, size_t *len); > > -bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog, > char *line, size_t size, size_t *len); > > -bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog, > char *buf, size_t size, size_t *len_out); > > -void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper); > +void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter); > > -void kmsg_dump_rewind(struct kmsg_dumper *dumper); > +void kmsg_dump_rewind(struct kmsg_dump_iter *iter); > > int kmsg_dump_register(struct kmsg_dumper *dumper); > > @@ -76,30 +80,30 @@ static inline void kmsg_dump(enum kmsg_dump_reason reason) > { > } > > -static inline bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, > +static inline bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, > bool syslog, const char *line, > size_t size, size_t *len) > { > return false; > } > > -static inline bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog, > +static inline bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog, > const char *line, size_t size, size_t *len) > { > return false; > } > > -static inline bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > +static inline bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog, > char *buf, size_t size, size_t *len) > { > return false; > } > > -static inline void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper) > +static inline void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter) > { > } > > -static inline void kmsg_dump_rewind(struct kmsg_dumper *dumper) > +static inline void kmsg_dump_rewind(struct kmsg_dump_iter *iter) > { > } > > diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c > index 315169d5e119..8544d7a55a57 100644 > --- a/kernel/debug/kdb/kdb_main.c > +++ b/kernel/debug/kdb/kdb_main.c > @@ -2101,7 +2101,7 @@ static int kdb_dmesg(int argc, const char **argv) > int adjust = 0; > int n = 0; > int skip = 0; > - struct kmsg_dumper dumper; > + struct kmsg_dump_iter iter; > size_t len; > char buf[201]; > > @@ -2126,8 +2126,8 @@ static int kdb_dmesg(int argc, const char **argv) > kdb_set(2, setargs); > } > > - kmsg_dump_rewind_nolock(&dumper); > - while (kmsg_dump_get_line_nolock(&dumper, 1, NULL, 0, NULL)) > + kmsg_dump_rewind_nolock(&iter); > + while (kmsg_dump_get_line_nolock(&iter, 1, NULL, 0, NULL)) > n++; > > if (lines < 0) { > @@ -2159,8 +2159,8 @@ static int kdb_dmesg(int argc, const char **argv) > if (skip >= n || skip < 0) > return 0; > > - kmsg_dump_rewind_nolock(&dumper); > - while (kmsg_dump_get_line_nolock(&dumper, 1, buf, sizeof(buf), &len)) { > + kmsg_dump_rewind_nolock(&iter); > + while (kmsg_dump_get_line_nolock(&iter, 1, buf, sizeof(buf), &len)) { > if (skip) { > skip--; > continue; > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index 45cb3e9c62c5..e58ccc368348 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -3390,7 +3390,6 @@ EXPORT_SYMBOL_GPL(kmsg_dump_reason_str); > void kmsg_dump(enum kmsg_dump_reason reason) > { > struct kmsg_dumper *dumper; > - unsigned long flags; > > rcu_read_lock(); > list_for_each_entry_rcu(dumper, &dump_list, list) { > @@ -3407,21 +3406,15 @@ void kmsg_dump(enum kmsg_dump_reason reason) > if (reason > max_reason) > continue; > > - /* initialize iterator with data about the stored records */ > - logbuf_lock_irqsave(flags); > - dumper->cur_seq = latched_seq_read_nolock(&clear_seq); > - dumper->next_seq = prb_next_seq(prb); > - logbuf_unlock_irqrestore(flags); > - > /* invoke dumper which will iterate over records */ > - dumper->dump(dumper, reason); > + dumper->dump(reason); > } > rcu_read_unlock(); > } > > /** > * kmsg_dump_get_line_nolock - retrieve one kmsg log line (unlocked version) > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * @syslog: include the "<4>" prefixes > * @line: buffer to copy the line to > * @size: maximum size of the buffer > @@ -3438,7 +3431,7 @@ void kmsg_dump(enum kmsg_dump_reason reason) > * > * The function is similar to kmsg_dump_get_line(), but grabs no locks. > */ > -bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog, > char *line, size_t size, size_t *len) > { > struct printk_info info; > @@ -3451,11 +3444,11 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > > /* Read text or count text lines? */ > if (line) { > - if (!prb_read_valid(prb, dumper->cur_seq, &r)) > + if (!prb_read_valid(prb, iter->cur_seq, &r)) > goto out; > l = record_print_text(&r, syslog, printk_time); > } else { > - if (!prb_read_valid_info(prb, dumper->cur_seq, > + if (!prb_read_valid_info(prb, iter->cur_seq, > &info, &line_count)) { > goto out; > } > @@ -3464,7 +3457,7 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > > } > > - dumper->cur_seq = r.info->seq + 1; > + iter->cur_seq = r.info->seq + 1; > ret = true; > out: > if (len) > @@ -3474,7 +3467,7 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > > /** > * kmsg_dump_get_line - retrieve one kmsg log line > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * @syslog: include the "<4>" prefixes > * @line: buffer to copy the line to > * @size: maximum size of the buffer > @@ -3489,14 +3482,14 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > * A return value of FALSE indicates that there are no more records to > * read. > */ > -bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog, > char *line, size_t size, size_t *len) > { > unsigned long flags; > bool ret; > > logbuf_lock_irqsave(flags); > - ret = kmsg_dump_get_line_nolock(dumper, syslog, line, size, len); > + ret = kmsg_dump_get_line_nolock(iter, syslog, line, size, len); > logbuf_unlock_irqrestore(flags); > > return ret; > @@ -3505,7 +3498,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_line); > > /** > * kmsg_dump_get_buffer - copy kmsg log lines > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * @syslog: include the "<4>" prefixes > * @buf: buffer to copy the line to > * @size: maximum size of the buffer > @@ -3522,7 +3515,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_line); > * A return value of FALSE indicates that there are no more records to > * read. > */ > -bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog, > char *buf, size_t size, size_t *len_out) > { > struct printk_info info; > @@ -3538,15 +3531,15 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > goto out; > > logbuf_lock_irqsave(flags); > - if (prb_read_valid_info(prb, dumper->cur_seq, &info, NULL)) { > - if (info.seq != dumper->cur_seq) { > + if (prb_read_valid_info(prb, iter->cur_seq, &info, NULL)) { > + if (info.seq != iter->cur_seq) { > /* messages are gone, move to first available one */ > - dumper->cur_seq = info.seq; > + iter->cur_seq = info.seq; > } > } > > /* last entry */ > - if (dumper->cur_seq >= dumper->next_seq) { > + if (iter->cur_seq >= iter->next_seq) { > logbuf_unlock_irqrestore(flags); > goto out; > } > @@ -3557,7 +3550,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > * because this function (by way of record_print_text()) will > * not write more than size-1 bytes of text into @buf. > */ > - seq = find_first_fitting_seq(dumper->cur_seq, dumper->next_seq, > + seq = find_first_fitting_seq(iter->cur_seq, iter->next_seq, > size - 1, syslog, time); > > /* > @@ -3570,7 +3563,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > > len = 0; > prb_for_each_record(seq, prb, seq, &r) { > - if (r.info->seq >= dumper->next_seq) > + if (r.info->seq >= iter->next_seq) > break; > > len += record_print_text(&r, syslog, time); > @@ -3579,7 +3572,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > prb_rec_init_rd(&r, &info, buf + len, size - len); > } > > - dumper->next_seq = next_seq; > + iter->next_seq = next_seq; > ret = true; > logbuf_unlock_irqrestore(flags); > out: > @@ -3591,7 +3584,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer); > > /** > * kmsg_dump_rewind_nolock - reset the iterator (unlocked version) > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * > * Reset the dumper's iterator so that kmsg_dump_get_line() and > * kmsg_dump_get_buffer() can be called again and used multiple > @@ -3599,26 +3592,26 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer); > * > * The function is similar to kmsg_dump_rewind(), but grabs no locks. > */ > -void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper) > +void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter) > { > - dumper->cur_seq = latched_seq_read_nolock(&clear_seq); > - dumper->next_seq = prb_next_seq(prb); > + iter->cur_seq = latched_seq_read_nolock(&clear_seq); > + iter->next_seq = prb_next_seq(prb); > } > > /** > * kmsg_dump_rewind - reset the iterator > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * > * Reset the dumper's iterator so that kmsg_dump_get_line() and > * kmsg_dump_get_buffer() can be called again and used multiple > * times within the same dumper.dump() callback. > */ > -void kmsg_dump_rewind(struct kmsg_dumper *dumper) > +void kmsg_dump_rewind(struct kmsg_dump_iter *iter) > { > unsigned long flags; > > logbuf_lock_irqsave(flags); > - kmsg_dump_rewind_nolock(dumper); > + kmsg_dump_rewind_nolock(iter); > logbuf_unlock_irqrestore(flags); > } > EXPORT_SYMBOL_GPL(kmsg_dump_rewind); > -- > 2.20.1 > -- Kees Cook From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 164D9C433DB for ; Thu, 25 Feb 2021 22:00:33 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B1AF7614A7 for ; Thu, 25 Feb 2021 22:00:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B1AF7614A7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-mtd-bounces+linux-mtd=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=+0gTbA39kG4iqzLOG28qSaKYsNnnIKcJGI2pgJpzNDk=; b=OuLtpXodPe9ByZZEdRzn8qZLC +lDSt9OO+U/o9C/gu2tdLkUJ8vN5vTg09W11YQsdN2zsf0qwEvk6mHUDzztf4jld/JihCXm5mgW9a FcPEF38Xyb4lkeHPWsRrba4MSkhs9uJGqnF6adoRQS0l/qnaaH02UiYSZpOd/EY40IPsC9Vvj+KDz 7fz6Pokh5UmD8KeAoQkPPXywYOJ+dT1gPT1Aos42R1Onlew1k8d1bK6zkFqNp4wQwssjWILuX/wz3 kZwf/DDGJIqVO6SPJlZqmcFwzJl1vJJ6l0QWjxS38mzMWZFUFCrKL6rVVl/5xc1Gfd0VMW8Y0sLkw 9DXUcAzFA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1lFOfP-0006p7-MN; Thu, 25 Feb 2021 21:59:27 +0000 Received: from mail-pg1-x52e.google.com ([2607:f8b0:4864:20::52e]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1lFOfN-0006oQ-98 for linux-mtd@lists.infradead.org; Thu, 25 Feb 2021 21:59:26 +0000 Received: by mail-pg1-x52e.google.com with SMTP id a4so4715342pgc.11 for ; Thu, 25 Feb 2021 13:59:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=Lr+EAj1L3SvhS0sgLJSE5BurhCAU0L6vbZIwL4lD+n0=; b=Z+d9Z01pJ/XoTC1j3PIYh8Ln2Le+IRissEssJVg6YNao725xaEF8bjpXlUlTDafXw3 +X4+UGpVNnopdZH4n08e6Z42aULD6jxU7PhmbZXrGT0TvxJjtU2/1TJ8w+t26hmbItnN JuimIMyxjua8zK/cjQEM3tlvZOF35MWYInuI8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=Lr+EAj1L3SvhS0sgLJSE5BurhCAU0L6vbZIwL4lD+n0=; b=M0hb9i6hBtka+isiWq/lOJThAieYn9GpV5JKfL6hV8cjN371pWUFQgsJvfwqYWmy+b TNKwU3oLC73l3rnQqKFQOTlnx4YurWJbu6uV20Z/uzMMoe5EL59wrYesBgSdlnuAJ+oK 8ze1dzpvt09naBRRbG1uI5F1GaWgLmugjAgIYKvOC2O4uYQedNA1fVoutZSCn50shHux swLlm8YDMWQfyyajtMtkCnbJDYUNfsHs37pVYwh/JJSfAIrSJwOydcykZl+X+HFI+qa5 1ljFKUBHozNOGTq3Nx3UUAH3MtHGoc8UUDXGjOtNu+Gv5k94jNmA+guLdA9YjxTxkLrV DJ9g== X-Gm-Message-State: AOAM533LPlXuHhKu+UdgM3FXPmHB1YrNiaeF6hhkOW0T71jn9beyR6y/ 5d9z5NwLguUbSnMVEhRzGQqcyg== X-Google-Smtp-Source: ABdhPJwl2Ti+XW6IUIm1JffdsYaS5HUTaKwMNdPcSB7kzRn/dqv2nPmmkKujDxOZzCXsD3O1WddSZQ== X-Received: by 2002:a63:a401:: with SMTP id c1mr13477pgf.60.1614290360980; Thu, 25 Feb 2021 13:59:20 -0800 (PST) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id i8sm7302055pgn.94.2021.02.25.13.59.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Feb 2021 13:59:20 -0800 (PST) Date: Thu, 25 Feb 2021 13:59:19 -0800 From: Kees Cook To: John Ogness Subject: Re: [PATCH next v3 12/15] printk: introduce a kmsg_dump iterator Message-ID: <202102251358.60700B3FFA@keescook> References: <20210225202438.28985-1-john.ogness@linutronix.de> <20210225202438.28985-13-john.ogness@linutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210225202438.28985-13-john.ogness@linutronix.de> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210225_165925_365071_DEA9511B X-CRM114-Status: GOOD ( 38.89 ) X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-hyperv@vger.kernel.org, Sergey Senozhatsky , Ravi Bangoria , Benjamin Herrenschmidt , Douglas Anderson , Paul Mackerras , Miquel Raynal , "K. Y. Srinivasan" , Thomas Meyer , Vignesh Raghavendra , Wei Liu , Madhavan Srinivasan , Stephen Hemminger , Michael Ellerman , Anton Vorontsov , Joel Stanley , Jason Wessel , Anton Ivanov , Wei Li , Haiyang Zhang , Petr Mladek , Pavel Tatashin , Alistair Popple , Jeff Dike , Colin Cross , linux-um@lists.infradead.org, Daniel Thompson , Steven Rostedt , Davidlohr Bueso , Nicholas Piggin , Oleg Nesterov , Thomas Gleixner , Andy Shevchenko , Jordan Niethe , Michael Kelley , Christophe Leroy , Tony Luck , linux-kernel@vger.kernel.org, Sergey Senozhatsky , Richard Weinberger , kgdb-bugreport@lists.sourceforge.net, linux-mtd@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, Mike Rapoport Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-mtd" Errors-To: linux-mtd-bounces+linux-mtd=archiver.kernel.org@lists.infradead.org On Thu, Feb 25, 2021 at 09:24:35PM +0100, John Ogness wrote: > Rather than storing the iterator information in the registered > kmsg_dumper structure, create a separate iterator structure. The > kmsg_dump_iter structure can reside on the stack of the caller, thus > allowing lockless use of the kmsg_dump functions. > > This change also means that the kmsg_dumper dump() callback no > longer needs to pass in the kmsg_dumper as an argument. If > kmsg_dumpers want to access the kernel logs, they can use the new > iterator. > > Update the kmsg_dumper callback prototype. Update code that accesses > the kernel logs using the kmsg_dumper structure to use the new > kmsg_dump_iter structure. For kmsg_dumpers, this also means adding a > call to kmsg_dump_rewind() to initialize the iterator. > > All this is in preparation for removal of @logbuf_lock. > > Signed-off-by: John Ogness > --- > arch/powerpc/kernel/nvram_64.c | 14 +++--- > arch/powerpc/platforms/powernv/opal-kmsg.c | 3 +- > arch/powerpc/xmon/xmon.c | 6 +-- > arch/um/kernel/kmsg_dump.c | 8 +-- > drivers/hv/vmbus_drv.c | 7 +-- > drivers/mtd/mtdoops.c | 8 +-- > fs/pstore/platform.c | 8 +-- Reviewed-by: Kees Cook # pstore -Kees > include/linux/kmsg_dump.h | 38 ++++++++------- > kernel/debug/kdb/kdb_main.c | 10 ++-- > kernel/printk/printk.c | 57 ++++++++++------------ > 10 files changed, 81 insertions(+), 78 deletions(-) > > diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c > index 532f22637783..5a64b24a91c2 100644 > --- a/arch/powerpc/kernel/nvram_64.c > +++ b/arch/powerpc/kernel/nvram_64.c > @@ -72,8 +72,7 @@ static const char *nvram_os_partitions[] = { > NULL > }; > > -static void oops_to_nvram(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason); > +static void oops_to_nvram(enum kmsg_dump_reason reason); > > static struct kmsg_dumper nvram_kmsg_dumper = { > .dump = oops_to_nvram > @@ -642,11 +641,11 @@ void __init nvram_init_oops_partition(int rtas_partition_exists) > * that we think will compress sufficiently to fit in the lnx,oops-log > * partition. If that's too much, go back and capture uncompressed text. > */ > -static void oops_to_nvram(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void oops_to_nvram(enum kmsg_dump_reason reason) > { > struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf; > static unsigned int oops_count = 0; > + static struct kmsg_dump_iter iter; > static bool panicking = false; > static DEFINE_SPINLOCK(lock); > unsigned long flags; > @@ -681,13 +680,14 @@ static void oops_to_nvram(struct kmsg_dumper *dumper, > return; > > if (big_oops_buf) { > - kmsg_dump_get_buffer(dumper, false, > + kmsg_dump_rewind(&iter); > + kmsg_dump_get_buffer(&iter, false, > big_oops_buf, big_oops_buf_sz, &text_len); > rc = zip_oops(text_len); > } > if (rc != 0) { > - kmsg_dump_rewind(dumper); > - kmsg_dump_get_buffer(dumper, false, > + kmsg_dump_rewind(&iter); > + kmsg_dump_get_buffer(&iter, false, > oops_data, oops_data_sz, &text_len); > err_type = ERR_TYPE_KERNEL_PANIC; > oops_hdr->version = cpu_to_be16(OOPS_HDR_VERSION); > diff --git a/arch/powerpc/platforms/powernv/opal-kmsg.c b/arch/powerpc/platforms/powernv/opal-kmsg.c > index 6c3bc4b4da98..a7bd6ac681f4 100644 > --- a/arch/powerpc/platforms/powernv/opal-kmsg.c > +++ b/arch/powerpc/platforms/powernv/opal-kmsg.c > @@ -19,8 +19,7 @@ > * may not be completely printed. This function does not actually dump the > * message, it just ensures that OPAL completely flushes the console buffer. > */ > -static void kmsg_dump_opal_console_flush(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void kmsg_dump_opal_console_flush(enum kmsg_dump_reason reason) > { > /* > * Outside of a panic context the pollers will continue to run, > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c > index 80ed3e1becf9..5978b90a885f 100644 > --- a/arch/powerpc/xmon/xmon.c > +++ b/arch/powerpc/xmon/xmon.c > @@ -3001,7 +3001,7 @@ print_address(unsigned long addr) > static void > dump_log_buf(void) > { > - struct kmsg_dumper dumper; > + struct kmsg_dump_iter iter; > unsigned char buf[128]; > size_t len; > > @@ -3013,9 +3013,9 @@ dump_log_buf(void) > catch_memory_errors = 1; > sync(); > > - kmsg_dump_rewind_nolock(&dumper); > + kmsg_dump_rewind_nolock(&iter); > xmon_start_pagination(); > - while (kmsg_dump_get_line_nolock(&dumper, false, buf, sizeof(buf), &len)) { > + while (kmsg_dump_get_line_nolock(&iter, false, buf, sizeof(buf), &len)) { > buf[len] = '\0'; > printf("%s", buf); > } > diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c > index 4869e2cc787c..9fbc5e5b1023 100644 > --- a/arch/um/kernel/kmsg_dump.c > +++ b/arch/um/kernel/kmsg_dump.c > @@ -7,9 +7,9 @@ > #include > #include > > -static void kmsg_dumper_stdout(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void kmsg_dumper_stdout(enum kmsg_dump_reason reason) > { > + static struct kmsg_dump_iter iter; > static DEFINE_SPINLOCK(lock); > static char line[1024]; > struct console *con; > @@ -34,8 +34,10 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper, > if (!spin_trylock(&lock)) > return; > > + kmsg_dump_rewind(&iter); > + > printf("kmsg_dump:\n"); > - while (kmsg_dump_get_line(dumper, true, line, sizeof(line), &len)) { > + while (kmsg_dump_get_line(&iter, true, line, sizeof(line), &len)) { > line[len] = '\0'; > printf("%s", line); > } > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > index 10dce9f91216..1b858f280e22 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -1388,9 +1388,9 @@ static void vmbus_isr(void) > * Callback from kmsg_dump. Grab as much as possible from the end of the kmsg > * buffer and call into Hyper-V to transfer the data. > */ > -static void hv_kmsg_dump(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void hv_kmsg_dump(enum kmsg_dump_reason reason) > { > + struct kmsg_dump_iter iter; > size_t bytes_written; > phys_addr_t panic_pa; > > @@ -1404,7 +1404,8 @@ static void hv_kmsg_dump(struct kmsg_dumper *dumper, > * Write dump contents to the page. No need to synchronize; panic should > * be single-threaded. > */ > - kmsg_dump_get_buffer(dumper, false, hv_panic_page, HV_HYP_PAGE_SIZE, > + kmsg_dump_rewind(&iter); > + kmsg_dump_get_buffer(&iter, false, hv_panic_page, HV_HYP_PAGE_SIZE, > &bytes_written); > if (bytes_written) > hyperv_report_panic_msg(panic_pa, bytes_written); > diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c > index 8bbfba40a554..d179b726a1c9 100644 > --- a/drivers/mtd/mtdoops.c > +++ b/drivers/mtd/mtdoops.c > @@ -272,19 +272,21 @@ static void find_next_position(struct mtdoops_context *cxt) > mtdoops_inc_counter(cxt); > } > > -static void mtdoops_do_dump(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void mtdoops_do_dump(enum kmsg_dump_reason reason) > { > struct mtdoops_context *cxt = container_of(dumper, > struct mtdoops_context, dump); > + struct kmsg_dump_iter iter; > > /* Only dump oopses if dump_oops is set */ > if (reason == KMSG_DUMP_OOPS && !dump_oops) > return; > > + kmsg_dump_rewind(&iter); > + > if (test_and_set_bit(0, &cxt->oops_buf_busy)) > return; > - kmsg_dump_get_buffer(dumper, true, cxt->oops_buf + MTDOOPS_HEADER_SIZE, > + kmsg_dump_get_buffer(&iter, true, cxt->oops_buf + MTDOOPS_HEADER_SIZE, > record_size - MTDOOPS_HEADER_SIZE, NULL); > clear_bit(0, &cxt->oops_buf_busy); > > diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c > index d963ae7902f9..edfc9504e024 100644 > --- a/fs/pstore/platform.c > +++ b/fs/pstore/platform.c > @@ -382,9 +382,9 @@ void pstore_record_init(struct pstore_record *record, > * callback from kmsg_dump. Save as much as we can (up to kmsg_bytes) from the > * end of the buffer. > */ > -static void pstore_dump(struct kmsg_dumper *dumper, > - enum kmsg_dump_reason reason) > +static void pstore_dump(enum kmsg_dump_reason reason) > { > + struct kmsg_dump_iter iter; > unsigned long total = 0; > const char *why; > unsigned int part = 1; > @@ -405,6 +405,8 @@ static void pstore_dump(struct kmsg_dumper *dumper, > } > } > > + kmsg_dump_rewind(&iter); > + > oopscount++; > while (total < kmsg_bytes) { > char *dst; > @@ -435,7 +437,7 @@ static void pstore_dump(struct kmsg_dumper *dumper, > dst_size -= header_size; > > /* Write dump contents. */ > - if (!kmsg_dump_get_buffer(dumper, true, dst + header_size, > + if (!kmsg_dump_get_buffer(&iter, true, dst + header_size, > dst_size, &dump_size)) > break; > > diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h > index 84eaa2090efa..5d3bf20f9f0a 100644 > --- a/include/linux/kmsg_dump.h > +++ b/include/linux/kmsg_dump.h > @@ -29,6 +29,16 @@ enum kmsg_dump_reason { > KMSG_DUMP_MAX > }; > > +/** > + * struct kmsg_dump_iter - iterator for retrieving kernel messages > + * @cur_seq: Points to the oldest message to dump > + * @next_seq: Points after the newest message to dump > + */ > +struct kmsg_dump_iter { > + u64 cur_seq; > + u64 next_seq; > +}; > + > /** > * struct kmsg_dumper - kernel crash message dumper structure > * @list: Entry in the dumper list (private) > @@ -36,35 +46,29 @@ enum kmsg_dump_reason { > * through the record iterator > * @max_reason: filter for highest reason number that should be dumped > * @registered: Flag that specifies if this is already registered > - * @cur_seq: Points to the oldest message to dump > - * @next_seq: Points after the newest message to dump > */ > struct kmsg_dumper { > struct list_head list; > - void (*dump)(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason); > + void (*dump)(enum kmsg_dump_reason reason); > enum kmsg_dump_reason max_reason; > bool registered; > - > - /* private state of the kmsg iterator */ > - u64 cur_seq; > - u64 next_seq; > }; > > #ifdef CONFIG_PRINTK > void kmsg_dump(enum kmsg_dump_reason reason); > > -bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog, > char *line, size_t size, size_t *len); > > -bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog, > char *line, size_t size, size_t *len); > > -bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog, > char *buf, size_t size, size_t *len_out); > > -void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper); > +void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter); > > -void kmsg_dump_rewind(struct kmsg_dumper *dumper); > +void kmsg_dump_rewind(struct kmsg_dump_iter *iter); > > int kmsg_dump_register(struct kmsg_dumper *dumper); > > @@ -76,30 +80,30 @@ static inline void kmsg_dump(enum kmsg_dump_reason reason) > { > } > > -static inline bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, > +static inline bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, > bool syslog, const char *line, > size_t size, size_t *len) > { > return false; > } > > -static inline bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog, > +static inline bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog, > const char *line, size_t size, size_t *len) > { > return false; > } > > -static inline bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > +static inline bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog, > char *buf, size_t size, size_t *len) > { > return false; > } > > -static inline void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper) > +static inline void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter) > { > } > > -static inline void kmsg_dump_rewind(struct kmsg_dumper *dumper) > +static inline void kmsg_dump_rewind(struct kmsg_dump_iter *iter) > { > } > > diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c > index 315169d5e119..8544d7a55a57 100644 > --- a/kernel/debug/kdb/kdb_main.c > +++ b/kernel/debug/kdb/kdb_main.c > @@ -2101,7 +2101,7 @@ static int kdb_dmesg(int argc, const char **argv) > int adjust = 0; > int n = 0; > int skip = 0; > - struct kmsg_dumper dumper; > + struct kmsg_dump_iter iter; > size_t len; > char buf[201]; > > @@ -2126,8 +2126,8 @@ static int kdb_dmesg(int argc, const char **argv) > kdb_set(2, setargs); > } > > - kmsg_dump_rewind_nolock(&dumper); > - while (kmsg_dump_get_line_nolock(&dumper, 1, NULL, 0, NULL)) > + kmsg_dump_rewind_nolock(&iter); > + while (kmsg_dump_get_line_nolock(&iter, 1, NULL, 0, NULL)) > n++; > > if (lines < 0) { > @@ -2159,8 +2159,8 @@ static int kdb_dmesg(int argc, const char **argv) > if (skip >= n || skip < 0) > return 0; > > - kmsg_dump_rewind_nolock(&dumper); > - while (kmsg_dump_get_line_nolock(&dumper, 1, buf, sizeof(buf), &len)) { > + kmsg_dump_rewind_nolock(&iter); > + while (kmsg_dump_get_line_nolock(&iter, 1, buf, sizeof(buf), &len)) { > if (skip) { > skip--; > continue; > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index 45cb3e9c62c5..e58ccc368348 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -3390,7 +3390,6 @@ EXPORT_SYMBOL_GPL(kmsg_dump_reason_str); > void kmsg_dump(enum kmsg_dump_reason reason) > { > struct kmsg_dumper *dumper; > - unsigned long flags; > > rcu_read_lock(); > list_for_each_entry_rcu(dumper, &dump_list, list) { > @@ -3407,21 +3406,15 @@ void kmsg_dump(enum kmsg_dump_reason reason) > if (reason > max_reason) > continue; > > - /* initialize iterator with data about the stored records */ > - logbuf_lock_irqsave(flags); > - dumper->cur_seq = latched_seq_read_nolock(&clear_seq); > - dumper->next_seq = prb_next_seq(prb); > - logbuf_unlock_irqrestore(flags); > - > /* invoke dumper which will iterate over records */ > - dumper->dump(dumper, reason); > + dumper->dump(reason); > } > rcu_read_unlock(); > } > > /** > * kmsg_dump_get_line_nolock - retrieve one kmsg log line (unlocked version) > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * @syslog: include the "<4>" prefixes > * @line: buffer to copy the line to > * @size: maximum size of the buffer > @@ -3438,7 +3431,7 @@ void kmsg_dump(enum kmsg_dump_reason reason) > * > * The function is similar to kmsg_dump_get_line(), but grabs no locks. > */ > -bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_line_nolock(struct kmsg_dump_iter *iter, bool syslog, > char *line, size_t size, size_t *len) > { > struct printk_info info; > @@ -3451,11 +3444,11 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > > /* Read text or count text lines? */ > if (line) { > - if (!prb_read_valid(prb, dumper->cur_seq, &r)) > + if (!prb_read_valid(prb, iter->cur_seq, &r)) > goto out; > l = record_print_text(&r, syslog, printk_time); > } else { > - if (!prb_read_valid_info(prb, dumper->cur_seq, > + if (!prb_read_valid_info(prb, iter->cur_seq, > &info, &line_count)) { > goto out; > } > @@ -3464,7 +3457,7 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > > } > > - dumper->cur_seq = r.info->seq + 1; > + iter->cur_seq = r.info->seq + 1; > ret = true; > out: > if (len) > @@ -3474,7 +3467,7 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > > /** > * kmsg_dump_get_line - retrieve one kmsg log line > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * @syslog: include the "<4>" prefixes > * @line: buffer to copy the line to > * @size: maximum size of the buffer > @@ -3489,14 +3482,14 @@ bool kmsg_dump_get_line_nolock(struct kmsg_dumper *dumper, bool syslog, > * A return value of FALSE indicates that there are no more records to > * read. > */ > -bool kmsg_dump_get_line(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog, > char *line, size_t size, size_t *len) > { > unsigned long flags; > bool ret; > > logbuf_lock_irqsave(flags); > - ret = kmsg_dump_get_line_nolock(dumper, syslog, line, size, len); > + ret = kmsg_dump_get_line_nolock(iter, syslog, line, size, len); > logbuf_unlock_irqrestore(flags); > > return ret; > @@ -3505,7 +3498,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_line); > > /** > * kmsg_dump_get_buffer - copy kmsg log lines > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * @syslog: include the "<4>" prefixes > * @buf: buffer to copy the line to > * @size: maximum size of the buffer > @@ -3522,7 +3515,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_line); > * A return value of FALSE indicates that there are no more records to > * read. > */ > -bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > +bool kmsg_dump_get_buffer(struct kmsg_dump_iter *iter, bool syslog, > char *buf, size_t size, size_t *len_out) > { > struct printk_info info; > @@ -3538,15 +3531,15 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > goto out; > > logbuf_lock_irqsave(flags); > - if (prb_read_valid_info(prb, dumper->cur_seq, &info, NULL)) { > - if (info.seq != dumper->cur_seq) { > + if (prb_read_valid_info(prb, iter->cur_seq, &info, NULL)) { > + if (info.seq != iter->cur_seq) { > /* messages are gone, move to first available one */ > - dumper->cur_seq = info.seq; > + iter->cur_seq = info.seq; > } > } > > /* last entry */ > - if (dumper->cur_seq >= dumper->next_seq) { > + if (iter->cur_seq >= iter->next_seq) { > logbuf_unlock_irqrestore(flags); > goto out; > } > @@ -3557,7 +3550,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > * because this function (by way of record_print_text()) will > * not write more than size-1 bytes of text into @buf. > */ > - seq = find_first_fitting_seq(dumper->cur_seq, dumper->next_seq, > + seq = find_first_fitting_seq(iter->cur_seq, iter->next_seq, > size - 1, syslog, time); > > /* > @@ -3570,7 +3563,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > > len = 0; > prb_for_each_record(seq, prb, seq, &r) { > - if (r.info->seq >= dumper->next_seq) > + if (r.info->seq >= iter->next_seq) > break; > > len += record_print_text(&r, syslog, time); > @@ -3579,7 +3572,7 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog, > prb_rec_init_rd(&r, &info, buf + len, size - len); > } > > - dumper->next_seq = next_seq; > + iter->next_seq = next_seq; > ret = true; > logbuf_unlock_irqrestore(flags); > out: > @@ -3591,7 +3584,7 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer); > > /** > * kmsg_dump_rewind_nolock - reset the iterator (unlocked version) > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * > * Reset the dumper's iterator so that kmsg_dump_get_line() and > * kmsg_dump_get_buffer() can be called again and used multiple > @@ -3599,26 +3592,26 @@ EXPORT_SYMBOL_GPL(kmsg_dump_get_buffer); > * > * The function is similar to kmsg_dump_rewind(), but grabs no locks. > */ > -void kmsg_dump_rewind_nolock(struct kmsg_dumper *dumper) > +void kmsg_dump_rewind_nolock(struct kmsg_dump_iter *iter) > { > - dumper->cur_seq = latched_seq_read_nolock(&clear_seq); > - dumper->next_seq = prb_next_seq(prb); > + iter->cur_seq = latched_seq_read_nolock(&clear_seq); > + iter->next_seq = prb_next_seq(prb); > } > > /** > * kmsg_dump_rewind - reset the iterator > - * @dumper: registered kmsg dumper > + * @iter: kmsg dump iterator > * > * Reset the dumper's iterator so that kmsg_dump_get_line() and > * kmsg_dump_get_buffer() can be called again and used multiple > * times within the same dumper.dump() callback. > */ > -void kmsg_dump_rewind(struct kmsg_dumper *dumper) > +void kmsg_dump_rewind(struct kmsg_dump_iter *iter) > { > unsigned long flags; > > logbuf_lock_irqsave(flags); > - kmsg_dump_rewind_nolock(dumper); > + kmsg_dump_rewind_nolock(iter); > logbuf_unlock_irqrestore(flags); > } > EXPORT_SYMBOL_GPL(kmsg_dump_rewind); > -- > 2.20.1 > -- Kees Cook ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/