lkml.org 
[lkml]   [2012]   [Oct]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 01/11] kexec: introduce kexec_ops struct
    On Fri, Sep 28, 2012 at 12:07:42PM -0400, Konrad Rzeszutek Wilk wrote:
    > On Thu, Sep 27, 2012 at 08:06:28PM +0200, Daniel Kiper wrote:
    > > Some kexec/kdump implementations (e.g. Xen PVOPS) on different archs could
    > > not use default functions or require some changes in behavior of kexec/kdump
    > > generic code. To cope with that problem kexec_ops struct was introduced.
    > > It allows a developer to replace all or some functions and control some
    > > functionality of kexec/kdump generic code.
    > >
    > > Default behavior of kexec/kdump generic code is not changed.
    > >
    > > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
    > > ---
    > > include/linux/kexec.h | 18 +++++++
    > > kernel/kexec.c | 125 ++++++++++++++++++++++++++++++++++++-------------
    > > 2 files changed, 111 insertions(+), 32 deletions(-)
    > >
    > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
    > > index 37c5f72..beb08ca 100644
    > > --- a/include/linux/kexec.h
    > > +++ b/include/linux/kexec.h
    > > @@ -165,7 +165,25 @@ struct kimage {
    > > #endif
    > > };
    > >
    > > +struct kexec_ops {
    > > + bool always_use_normal_alloc;
    >
    > So most of these are self-explanatory. But the bool is not that clear
    > to me. Could you include a documentation comment explaining
    > its purpose and its implications?

    OK.

    > > + struct page *(*kimage_alloc_pages)(gfp_t gfp_mask,
    > > + unsigned int order,
    > > + unsigned long limit);
    > > + void (*kimage_free_pages)(struct page *page);
    > > + unsigned long (*page_to_pfn)(struct page *page);
    > > + struct page *(*pfn_to_page)(unsigned long pfn);
    > > + unsigned long (*virt_to_phys)(volatile void *address);
    > > + void *(*phys_to_virt)(unsigned long address);
    > > + int (*machine_kexec_prepare)(struct kimage *image);
    > > + int (*machine_kexec_load)(struct kimage *image);
    > > + void (*machine_kexec_cleanup)(struct kimage *image);
    > > + void (*machine_kexec_unload)(struct kimage *image);
    > > + void (*machine_kexec_shutdown)(void);
    > > + void (*machine_kexec)(struct kimage *image);
    > > +};
    > >
    > > +extern struct kexec_ops kexec_ops;
    >
    > Is this neccessary?

    Yes, because it is used by Xen machine_kexec_??.c files.

    > > /* kexec interface functions */
    > > extern void machine_kexec(struct kimage *image);
    > > diff --git a/kernel/kexec.c b/kernel/kexec.c
    > > index 0668d58..98556f3 100644
    > > --- a/kernel/kexec.c
    > > +++ b/kernel/kexec.c
    > > @@ -56,6 +56,47 @@ struct resource crashk_res = {
    > > .flags = IORESOURCE_BUSY | IORESOURCE_MEM
    > > };
    > >
    > > +static struct page *kimage_alloc_pages(gfp_t gfp_mask,
    > > + unsigned int order,
    > > + unsigned long limit);
    > > +static void kimage_free_pages(struct page *page);
    > > +
    > > +static unsigned long generic_page_to_pfn(struct page *page)
    > > +{
    > > + return page_to_pfn(page);
    > > +}
    > > +
    > > +static struct page *generic_pfn_to_page(unsigned long pfn)
    > > +{
    > > + return pfn_to_page(pfn);
    > > +}
    > > +
    > > +static unsigned long generic_virt_to_phys(volatile void *address)
    > > +{
    > > + return virt_to_phys(address);
    > > +}
    > > +
    > > +static void *generic_phys_to_virt(unsigned long address)
    > > +{
    > > + return phys_to_virt(address);
    > > +}
    > > +
    > > +struct kexec_ops kexec_ops = {
    > > + .always_use_normal_alloc = false,
    > > + .kimage_alloc_pages = kimage_alloc_pages,
    > > + .kimage_free_pages = kimage_free_pages,
    > > + .page_to_pfn = generic_page_to_pfn,
    > > + .pfn_to_page = generic_pfn_to_page,
    > > + .virt_to_phys = generic_virt_to_phys,
    > > + .phys_to_virt = generic_phys_to_virt,
    > > + .machine_kexec_prepare = machine_kexec_prepare,
    > > + .machine_kexec_load = NULL,
    >
    > Instead of NULL should they just point to some nop function?

    OK.

    Daniel


    \
     
     \ /
      Last update: 2012-10-01 17:21    [W:4.083 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site