LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Dave Hansen <haveblue@us.ibm.com>
To: Jan-Bernd Themann <ossthema@de.ibm.com>
Cc: linuxppc-dev@ozlabs.org, Thomas Klein <tklein@de.ibm.com>,
	"Themann, Jan-Bernd" <themann@de.ibm.com>,
	netdev <netdev@vger.kernel.org>, apw <apw@uk.ibm.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Thomas Klein <osstklei@de.ibm.com>,
	Christoph Raisch <RAISCH@de.ibm.com>,
	Badari Pulavarty <pbadari@us.ibm.com>, Greg KH <greg@kroah.com>
Subject: Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier
Date: Wed, 13 Feb 2008 09:05:00 -0800	[thread overview]
Message-ID: <1202922300.25500.37.camel@nimitz.home.sr71.net> (raw)
In-Reply-To: <200802131617.58646.ossthema@de.ibm.com>

On Wed, 2008-02-13 at 16:17 +0100, Jan-Bernd Themann wrote:
> Constraints imposed by HW / FW:
> - eHEA has own MMU
> - eHEA  Memory Regions (MRs) are used by the eHEA MMU  to translate virtual
>   addresses to absolute addresses (like DMA mapped memory on a PCI bus)
> - The number of MRs is limited (not enough to have one MR per packet)

Are there enough to have one per 16MB section?

> Our current understanding about the current Memory Hotplug System are
> (please correct me if I'm wrong):
> 
> - depends on sparse mem

You're wrong ;).  In mainline, this is true.  There was a version of the
SUSE kernel that did otherwise.  But you can not and should not depend
on this never changing.  But, someone is perfectly free to go out an
implement something better than sparsemem for memory hotplug.  If they
go and do this, your driver may get left behind. 

> - only whole memory sections are added / removed
> - for each section a memory resource is registered

True, and true. (There might be exceptions to the whole sections one,
but that's blatant abuse and should be fixed. :)

> From the driver side we need:
> - some kind of memory notification mechanism.
>   For memory add we can live without any external memory notification
>   event. For memory remove we do need an external trigger (see explanation
>   above).

You can export and use (un)register_memory_notifier.  You just need to
do it in a reasonable way that compiles for randconfig on your
architecture.  Believe me, we don't want to start teaching drivers about
sparsemem.  

> - a way to iterate over all kernel pages and a way to detect holes in the
>   kernel memory layout in order to build up our own ehea_bmap.

Look at kernel/resource.c

But, I'm really not convinced that you can actually keep this map
yourselves.  It's not as simple as you think.  What happens if you get
on an LPAR with two sections, one 256MB@0x0 and another
16MB@0x1000000000000000.  That's quite possible.  I think your vmalloc'd
array will eat all of memory.  

That's why we have SPARSEMEM_EXTREME and SPARSEMEM_VMEMMAP implemented
in the core, so that we can deal with these kinds of problems, once and
*NOT* in every single little driver out there.  

> Functions to use while building ehea_bmap + MRs:
> - Use either the functions that are used by the memory hotplug system as
>   well, that means using the section defines + functions (section_nr_to_pfn,
>   pfn_valid)

Basically, you can't use anything related to sections outside of the
core code.  You can use things like pfn_valid(), or you can create new
interfaces that are properly abstracted.  

> - Use currently other not exported functions in kernel/resource.c, like
>   walk_memory_resource (where we would still need the maximum possible number
>   of pages NR_MEM_SECTIONS)

It isn't the act of exporting that's the problem.  It's making sure that
the exports won't be prone to abuse and that people are using them
properly.  You should assume that you can export and use
walk_memory_resource().

Do you know what other operating systems do with this hardware?

In the future, please make an effort to get review from knowledgeable
people about these kinds of things before using them in your driver.
Your company has many, many resources available, and all you need to do
is ask.  All that you have to do is look to the tops of the files of the
functions you are calling.

-- Dave


  reply	other threads:[~2008-02-13 17:06 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-11 16:24 Jan-Bernd Themann
2008-02-11 16:47 ` Dave Hansen
2008-02-13 15:17   ` Jan-Bernd Themann
2008-02-13 17:05     ` Dave Hansen [this message]
2008-02-14  8:46     ` Christoph Raisch
2008-02-14 17:12       ` Dave Hansen
2008-02-14 17:36         ` Badari Pulavarty
2008-02-14 17:38           ` Dave Hansen
2008-02-15 13:22         ` Christoph Raisch
2008-02-15 16:55           ` Dave Hansen
2008-02-18 10:00             ` Jan-Bernd Themann
2008-02-20 18:14               ` Dave Hansen
2008-02-11 16:50 ` Dave Hansen
2008-02-12 18:04 ` Dave Hansen
  -- strict thread matches above, loose matches on Subject: below --
2008-02-11 15:57 Jan-Bernd Themann
2008-02-11 16:02 ` Sam Ravnborg
2008-02-11 16:04 ` Dave Hansen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1202922300.25500.37.camel@nimitz.home.sr71.net \
    --to=haveblue@us.ibm.com \
    --cc=RAISCH@de.ibm.com \
    --cc=apw@uk.ibm.com \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=netdev@vger.kernel.org \
    --cc=ossthema@de.ibm.com \
    --cc=osstklei@de.ibm.com \
    --cc=pbadari@us.ibm.com \
    --cc=themann@de.ibm.com \
    --cc=tklein@de.ibm.com \
    --subject='Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).