[PATCH] vmcore-dmesg: avoid allocating large memory chunk for log buf

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ping... Any review?

On Mon, Jun 19, 2017 at 3:23 PM, Cong Wang <xiyou.wangcong at gmail.com> wrote:
> In kdump kernel, we got:
>
>     Saving vmcore-dmesg.txt
>     Failed to malloc 67108864 bytes for the logbuf: Cannot allocate memory
>
> Apparently 64M is too luxury for a kdump kernel which only has 128M
> in total.
>
> We can avoid allocating such a large memory chunk, instead just allocate
> a smaller chunk inside the loop each time.
>
> Verify the result by comparing the vmcore-dmesg output with and without
> this patch.
>
> Cc: Simon Horman <horms at verge.net.au>
> Signed-off-by: Cong Wang <xiyou.wangcong at gmail.com>
> ---
>  vmcore-dmesg/vmcore-dmesg.c | 101 ++++++++++++++++++--------------------------
>  1 file changed, 41 insertions(+), 60 deletions(-)
>
> diff --git a/vmcore-dmesg/vmcore-dmesg.c b/vmcore-dmesg/vmcore-dmesg.c
> index a8f56df..593557c 100644
> --- a/vmcore-dmesg/vmcore-dmesg.c
> +++ b/vmcore-dmesg/vmcore-dmesg.c
> @@ -565,53 +565,12 @@ static inline uint64_t struct_val_u64(char *ptr, unsigned int offset)
>         return(file64_to_cpu(*(uint64_t *)(ptr + offset)));
>  }
>
> -/* human readable text of the record */
> -static char *log_text(char *msg)
> -{
> -       return msg + log_sz;
> -}
> -
> -/* get record by index; idx must point to valid msg */
> -static char *log_from_idx(char *log_buf, uint32_t idx)
> -{
> -       char *msg = log_buf + idx;
> -
> -       /*
> -        * A length == 0 record is the end of buffer marker. Wrap around and
> -        * read the message at the start of the buffer.
> -        */
> -       if (!struct_val_u16(msg, log_offset_len))
> -               return log_buf;
> -       return msg;
> -}
> -
> -/* get next record; idx must point to valid msg */
> -static uint32_t log_next(char *log_buf, uint32_t idx)
> -{
> -       char *msg = log_buf + idx;
> -       uint16_t len;
> -
> -       /* length == 0 indicates the end of the buffer; wrap */
> -       /*
> -        * A length == 0 record is the end of buffer marker. Wrap around and
> -        * read the message at the start of the buffer as *this* one, and
> -        * return the one after that.
> -        */
> -       len = struct_val_u16(msg, log_offset_len);
> -       if (!len) {
> -               msg = log_buf;
> -               return struct_val_u16(msg, log_offset_len);
> -       }
> -       return idx + len;
> -}
> -
>  /* Read headers of log records and dump accordingly */
>  static void dump_dmesg_structured(int fd)
>  {
>  #define OUT_BUF_SIZE   4096
>         uint64_t log_buf, log_buf_offset, ts_nsec;
>         uint32_t log_first_idx, log_next_idx, current_idx, len = 0, i;
> -       int log_buf_len;
>         char *buf, out_buf[OUT_BUF_SIZE];
>         ssize_t ret;
>         char *msg;
> @@ -659,34 +618,33 @@ static void dump_dmesg_structured(int fd)
>         }
>
>         log_buf = read_file_pointer(fd, vaddr_to_offset(log_buf_vaddr));
> -       log_buf_len = read_file_s32(fd, vaddr_to_offset(log_buf_len_vaddr));
>
>         log_first_idx = read_file_u32(fd, vaddr_to_offset(log_first_idx_vaddr));
>         log_next_idx = read_file_u32(fd, vaddr_to_offset(log_next_idx_vaddr));
>
>         log_buf_offset = vaddr_to_offset(log_buf);
>
> -       buf = calloc(1, log_buf_len);
> +       buf = calloc(1, log_sz);
>         if (!buf) {
> -               fprintf(stderr, "Failed to malloc %d bytes for the logbuf:"
> -                               " %s\n", log_buf_len, strerror(errno));
> +               fprintf(stderr, "Failed to malloc %" PRId64 " bytes for the log:"
> +                               " %s\n", log_sz, strerror(errno));
>                 exit(64);
>         }
>
> -       ret = pread(fd, buf, log_buf_len, log_buf_offset);
> -       if (ret != log_buf_len) {
> -               fprintf(stderr, "Failed to read log buffer of size %d bytes:"
> -                       " %s\n", log_buf_len, strerror(errno));
> -               exit(65);
> -       }
> -
>         /* Parse records and write out data at standard output */
>
>         current_idx = log_first_idx;
>         len = 0;
>         while (current_idx != log_next_idx) {
> -               msg = log_from_idx(buf, current_idx);
> -               ts_nsec = struct_val_u64(msg, log_offset_ts_nsec);
> +               uint16_t loglen;
> +
> +               ret = pread(fd, buf, log_sz, log_buf_offset + current_idx);
> +               if (ret != log_sz) {
> +                       fprintf(stderr, "Failed to read log of size %" PRId64 " bytes:"
> +                               " %s\n", log_sz, strerror(errno));
> +                       exit(65);
> +               }
> +               ts_nsec = struct_val_u64(buf, log_offset_ts_nsec);
>                 imaxdiv_sec = imaxdiv(ts_nsec, 1000000000);
>                 imaxdiv_usec = imaxdiv(imaxdiv_sec.rem, 1000);
>
> @@ -695,9 +653,22 @@ static void dump_dmesg_structured(int fd)
>                         (long long unsigned int)imaxdiv_usec.quot);
>
>                 /* escape non-printable characters */
> -               text_len = struct_val_u16(msg, log_offset_text_len);
> +               text_len = struct_val_u16(buf, log_offset_text_len);
> +               msg = calloc(1, text_len);
> +               if (!msg) {
> +                       fprintf(stderr, "Failed to malloc %u bytes for log text:"
> +                               " %s\n", text_len, strerror(errno));
> +                       exit(64);
> +               }
> +
> +               ret = pread(fd, msg, text_len, log_buf_offset + current_idx + log_sz);
> +               if (ret != text_len) {
> +                       fprintf(stderr, "Failed to read log text of size %u bytes:"
> +                               " %s\n", text_len, strerror(errno));
> +                       exit(65);
> +               }
>                 for (i = 0; i < text_len; i++) {
> -                       unsigned char c = log_text(msg)[i];
> +                       unsigned char c = msg[i];
>
>                         if (!isprint(c) && !isspace(c))
>                                 len += sprintf(out_buf + len, "\\x%02x", c);
> @@ -711,11 +682,21 @@ static void dump_dmesg_structured(int fd)
>                 }
>
>                 out_buf[len++] = '\n';
> -
> -               /* Move to next record */
> -               current_idx = log_next(buf, current_idx);
> +               free(msg);
> +               /* length == 0 indicates the end of the buffer; wrap */
> +               /*
> +                * A length == 0 record is the end of buffer marker. Wrap around and
> +                * read the message at the start of the buffer as *this* one, and
> +                * return the one after that.
> +                */
> +               loglen = struct_val_u16(buf, log_offset_len);
> +               if (!loglen)
> +                       current_idx = log_first_idx;
> +               else
> +                       /* Move to next record */
> +                       current_idx += loglen;
>         }
> -
> +       free(buf);
>         if (len)
>                 write_to_stdout(out_buf, len);
>  }
> --
> 2.5.5
>



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux