On Mon, 1 Jul 2013 13:37:27 -0400 Vivek Goyal <vgoyal at redhat.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 at redhat.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 at redhat.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
