Re: [PATCH v2 4/4] make: replace make by $(MAKE)

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

 



Hi, Sven and Kazu

The patchset really saves compilation time for crash build and the following three patches look good to me(need a minor adjustment as Kazu mentioned).
Thank you for the work, Sven.

[PATCH v2 1/4] make: set --no-print-directory once
[PATCH v2 3/4] make: use -C instead of (cd x; make)
[PATCH v2 4/4] make: replace make by $(MAKE)

Given that the gdb patch has specific rules,  Kazu(or me) can help to pack them into one patch with Sven's signature, if you have no objections.
Or could you please update the patchset? Sven.

Thanks.
Lianbo

On Thu, Dec 23, 2021 at 5:14 PM <crash-utility-request@xxxxxxxxxx> wrote:
Date: Thu, 23 Dec 2021 00:33:16 +0000
From: HAGIO KAZUHITO(?????)     <k-hagio-ab@xxxxxxx>
To: Sven Schnelle <svens@xxxxxxxxxxxxx>
Cc: "Discussion list for crash utility usage,   maintenance and
        development" <crash-utility@xxxxxxxxxx>
Subject: Re: [PATCH v2 4/4] make: replace make by
        $(MAKE)
Message-ID:
        <OS3PR01MB677601D042591DF6451F477DDD7E9@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>

Content-Type: text/plain; charset="iso-2022-jp"

Hi Sven,

thank you for the update.

-----Original Message-----
> diff --git a/gdb-10.2.patch b/gdb-10.2.patch
> index 1280d0688e83..afdbe99eae0d 100644
> --- a/gdb-10.2.patch
> +++ b/gdb-10.2.patch
> @@ -73,7 +73,7 @@
>   # Removing the old gdb first works better if it is running, at least on SunOS.
>   gdb$(EXEEXT): gdb.o $(LIBGDB_OBS) $(CDEPS) $(TDEPLIBS)
>       $(SILENCE) rm -f gdb$(EXEEXT)
> -+    @make -C ../.. GDB_FLAGS=-DGDB_10_2 library
> ++    @$(MAKE) -C ../.. GDB_FLAGS=-DGDB_10_2 library
>       $(ECHO_CXXLD) $(CC_LD) $(INTERNAL_LDFLAGS) $(WIN32LDAPP) \
>  -            -o gdb$(EXEEXT) gdb.o $(LIBGDB_OBS) \
>  -            $(TDEPLIBS) $(TUI_LIBRARY) $(CLIBS) $(LOADLIBES)

When detecting any change of the gdb patch, it tries to re-apply the new one
using "patch -N --fuzz=0" in order to update the gdb.  Please refer to
Makefile::rebuild and the head of the gdb-10.2.patch.

So I think that, to make a change to the gdb sources,

1. we have to "add" patches to the end of the gdb patch, and
2. if there are multiple patches for a gdb file, the "patch -N" doesn't work,
so we have to revert the gdb file to the original one.

(I will add this custom to crash guidelines later..)

In your patches, there are multiple patches for gdb-10.2/gdb/Makefile.in
so I would suggest that:

1. pack the 1/4, 3/4 and 4/4 patches into a patch, do the three things together.
2. add a hunk for the gdb-10.2/gdb/Makefile.in to the end of the gdb-10.2.patch.
3. add the following change to the head the gdb-10.2.patch.

--- a/gdb-10.2.patch
+++ b/gdb-10.2.patch
@@ -8,6 +8,11 @@
 # shell script that can restore any gdb file to its original state prior
 # to all subsequent patch applications.

+tar xvzmf gdb-10.2.tar.gz \
+       gdb-10.2/gdb/Makefile.in
+
+exit 0
+

(yeah, we forgot the "exit 0" here when moving to gdb-10.2..)

Thanks,
Kazu




------------------------------

Message: 2
Date: Thu, 23 Dec 2021 14:42:33 +0800
From: Lianbo Jiang <lijiang@xxxxxxxxxx>
To: crash-utility@xxxxxxxxxx
Subject: [PATCH] Handle blk_mq_ctx member changes for
        kernels v5.16-rc1~75^2~44
Message-ID: <20211223064233.16028-1-lijiang@xxxxxxxxxx>

Kernel commit <9a14d6ce4135> ("block: remove debugfs blk_mq_ctx
dispatched/merged/completed attributes") removed the member
rq_dispatched and rq_completed from struct blk_mq_ctx. Without
this patch, crash will fail with the following error:

crash> dev -d
MAJOR GENDISK            NAME       REQUEST_QUEUE      TOTAL ASYNC  SYNC

dev: invalid structure member offset: blk_mq_ctx_rq_dispatched
     FILE: dev.c  LINE: 4229  FUNCTION: get_one_mctx_diskio()

Signed-off-by: Lianbo Jiang <lijiang@xxxxxxxxxx>
---
 dev.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/dev.c b/dev.c
index effe789f38d8..dd21511e5dfc 100644
--- a/dev.c
+++ b/dev.c
@@ -4246,6 +4246,10 @@ get_mq_diskio(unsigned long q, unsigned long *mq_count)
        unsigned long mctx_addr;
        struct diskio tmp;

+       if (!MEMBER_EXISTS("blk_mq_ctx", "rq_dispatched") &&
+               !MEMBER_EXISTS("blk_mq_ctx", "rq_completed"))
+               return;
+
        memset(&tmp, 0x00, sizeof(struct diskio));

        readmem(q + OFFSET(request_queue_queue_ctx), KVADDR, &queue_ctx,
--
2.20.1



------------------------------

Message: 3
Date: Thu, 23 Dec 2021 09:13:19 +0000
From: "d.hatayama@xxxxxxxxxxx" <d.hatayama@xxxxxxxxxxx>
To: "shogo.matsumoto@xxxxxxxxxxx" <shogo.matsumoto@xxxxxxxxxxx>
Cc: "'crash-utility@xxxxxxxxxx'" <crash-utility@xxxxxxxxxx>
Subject: Re: [PATCH] log: output logs of printk safe
        buffers
Message-ID:
        <TYAPR01MB6507BFD2235A5C6806C3D0E5957E9@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>

Content-Type: text/plain; charset="iso-2022-jp"

Hi,

>
> ________________________________________
> From: crash-utility-bounces@xxxxxxxxxx <crash-utility-bounces@xxxxxxxxxx> on behalf of shogo.matsumoto@xxxxxxxxxxx <shogo.matsumoto@xxxxxxxxxxx>
> Sent: Thursday, December 16, 2021 16:39
> To: 'crash-utility@xxxxxxxxxx'
> Subject: [PATCH] log: output logs of printk safe buffers
>
> We sometimes overlook logs written to printk safe buffers
> (safe_print_seq/nmi_print_seq) which have not been flushed yet.
>
> This patch will output unflushed logs of the safe buffers
> at the bottom of log command output as follows:
>
> [nmi_print_seq] CPU: 0  BUFFER: ffff888063c18ac0  LEN: 28
> nmi print seq test message
> [safe_print_seq] CPU: 1  BUFFER: ffff888063d19ae0  LEN: 30
> safe print seq test message

Could you share how to test this patch?
such as how to create a memory dump where some messages are left
in each log buffers without being flushed.
I guess it would be helpful for reviewers.

>
> Note that the safe buffer (struct printk_safe_seq_buf) was introduced
> in kernel-4.11 and removed in kernel-5.15.

Describing the exact commit hashs in the kernel git repo are helpful.

>
> Signed-off-by: Shogo Matsumoto <shogo.matsumoto@xxxxxxxxxxx>
> ---
>  defs.h   |  3 +++
>  kernel.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
>
> diff --git a/defs.h b/defs.h
> index 7e2a16e..3ee51e0 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -2146,6 +2146,8 @@ struct offset_table {                    /* stash of commonly-used offsets */
>         long wait_queue_entry_private;
>         long wait_queue_head_head;
>         long wait_queue_entry_entry;
> +       long printk_safe_seq_buf_len;
> +       long printk_safe_seq_buf_buffer;
>  };
>
>  struct size_table {         /* stash of commonly-used sizes */
> @@ -2310,6 +2312,7 @@ struct size_table {         /* stash of commonly-used sizes */
>         long prb_desc;
>         long wait_queue_entry;
>         long task_struct_state;
> +       long printk_safe_seq_buf_buffer;
>  };

Could you add support for the new members to help -o?

help -o dumps contents of offset_table, size_table and array_table:

crash> help help

NAME
  help -get help
...snip...
    -n - dumpfile contents/statistics
    -o - offset_table and size_table
    -p - program_context
    -r - dump registers from dumpfile header
    -s - symbol table data
    -t - task_table
    -T - task_table plus context_array
    -v - vm_table
    -V - vm_table (verbose)
    -x - text cache
    -z - help options
crash> help -o | tail
              prio_array_queue: 0
            height_to_maxindex: 0
            height_to_maxnodes: 0
                      pid_hash: 0
               kmem_cache_node: 1024
           kmem_cache_cpu_slab: 0
           rt_prio_array_queue: 0
              task_struct_rlim: 0
            signal_struct_rlim: 0
                  vm_numa_stat: 0


>
>  struct array_table {
> diff --git a/kernel.c b/kernel.c
> index f4598ea..cc97176 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -93,6 +93,7 @@ static void source_tree_init(void);
>  static ulong dump_audit_skb_queue(ulong);
>  static ulong __dump_audit(char *);
>  static void dump_audit(void);
> +static void dump_printk_safe_seq_buf(void);
>  static char *vmcoreinfo_read_string(const char *);
>  static void check_vmcoreinfo(void);
>  static int is_pvops_xen(void);
> @@ -5048,6 +5049,7 @@ cmd_log(void)
>         }
>
>         dump_log(msg_flags);
> +       dump_printk_safe_seq_buf();
>  }
>
>
> @@ -11534,6 +11536,62 @@ dump_audit(void)
>                 error(INFO, "kernel audit log is empty\n");
>  }
>
> +static void
> +__dump_printk_safe_seq_buf(char *buf_name)
> +{
> +       int cpu, buffer_size;
> +       char *buffer;
> +
> +       if (!symbol_exists(buf_name)) {
> +               return;
> +       }
> +
> +       buffer_size = SIZE(printk_safe_seq_buf_buffer);
> +       buffer = GETBUF(buffer_size);
> +       for (cpu = 0; cpu < kt->cpus; cpu++) {
> +               ulong len_addr, buffer_addr;
> +               int len;
> +
> +               len_addr = symbol_value(buf_name) + kt->__per_cpu_offset[cpu] + OFFSET(printk_safe_seq_buf_len);
> +               buffer_addr = symbol_value(buf_name) + kt->__per_cpu_offset[cpu] + OFFSET(printk_safe_seq_buf_buffer);
> +               readmem(len_addr, KVADDR, &len, STRUCT_SIZE("atomic_t"), "printk_safe_seq_buf len", FAULT_ON_ERROR);
> +               readmem(buffer_addr, KVADDR, buffer, buffer_size, "printk_safe_seq_buf buffer", FAULT_ON_ERROR);
> +
> +               if (len > 0) {
> +                       int i, n;
> +                       char *p;
> +                       fprintf(fp, "[%s] CPU: %d  BUFFER: %lx  LEN: %d\n", buf_name, cpu, buffer_addr, len);
> +                       n = (len <= buffer_size) ? len : buffer_size;
> +                       for (i = 0, p = buffer; i < n; i++, p++) {
> +                               if (*p == 0x1) { //SOH
> +                                       i++; p++;
> +                                       continue;
> +                               } else {
> +                                       fputc(ascii(*p) ? *p : '.', fp);
> +                               }
> +                       }
> +                       fputc('\n', fp);
> +               }
> +       }
> +       FREEBUF(buffer);
> +}
> +
> +static void
> +dump_printk_safe_seq_buf(void)
> +{
> +       if (!STRUCT_EXISTS("printk_safe_seq_buf"))
> +               return;
> +
> +       if (INVALID_SIZE(printk_safe_seq_buf_buffer)) {
> +               MEMBER_OFFSET_INIT(printk_safe_seq_buf_len, "printk_safe_seq_buf", "len");
> +               MEMBER_OFFSET_INIT(printk_safe_seq_buf_buffer, "printk_safe_seq_buf", "buffer");
> +               MEMBER_SIZE_INIT(printk_safe_seq_buf_buffer, "printk_safe_seq_buf", "buffer");
> +       }
> +
> +       __dump_printk_safe_seq_buf("nmi_print_seq");
> +       __dump_printk_safe_seq_buf("safe_print_seq");
> +}
> +
>  /*
>   * Reads a string value from the VMCOREINFO data stored in (live) memory.
>   *





------------------------------

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility

End of Crash-utility Digest, Vol 195, Issue 24
**********************************************

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility

[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux