Re: [Crash-utility] [PATCH v2] Fix 'waitq' command for 4.13 and later kernels

-----Original Message----- > On Thu, Jun 24, 2021 at 12:51 PM HAGIO KAZUHITO(萩尾　一仁) > <k-hagio-ab(a)nec.com&gt; wrote: > > > > -----Original Message----- > > > > > } else { > > > > > - return; > > > > > + error(FATAL, "cannot determine wait queue structure\n"); > > > > > > > > oh, I should have checked the replacement.. this emits compilation warnings: > > > > > > > > $ make clean ; make warn > > > > ... > > > > cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 kernel.c -Wall -O2 -Wstrict-prototypes > -Wmissing-prototypes > > > -fstack-protector -Wformat-security > > > > kernel.c: In function 'cmd_waitq': > > > > kernel.c:9380:6: warning: 'start_index' may be used uninitialized in this function > > > [-Wmaybe-uninitialized] > > > > int start_index; /* where to start in wq array */ > > > > ^~~~~~~~~~~ > > > > kernel.c:9454:22: warning: 'task_offset' may be used uninitialized in this function > > > [-Wmaybe-uninitialized] > > > > readmem(wq_list[i] + task_offset, KVADDR, &task, > > > > ~~~~~~~~~~~^~~~~~~~~~~~~ > > > > kernel.c:9378:8: note: 'task_offset' was declared here > > > > ulong task_offset; /* offset of task in wq element */ > > > > ^~~~~~~~~~~ > > > > > > > > Hmm, in this case, I'd like to put the return back when merging. > > > > Lianbo, is this ok? > > > > > > > > } else { > > > > error(FATAL, "cannot determine wait queue structure\n"); > > > > > > After the log level is set to FATAL, it will exit, and the "return" > > > has no chance to execute . So it could be good > > > to use the log level "WARNING or INFO" for error()? > > > > I thought this is actually a fatal level error for the waitq command, > > so we should use FATAL normally, and put return with the comment to > > avoid confusion. It's a little rough but lesser effort. > > > > + return; /* just to suppress compilation warings */ > > > > And the outputs are below respectively, I think "WARNING" in a message > > implies that there is a problem but the command can continue somehow. > > I don't want to use it in this case. > > > > crash> waitq kauditd_wait > > waitq: cannot determine wait queue structure // INFO > > waitq: WARNING: cannot determine wait queue structure // WARNING > > waitq: cannot determine wait queue structure // FATAL > > > > So I would prefer FATAL. INFO is better? > > Maybe we should keep the original changes: > - return; > + error(FATAL, "cannot determine wait queue structure\n"); > > And fix the warnings directly in the kernel.c(dump_waitq()) as below: > > - ulong task_offset; /* offset of task in wq element */ > + ulong task_offset = 0; /* offset of task in wq element */ > int cnt; /* # elems on Queue */ > - int start_index; /* where to start in wq array */ > + int start_index = -1; /* where to start in wq array */ OK, I see. Let's go with this reasonable fix.

Thanks, Kazu