printk: introduce a kmsg_dump iterator

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.

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 <john.ogness@linutronix.de>
Reviewed-by: Kees Cook <keescook@chromium.org> # pstore
Reviewed-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
Link: https://lore.kernel.org/r/20210303101528.29901-13-john.ogness@linutronix.de

diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index 532f226377831d89c592696df54794eaab311a8f..3c8d9bbb51cfa8c318834431ab7e2cc941545e92 100644 (file)
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -647,6 +647,7 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
 {
        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 +682,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/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a777573a0c38a2edae4f67503ac7bf955ecc4625..dbcf403b1d8dec47601886042a78ce12eb0c07f3 100644 (file)
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3005,7 +3005,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;
 
@@ -3017,9 +3017,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 a765d235e50ee7e020d301eedf9fefe7c60752ae..0224fcb36e22cb17a08a6c5276a809f426eb2070 100644 (file)
--- a/arch/um/kernel/kmsg_dump.c
+++ b/arch/um/kernel/kmsg_dump.c
@@ -10,6 +10,7 @@
 static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
                                enum kmsg_dump_reason reason)
 {
+       static struct kmsg_dump_iter iter;
        static DEFINE_SPINLOCK(lock);
        static char line[1024];
        struct console *con;
@@ -35,8 +36,10 @@ static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
        if (!spin_trylock_irqsave(&lock, flags))
                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 10dce9f9121689eb32e4c941c6ee8d88927bc5fc..b341b144bde8de038ad653183e375bf432e8659f 100644 (file)
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1391,6 +1391,7 @@ static void vmbus_isr(void)
 static void hv_kmsg_dump(struct kmsg_dumper *dumper,
                         enum kmsg_dump_reason reason)
 {
+       struct kmsg_dump_iter iter;
        size_t bytes_written;
        phys_addr_t panic_pa;
 
@@ -1404,7 +1405,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 8bbfba40a554047b964fa94f898dc848ae3acd7a..862c4a8892346bc1ff1919e9ffe4eb018abce379 100644 (file)
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -277,14 +277,17 @@ static void mtdoops_do_dump(struct kmsg_dumper *dumper,
 {
        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 d963ae7902f92fb5252f498f589c079ae65b0253..b9614db48b1def54fa302a1bd8cbf2c656a7bccf 100644 (file)
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -385,6 +385,7 @@ void pstore_record_init(struct pstore_record *record,
 static void pstore_dump(struct kmsg_dumper *dumper,
                        enum kmsg_dump_reason reason)
 {
+       struct kmsg_dump_iter iter;
        unsigned long   total = 0;
        const char      *why;
        unsigned int    part = 1;
@@ -405,6 +406,8 @@ static void pstore_dump(struct kmsg_dumper *dumper,
                }
        }
 
+       kmsg_dump_rewind(&iter);
+
        oopscount++;
        while (total < kmsg_bytes) {
                char *dst;
@@ -435,7 +438,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 84eaa2090efa1ed9c1f7a1f9ae1b7308d40e188d..36c8c57e10510ff88271f5cea149141c2a3ac53b 100644 (file)
--- 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);
        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 315169d5e1195a1503bde4a9d762d3b2bdc57c44..8544d7a55a57bd9b5ddf9436e8748910041f9050 100644 (file)
--- 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 ce4cc64ba7c940650c399f17f299913d184695b4..b49dee2569478db4279f78457009521b7dcdae25 100644 (file)
--- 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,12 +3406,6 @@ 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);
        }
@@ -3421,7 +3414,7 @@ void kmsg_dump(enum kmsg_dump_reason reason)
 
 /**
  * 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,24 +3431,28 @@ 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)
 {
+       u64 min_seq = latched_seq_read_nolock(&clear_seq);
        struct printk_info info;
        unsigned int line_count;
        struct printk_record r;
        size_t l = 0;
        bool ret = false;
 
+       if (iter->cur_seq < min_seq)
+               iter->cur_seq = min_seq;
+
        prb_rec_init_rd(&r, &info, line, size);
 
        /* 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 +3461,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 +3471,7 @@ out:
 
 /**
  * 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 +3486,14 @@ out:
  * 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 +3502,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,9 +3519,10 @@ 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)
 {
+       u64 min_seq = latched_seq_read_nolock(&clear_seq);
        struct printk_info info;
        struct printk_record r;
        unsigned long flags;
@@ -3537,16 +3535,19 @@ bool kmsg_dump_get_buffer(struct kmsg_dumper *dumper, bool syslog,
        if (!buf || !size)
                goto out;
 
+       if (iter->cur_seq < min_seq)
+               iter->cur_seq = min_seq;
+
        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 +3558,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 +3571,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 +3580,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 +3592,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 +3600,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);