Thanks for the feedback, Michael. I'll send a v2. Thanks,
Joe -----Original Message----- From: Michael Kelley <mikel...@microsoft.com> Sent: Friday, June 26, 2020 4:53 PM To: Joseph Salisbury <joseph.salisb...@microsoft.com>; KY Srinivasan <k...@microsoft.com>; Haiyang Zhang <haiya...@microsoft.com>; Stephen Hemminger <sthem...@microsoft.com>; sas...@kernel.org; wei....@kernel.org Cc: linux-hyp...@vger.kernel.org; linux-kernel@vger.kernel.org; sta...@vger.kernel.org Subject: RE: [PATCH] Drivers: hv: Change flag to write log level in panic msg to false From: Joseph Salisbury <joseph.salisb...@microsoft.com> Sent: Friday, June 26, 2020 10:48 AM > > When the kernel panics, one page worth of kmsg data is written to an > allocated page. The Hypervisor is notified of the page address trough > the MSR. This panic information is collected on the host. Since we > are only collecting one page of data, the full panic message may not be > collected. > > Each line of the panic message is prefixed with the log level of that > particular message in the form <N>, where N is the log level. The typical > 4 Kbytes contains anywhere from 50 to 100 lines with that log level prefix. > > hv_dmsg_dump() makes a call to kmsg_dump_get_buffer(). The second > argument in the call is a bool described as: ‘@syslog: include the “<4>” > Prefixes’. > > With this change, we will not write the log level to the allocated > page. This will provide additional room in the allocated page for > more informative panic information. Let me suggest tightening the commit message a bit, with focus on the "what" and "why" rather than the details of the code change. Also use imperative voice per the Linux kernel guidelines: When the kernel panics, one page of kmsg data may be collected and sent to Hyper-V to aid in diagnosing the failure. The collected kmsg data typically contains 50 to 100 lines, each of which has a log level prefix that isn't very useful from a diagnostic standpoint. So tell kmsg_dump_get_buffer() to not include the log level, enabling more information that *is* useful to fit in the page. > > Requesting in stable kernels, since many kernels running in production > are stable releases. > > Cc: sta...@vger.kernel.org > Signed-off-by: Joseph Salisbury <joseph.salisb...@microsoft.com> > --- > drivers/hv/vmbus_drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index > 9147ee9d5f7d..d69f4efa3719 100644 > --- a/drivers/hv/vmbus_drv.c > +++ b/drivers/hv/vmbus_drv.c > @@ -1368,7 +1368,7 @@ 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, true, hv_panic_page, HV_HYP_PAGE_SIZE, > + kmsg_dump_get_buffer(dumper, false, hv_panic_page, HV_HYP_PAGE_SIZE, > &bytes_written); > if (bytes_written) > hyperv_report_panic_msg(panic_pa, bytes_written); > -- > 2.17.1 With the commit message changes, Reviewed-by: Michael Kelley <mikel...@microsoft.com>
