All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>
To: Mark Brown <broonie@kernel.org>
Cc: mark.rutland@arm.com, jpoimboe@redhat.com, jthierry@redhat.com,
	catalin.marinas@arm.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1 1/4] arm64: Implement infrastructure for stack trace reliability checks
Date: Thu, 1 Apr 2021 12:44:53 -0500	[thread overview]
Message-ID: <ecdca67a-223f-40de-ebfa-89183e15a2a8@linux.microsoft.com> (raw)
In-Reply-To: <20210401152741.GK4758@sirena.org.uk>



On 4/1/21 10:27 AM, Mark Brown wrote:
> On Tue, Mar 30, 2021 at 02:09:52PM -0500, madvenka@linux.microsoft.com wrote:
>> From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>
>>
>> Implement a check_reliability() function that will contain checks for the
>> presence of various features and conditions that can render the stack trace
>> unreliable.
> 
> This looks good to me with one minor stylistic thing:
> 
>> +/*
>> + * Special functions where the stack trace is unreliable.
>> + */
>> +static struct function_range	special_functions[] = {
>> +	{ 0, 0 }
>> +};
> 
> Might be good to put a comment here saying that this is terminating the
> list rather than detecting a NULL function pointer:
> 
> 	{ /* sentinel */ }
> 
> is a common idiom for that.
> 
> Given that it's a fixed array we could also...
> 
>> +	for (func = special_functions; func->start; func++) {
>> +		if (pc >= func->start && pc < func->end)
> 
> ...do these as
> 
> 	for (i = 0; i < ARRAY_SIZE(special_functions); i++) 
> 
> so you don't need something like that, though that gets awkward when you
> have to write out special_functions[i].field a lot.
> 
> So many different potential colours for the bikeshed!
I will make the above changes.

Thanks!

Madhavan

WARNING: multiple messages have this Message-ID (diff)
From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>
To: Mark Brown <broonie@kernel.org>
Cc: mark.rutland@arm.com, jpoimboe@redhat.com, jthierry@redhat.com,
	catalin.marinas@arm.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1 1/4] arm64: Implement infrastructure for stack trace reliability checks
Date: Thu, 1 Apr 2021 12:44:53 -0500	[thread overview]
Message-ID: <ecdca67a-223f-40de-ebfa-89183e15a2a8@linux.microsoft.com> (raw)
In-Reply-To: <20210401152741.GK4758@sirena.org.uk>



On 4/1/21 10:27 AM, Mark Brown wrote:
> On Tue, Mar 30, 2021 at 02:09:52PM -0500, madvenka@linux.microsoft.com wrote:
>> From: "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>
>>
>> Implement a check_reliability() function that will contain checks for the
>> presence of various features and conditions that can render the stack trace
>> unreliable.
> 
> This looks good to me with one minor stylistic thing:
> 
>> +/*
>> + * Special functions where the stack trace is unreliable.
>> + */
>> +static struct function_range	special_functions[] = {
>> +	{ 0, 0 }
>> +};
> 
> Might be good to put a comment here saying that this is terminating the
> list rather than detecting a NULL function pointer:
> 
> 	{ /* sentinel */ }
> 
> is a common idiom for that.
> 
> Given that it's a fixed array we could also...
> 
>> +	for (func = special_functions; func->start; func++) {
>> +		if (pc >= func->start && pc < func->end)
> 
> ...do these as
> 
> 	for (i = 0; i < ARRAY_SIZE(special_functions); i++) 
> 
> so you don't need something like that, though that gets awkward when you
> have to write out special_functions[i].field a lot.
> 
> So many different potential colours for the bikeshed!
I will make the above changes.

Thanks!

Madhavan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-04-01 18:06 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <77bd5edeea72d44533c769b1e8c0fea7a9d7eb3a>
2021-03-30 19:09 ` [RFC PATCH v1 0/4] arm64: Implement stack trace reliability checks madvenka
2021-03-30 19:09   ` madvenka
2021-03-30 19:09   ` [RFC PATCH v1 1/4] arm64: Implement infrastructure for " madvenka
2021-03-30 19:09     ` madvenka
2021-04-01 15:27     ` Mark Brown
2021-04-01 15:27       ` Mark Brown
2021-04-01 17:44       ` Madhavan T. Venkataraman [this message]
2021-04-01 17:44         ` Madhavan T. Venkataraman
2021-03-30 19:09   ` [RFC PATCH v1 2/4] arm64: Mark a stack trace unreliable if an EL1 exception frame is detected madvenka
2021-03-30 19:09     ` madvenka
2021-04-01 17:21     ` Mark Brown
2021-04-01 17:21       ` Mark Brown
2021-03-30 19:09   ` [RFC PATCH v1 3/4] arm64: Detect FTRACE cases that make the stack trace unreliable madvenka
2021-03-30 19:09     ` madvenka
2021-04-01 14:27     ` Mark Brown
2021-04-01 14:27       ` Mark Brown
2021-04-01 17:43       ` Madhavan T. Venkataraman
2021-04-01 17:43         ` Madhavan T. Venkataraman
2021-04-01 18:28         ` Mark Brown
2021-04-01 18:28           ` Mark Brown
2021-04-01 18:40           ` Madhavan T. Venkataraman
2021-04-01 18:40             ` Madhavan T. Venkataraman
2021-04-01 18:53             ` Madhavan T. Venkataraman
2021-04-01 18:53               ` Madhavan T. Venkataraman
2021-04-01 19:47               ` Madhavan T. Venkataraman
2021-04-01 19:47                 ` Madhavan T. Venkataraman
2021-04-06 11:02                 ` Mark Brown
2021-04-06 11:02                   ` Mark Brown
2021-04-01 17:48       ` Madhavan T. Venkataraman
2021-04-01 17:48         ` Madhavan T. Venkataraman
2021-03-30 19:09   ` [RFC PATCH v1 4/4] arm64: Mark stack trace as unreliable if kretprobed functions are present madvenka
2021-03-30 19:09     ` madvenka
2021-04-01 17:23     ` Mark Brown
2021-04-01 17:23       ` Mark Brown
2021-04-03 17:01   ` [RFC PATCH v1 0/4] arm64: Implement stack trace reliability checks Josh Poimboeuf
2021-04-03 17:01     ` Josh Poimboeuf
2021-04-04  3:29     ` Madhavan T. Venkataraman
2021-04-04  3:29       ` Madhavan T. Venkataraman
2021-04-05 13:24       ` Masami Hiramatsu
2021-04-05 13:24         ` Masami Hiramatsu
2021-04-05 13:46         ` Madhavan T. Venkataraman
2021-04-05 13:46           ` Madhavan T. Venkataraman
2021-04-05 14:56         ` Madhavan T. Venkataraman
2021-04-05 14:56           ` Madhavan T. Venkataraman
2021-04-05 17:12           ` Madhavan T. Venkataraman
2021-04-05 17:12             ` Madhavan T. Venkataraman
2021-04-05 23:39             ` Masami Hiramatsu
2021-04-05 23:39               ` Masami Hiramatsu
2021-04-05 23:40           ` Masami Hiramatsu
2021-04-05 23:40             ` Masami Hiramatsu

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=ecdca67a-223f-40de-ebfa-89183e15a2a8@linux.microsoft.com \
    --to=madvenka@linux.microsoft.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=jpoimboe@redhat.com \
    --cc=jthierry@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=will@kernel.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.