[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 0/6] Remove dependency on __LINE__



On 03/10/2017 08:50 AM, Jan Beulich wrote:
On 10.03.17 at 09:29, <ross.lagerwall@xxxxxxxxxx> wrote:
On 03/09/2017 10:34 AM, Jan Beulich wrote:
On 08.03.17 at 18:46, <ross.lagerwall@xxxxxxxxxx> wrote:
Sorry for the long delay since the first version of this series
(previously called "Make building xSplice patches easier").  Here is a
set of changes that remove the use of __LINE__ when building with NDEBUG
and LivePatch enabled.  Tested to boot on x86.  Compile-tested on arm.

What I'm missing here is an evaluation of possible alternatives, as
converting from __LINE__ to code addresses implies an extra step
when wanting to look up the place where a problem occurred,
which I generally consider undesirable. For example, I had briefly
discussed with Andrew the addition of #line directives, but iirc he
had indicated some issue with that. Furthermore there is the
option (at least for some of the more simple patches) of squeezing
more than one statement on a single line, perhaps just for patch
creation. And I guess one can think of more.


Both of the options you suggested require extra work when building a
live patch. This series is intended to remove that extra work as the
process is already more difficult than I'd like.

I don't think using addr2line is much harder than looking up the line
number directly, especially as it is something that needs to be done for
many other crashes.

I agree with the second half, but I don't think I do for the first part:
I don't know how you would manage this, but for me this is several
steps:
- Locate and download the correct package (it's not reasonable to
  keep them all, there simply are too many)
- extract xen-syms or xen.efi (and of course I will need to know
  which one was actually used)
- run addr2line on it
Quite a bit more than just going to the source file and finding the
line right away - in most cases line numbers, as opposed to code
addresses, don't change much despite a delta of a few dozen
backports or other patches, so normally one doesn't even need
to look at the _precise_ tree.

On your suggestion, use of __LINE__ is disabled only
when CONFIG_LIVEPATCH=y and NDEBUG which means that in a typical
development scenario, you'd still get line numbers.

That's understood, but I'm concerned about extra overhead when
looking at customer reports.

They would be
removed only for "release" builds in which it is likely that the source
code & debuginfo is archived somewhere such that looking up a line
number requires several steps anyway. I could suggest making it a
separate config option but IIRC you prefer to limit the number of config
options.

I prefer to limit ones with non-EXPERT-visible prompts, yes. But
I also don't see how a separate config option would improve the
situation.


My suggestion was to have a separate config option so that the person configuring Xen could choose in the trade off between having line numbers in debug messages or easier live patching. Does that seem reasonable?

--
Ross Lagerwall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.