On Fri, May 27, 2016 at 09:46:10AM +0530, Pratyush Anand wrote:
> On 27/05/2016:12:18:56 PM, AKASHI Takahiro wrote:
> > On Thu, May 26, 2016 at 01:27:08PM +0530, Pratyush Anand wrote:
> > > On 26/05/2016:04:04:08 PM, AKASHI Takahiro wrote:
> > > > Pratyush,
> > > >
> > > > Just for debug purpose.
> > > > Please add the following line to *your* arch_crash_save_vmcoreinfo():
> > > > > vmcoreinfo_append_str("NUMBER(kimage_voffset)=%llx\n", kimage_voffset);
> > >
> > > Thanks for the pointer.
> > >
> > > I did had VMCOREINFO_NUMBER(kimage_voffset) in arch_crash_save_vmcoreinfo().
> > >
> > > https://github.com/pratyushanand/linux/commit/7011e478aae3e568cc8dca15b6c128fe728416f7#diff-cdf29c3b9471b9d813afc107dd154acdR291
> > >
> > > But, I noticed that in crash code you have "ms->kimage_voffset = htol(string,
> > > QUIET, NULL);". So, the change you have suggested will work.
> >
> > I know that. It is intentional.
> >
> > > However, I think its preferable to use VMCOREINFO_NUMBER() macro instead.
> > > makedumpfile is able to calculate kimage_voffset correctly with that without any
> > > issue.
> >
> > I think that VMCOREINFO_NUMBER() is, at least originally, intended
> > to be used for a small *unsigned* integer.
>
> Not sure..When I see all the places where it has been used, it seems it can
> accommodate *long* variable.
I don't know makedumpfile side, but as far as I look at the kernel code
in crash_save_vmcoreinfo_init(), all the values seem to be small positive
numbers.
> See, read_vmcoreinfo_long() of makedumpfile [1] , which is being used to read
> "NUMBER" string. This function uses strtol(). So I think, we must have similar
> implementation if we intend to use VMCOREINFO_NUMBER().
>
> [1] https://sourceforge.net/p/makedumpfile/code/ci/master/tree/makedumpfile.c#l2400
>
> >
> > I also know that PHYS_OFFSET can now be nagative in v4.6 on arm64.
> > Yet I'm thinking of adding "0x" as a prefix to VMCOREINOF string.
>
> I think, we should not change existing VMCOREINFO_xxxx() macros of kernel,
> because it will break the applications which rely on it's current structure.
I'm not saying that we change VMCOREINFO_NUMBER() macro.
> ATM, I also do not see any urgency of defining a new macro which can have
> implementation like vmcoreinfo_append_str("NUMBER(X)=%#llx\n", X), because
> exiting macros is able to take care of the needs.
>
> >
> > -Takahiro AKASHI
> >
> > > I will suggest to take following modification in crash code:
> > >
> > > diff --git a/arm64.c b/arm64.c
> > > index 6b97093..9397d6d 100644
> > > --- a/arm64.c
> > > +++ b/arm64.c
> > > @@ -122,7 +122,7 @@ arm64_init(int when)
> > > ms = machdep->machspec;
> > > if (!ms->kimage_voffset &&
> > > (string = pc->read_vmcoreinfo("NUMBER(kimage_voffset)"))) {
> > > - ms->kimage_voffset = htol(string, QUIET, NULL);
> > > + ms->kimage_voffset = dtol(string, QUIET, NULL);
> > > free(string);
> > > }
> > >
> > > diff --git a/tools.c b/tools.c
> > > index 384bebd..1383e43 100644
> > > --- a/tools.c
> > > +++ b/tools.c
> > > @@ -877,7 +877,7 @@ dtol(char *s, int flags, int *errptr)
> > > s++;
> > >
> > > for (j = 0; s[j] != '\0'; j++)
> > > - if ((s[j] < '0' || s[j] > '9'))
> > > + if ( (s[j] != '-') && ((s[j] < '0' || s[j] > '9')))
> > > break ;
> > >
> > > if (s[j] != '\0') {
> > >
>
> ~Pratyush
--
Thanks,
-Takahiro AKASHI
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
