<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=gb2312"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Hi Lianbo</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <span style="background-color:rgb(255, 255, 255);display:inline !important">Please help to review t</span>he new patch v7 with only one change for removing the redundant code.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Patch is patch, what's the better way attach it into an email? Copy & paste would not applicable for a large patch file.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> I have no vmcore file, but there is a kernel module which would help to trigger an overflow stack panic for testing, please download the module form link [1] and compile it as a module to load it into your test box, please read the README.txt and the source code for more details.</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> [1] <a href="https://github.com/yangh/crash/tree/pso-kernel-module/kernel-modules/panic-stack-overflow" id="LPNoLPOWALinkPreview">https://github.com/yangh/crash/tree/pso-kernel-module/kernel-modules/panic-stack-overflow</a></div> <div class="_Entity _EType_OWALinkPreview _EId_OWALinkPreview _EReadonly_1"></div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Best regards</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Hong</div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Hong YANG3 杨红 <hong.yang3@nio.com><br> <b>Sent:</b> Monday, November 29, 2021 11:40<br> <b>To:</b> lijiang <lijiang@redhat.com>; Discussion list for crash utility usage, maintenance and development <crash-utility@redhat.com><br> <b>Subject:</b> Re: arm64: Support overflow stack panic</font> <div> </div> </div> <style type="text/css" style="display:none">  </style> <div dir="ltr"> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)"> Hi Lianbo</div> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)"> <br> </div> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)"> I'm using outlook to send mail to this list, I'll try to find a better way to send out patch and mails more friendly for all reader, .</div> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)"> <br> </div> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)"> I'll send out a demo kernel module which can trigger an overflow panic for testing, and also the patch will be updated as your comment in previous mail.</div> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)"> <br> </div> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)"> Thanks for your quickly reply.</div> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)"> <br> </div> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)"> Best regards</div> <div style="font-family:Calibri,Arial,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)"> Hong</div> <div id="x_appendonsend"></div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> lijiang <lijiang@redhat.com><br> <b>Sent:</b> Monday, November 29, 2021 10:58<br> <b>To:</b> Hong YANG3 杨红 <hong.yang3@nio.com>; Discussion list for crash utility usage, maintenance and development <crash-utility@redhat.com><br> <b>Subject:</b> Re: arm64: Support overflow stack panic</font> <div> </div> </div> <div> <div style=""> <div><span style="font-size:11pt; font-family:'Cambria','times new roman','garamond',serif; color:#00bebe"><b>注意：此封邮件来自于公司外部，请注意信息安全！ </b></span><br> <span style="font-size:11pt; font-family:'Cambria','times new roman','garamond',serif; color:#00bebe"><b>Attention: This email comes from outside of the company, please pay attention to the information security! </b></span><br> </div> </div> <div> <div dir="ltr">Hi, Hong <div><br> </div> <div>Thank you for the patch. I added the comments below, other changes look good to me.</div> <div><br> </div> <div>@@ -1978,7 +2028,10 @@ arm64_in_exception_text(ulong ptr)<br> if ((ptr >= ms->__exception_text_start) &&<br> (ptr < ms->__exception_text_end))<br> return TRUE;<br> - } else if ((name = closest_symbol(ptr))) { /* Linux 5.5 and later */<br> + }<br> +<br> + name = closest_symbol(ptr);<br> + if (name != NULL) { /* Linux 5.5 and later */</div> <div><br> </div> <div>The above changes are irrelevant to your patch itself. But anyway this looks more readable to me.</div> <div><br> for (func = &arm64_exception_functions[0]; *func; func++) {<br> if (STREQ(name, *func))<br> return TRUE;<br> @@ -2255,12 +2308,14 @@ arm64_unwind_frame(struct bt_info *bt, struct arm64_stackframe *frame)<br> if (!(machdep->flags & IRQ_STACKS))<br> return TRUE;<br> <br> - if (!(machdep->flags & IRQ_STACKS))<br> + if (!(machdep->flags & OVERFLOW_STACKS))<br> return TRUE;<br> </div> <div><br> </div> <div>Originally, it had two same(repeated) statements, one of which must be redundant. This time, can it be changed to a statement as below?</div> <div><br> </div> <div>if (!(machdep->flags & (IRQ_STACKS | OVERFLOW_STACKS)))</div> <div> return TRUE;</div> <div><br> </div> <div>BTW: this patch was sent as an attachment, which is inconvenient for other reviewers to add comments.</div> <div><br> </div> <div>In addition, I have a request: can you share the vmcore with me if it doesn't have confidential data? I'm collecting the specific vmcore</div> <div>for the test, at least I haven't reproduced it.</div> <div><br> </div> <div>Thanks.</div> <div>Lianbo</div> </div> </div> </div> </div> </body> </html>