From: Heiko Carstens <firstname.lastname@example.org> To: Miroslav Benes <email@example.com> Cc: firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com Subject: Re: [PATCH v2 0/3] s390/livepatch: Implement reliable stack tracing for the consistency model Date: Thu, 31 Oct 2019 16:24:49 +0100 Message-ID: <20191031152449.GA6133@osiris> (raw) In-Reply-To: <alpine.LSU.firstname.lastname@example.org> On Wed, Oct 30, 2019 at 11:12:00AM +0100, Miroslav Benes wrote: > On Tue, 29 Oct 2019, Heiko Carstens wrote: > > > On Tue, Oct 29, 2019 at 03:39:01PM +0100, Miroslav Benes wrote: > > > - I tried to use the existing infrastructure as much as possible with > > > one exception. I kept unwind_next_frame_reliable() next to the > > > ordinary unwind_next_frame(). I did not come up with a nice solution > > > how to integrate it. The reliable unwinding is executed on a task > > > stack only, which leads to a nice simplification. My integration > > > attempts only obfuscated the existing unwind_next_frame() which is > > > already not easy to read. Ideas are definitely welcome. > > > > Ah, now I see. So patch 2 seems to be leftover(?). Could you just send > > how the result would look like? > > > > I'd really like to have only one function, since some of the sanity > > checks you added also make sense for what we already have - so code > > would diverge from the beginning. > > Ok, that is understandable. I tried a bit harder and the outcome does not > look as bad as my previous attempts (read, I gave up too early). > > I deliberately split unwind_reliable/!unwind_reliable case in "No > back-chain, look for a pt_regs structure" branch, because the purpose is > different there. In !unwind_reliable case we can continue on a different > stack (if I understood the code correctly when I analyzed it in the past. > I haven't found a good documentation unfortunately :(). While in > unwind_realiable case we just check if there are pt_regs in the right > place on a task stack and stop. If there are not, error out. > > It applies on top of the patch set. Only compile tested though. If it > looks ok-ish to you, I'll work on it. Yes, that looks much better. Note, from a coding style perspective the 80 characters per line limit is _not_ enforced on s390 kernel code; so that might be a possibility to make the code a bit more readable. Also it _might_ make sense to split the function into two or more functions (without duplicating code). Not sure if that would really increase readability though. FWIW, I just applied your first patch, since it makes sense anyway.
prev parent reply index Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-10-29 14:39 Miroslav Benes 2019-10-29 14:39 ` [PATCH v2 1/3] s390/unwind: drop unnecessary code around calling ftrace_graph_ret_addr() Miroslav Benes 2019-10-29 14:39 ` [PATCH v2 2/3] s390/unwind: prepare the unwinding interface for reliable stack traces Miroslav Benes 2019-10-29 14:39 ` [PATCH v2 3/3] s390/livepatch: Implement reliable stack tracing for the consistency model Miroslav Benes 2019-10-29 16:17 ` Heiko Carstens 2019-10-30 10:05 ` Miroslav Benes 2019-10-29 16:34 ` [PATCH v2 0/3] " Heiko Carstens 2019-10-30 10:12 ` Miroslav Benes 2019-10-31 15:24 ` Heiko Carstens [this message]
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=20191031152449.GA6133@osiris \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /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
Live-Patching Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/live-patching/0 live-patching/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 live-patching live-patching/ https://lore.kernel.org/live-patching \ email@example.com public-inbox-index live-patching Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.live-patching AGPL code for this site: git clone https://public-inbox.org/public-inbox.git