From: Daniel Walter <d.walter@0x90.at>
To: user-mode-linux-devel <user-mode-linux-devel@lists.sourceforge.net>
Cc: Richard Weinberger <richard@nod.at>, thomas@m3y3r.de
Subject: Re: [uml-devel] [PATCH] Dump really early stack traces
Date: Tue, 7 Oct 2014 10:28:24 +0200 (CEST) [thread overview]
Message-ID: <779241076.3024.1412670504362.JavaMail.root@0x90.at> (raw)
In-Reply-To: <1487140382.2997.1412670384789.JavaMail.root@0x90.at>
> <snip>
Hi Thomas,
Could you please fix your patch since there are multiple issues with it
1) I does not apply cleanly against the current linux tree
mostly due to kmsg_dump.c which is not in the proper format
it should be b/arch/um/kernel/kmsg_dump.c like the rest of the patch
2) After fixing this it did not build
Please make sure to build test your patch on a fresh checkout of the tree
by applying your patch, make ARCH=um mrproper, make ARCH=um defconfig make ARCH=um
3) The patch itself does not comply to patch standards.
please read Documentation/SubmittingPatches for more information
you can use scripts/checkpatch.pl to check for common errors
eg: Signed-off-by is missing, etc.
Now that we're done with the non-technical things, I don't like
a) that you move the inline function (current_thread_info) out of the
header file
b) introduce a branch into this function which is most likely never taken
(you could use if (unlikely(!is_kernel_addr((void *)ti)) )
c) I'd personally would make is_kernel_addr either an inline function or a macro
d) please split up the 2 changes in to different patches
1) return init_thread_info if not kernel_addr
2) add kmsg_dumper to panic_exit()
e) I'm not sure if returning init_thread_info is the right approach, it might
mask other errors like memory corruption bugs, etc.
I do like the kmsg_dumper thing, so would you mind sending this as a separate patch ?
Cheers,
Daniel
------------------------------------------------------------------------------
Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
parent reply other threads:[~2014-10-07 8:44 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <1487140382.2997.1412670384789.JavaMail.root@0x90.at>]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=779241076.3024.1412670504362.JavaMail.root@0x90.at \
--to=d.walter@0x90.at \
--cc=richard@nod.at \
--cc=thomas@m3y3r.de \
--cc=user-mode-linux-devel@lists.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).