Message-ID: <20130701202959.178a1697@holzheu>
Date:	Mon, 1 Jul 2013 20:29:59 +0200
From:	Michael Holzheu <holzheu@...ux.vnet.ibm.com>
To:	Vivek Goyal <vgoyal@...hat.com>
Cc:	HATAYAMA Daisuke <d.hatayama@...fujitsu.com>,
	Jan Willeke <willeke@...ibm.com>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	linux-kernel@...r.kernel.org, kexec@...ts.infradead.org
Subject: Re: [PATCH v5 1/5] vmcore: Introduce ELF header in new memory
 feature

On Mon, 1 Jul 2013 13:37:27 -0400
Vivek Goyal <vgoyal@...hat.com> wrote:

> On Fri, Jun 28, 2013 at 10:15:52AM +0200, Michael Holzheu wrote:
> > On Thu, 27 Jun 2013 16:23:34 -0400
> > Vivek Goyal <vgoyal@...hat.com> wrote:
> > 
> > > On Thu, Jun 27, 2013 at 03:32:02PM -0400, Vivek Goyal wrote:
> > > > On Fri, Jun 21, 2013 at 04:17:03PM +0200, Michael Holzheu wrote:
> > > > > On Fri, 14 Jun 2013 14:54:02 -0400
> > > > > Vivek Goyal <vgoyal@...hat.com> wrote:
> > 
> > [snip]
> > 
> > > Thinking more about it, I think let us cleanup with this little ugly
> > > bit too so that future changes become easy.
> > > 
> > > Current convention is that elfcorehdr_addr and elfcorehdr_size are
> > > already set by arch code by the time vmcore.c starts reading it. Can't
> > > s390 allocate elf headers in early boot code and elfcorehdr_addr? Then
> > > we don't have to call elfcorehdr_alloc().
> > > 
> > > And once we are done with reading headers, we can call elfcorehdr_free()
> > > and s390 could free memory and set elfcorehdr_addr to ELFCORE_ADDR_ERR
> > > and elfcorehdr_size=0. That would signify that one can not try to read
> > > elf headers now and it must have been freed.
> > > 
> > > is_kdump_kernel() will continue to work as elfcorehdr_addr is
> > > ELFCORE_ADDR_ERR. And that will mean that either elfcorehdr were not
> > > readable/usable to begin with or they have been freed now.
> > 
> > Hello Vivek,
> > 
> > We would like to keep the alloc/free symmetry as you have suggested in a
> > previous mail. This also has the advantage that we do not have to rely
> > on the ordering of init calls.
> > 
> > Wouldn't it be sufficient to just set elfcorehdr_addr to ELFCORE_ADDR_ERR
> > after elfcorehdr_free() and remove the comment?
> > 
> 
> Hi Michael,
> 
> This has only one problem and that is what's the initialization semantics
> of elfcorehdr_addr. 
> 
> So far we expected it to be initialized very early in boot process and
> once this is set, any component in the system could figure out if it
> is kdump kernel (is_kdump_kernel()) and do kdump specific things.
> 
> But if we move initialization of this variable little late, then it
> might be a problem for early users of is_kdump_kernel(). Though right
> now I don't see drivers making use of it and only arch specific early
> boot up code seems to have it.
> 
> So either we can stick to existing semantics of initializing headers
> early or we could create a separate variable for is_kdump_kernel() which
> is set in early boot and then one can delay initialization of
> elfcorehdr_addr() in vmcore_init().

The later makes much sense to me. This would also make the s390 code
easier to read since we then could exchange some early kdump checks
with the official is_kdump() function.

Perhaps we can do that as an additional patch after we are ready with
this patch series.

> 
> Given the fact that I don't see any users of is_kdump_kernel() in arch
> independent directory, and I am assuming that you will tackle all early
> users of is_kdump_kernel() in s390, I will be fine even with your patch
> below.

Ok great, so I will send you the current patch set version 6 with all the
discussed changes.

Thanks!
Michael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists