[PATCH] igb: fix unused igb_deliver_wake_packet() warning when CONFIG_PM=n

2017-07-12 Thread Dave Hansen 

From: Dave Hansen <dave.han...@linux.intel.com>

I'm seeing warnings on kernel configurations where CONFIG_PM is
disabled.  It happens in 4.12, at least:

drivers/ethernet/intel/igb/igb_main.c:7988:13: warning: 
'igb_deliver_wake_packet' defined but not used [-Wunused-function]

This is because igb_deliver_wake_packet() is defined outside of
the #ifdef", but is used only a single time within the #ifdef in
igb_resume().  Fix it by moving igb_deliver_wake_packet() next to
igb_resume() inside the #ifdef.

The diff ends up looking a bit funky here.  It *looks* like
igb_suspend() is getting moved, but that's an artifact of how
'diff' sees the changes.

Cc: Jeff Kirsher <jeffrey.t.kirs...@intel.com>
Cc: intel-wired-...@lists.osuosl.org
Cc: netdev@vger.kernel.org
Signed-off-by: Dave Hansen <dave.han...@linux.intel.com>
---

 b/drivers/net/ethernet/intel/igb/igb_main.c |   46 ++--
 1 file changed, 23 insertions(+), 23 deletions(-)

diff -puN 
drivers/net/ethernet/intel/igb/igb_main.c~undef-igb_deliver_wake_packet 
drivers/net/ethernet/intel/igb/igb_main.c
--- a/drivers/net/ethernet/intel/igb/igb_main.c~undef-igb_deliver_wake_packet   
2017-07-12 14:08:37.205721093 -0700
+++ b/drivers/net/ethernet/intel/igb/igb_main.c 2017-07-12 14:08:37.210721093 
-0700
@@ -7985,6 +7985,29 @@ static int __igb_shutdown(struct pci_dev
return 0;
 }
 
+#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
+static int igb_suspend(struct device *dev)
+{
+   int retval;
+   bool wake;
+   struct pci_dev *pdev = to_pci_dev(dev);
+
+   retval = __igb_shutdown(pdev, , 0);
+   if (retval)
+   return retval;
+
+   if (wake) {
+   pci_prepare_to_sleep(pdev);
+   } else {
+   pci_wake_from_d3(pdev, false);
+   pci_set_power_state(pdev, PCI_D3hot);
+   }
+
+   return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
 static void igb_deliver_wake_packet(struct net_device *netdev)
 {
struct igb_adapter *adapter = netdev_priv(netdev);
@@ -8015,29 +8038,6 @@ static void igb_deliver_wake_packet(stru
netif_rx(skb);
 }
 
-#ifdef CONFIG_PM
-#ifdef CONFIG_PM_SLEEP
-static int igb_suspend(struct device *dev)
-{
-   int retval;
-   bool wake;
-   struct pci_dev *pdev = to_pci_dev(dev);
-
-   retval = __igb_shutdown(pdev, , 0);
-   if (retval)
-   return retval;
-
-   if (wake) {
-   pci_prepare_to_sleep(pdev);
-   } else {
-   pci_wake_from_d3(pdev, false);
-   pci_set_power_state(pdev, PCI_D3hot);
-   }
-
-   return 0;
-}
-#endif /* CONFIG_PM_SLEEP */
-
 static int igb_resume(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
_

Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags

2016-10-19 Thread Dave Hansen 
On 10/19/2016 10:01 AM, Michal Hocko wrote:
> The question I had earlier was whether this has to be an explicit FOLL
> flag used by g-u-p users or we can just use it internally when mm !=
> current->mm

The reason I chose not to do that was that deferred work gets run under
a basically random 'current'.  If we just use 'mm != current->mm', then
the deferred work will sometimes have pkeys enforced and sometimes not,
basically randomly.

We want to be consistent with whether they are enforced or not, so we
explicitly indicate that by calling the remote variant vs. plain.

Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags

2016-10-19 Thread Dave Hansen 
On 10/19/2016 02:07 AM, Michal Hocko wrote:
> On Wed 19-10-16 09:58:15, Lorenzo Stoakes wrote:
>> On Tue, Oct 18, 2016 at 05:30:50PM +0200, Michal Hocko wrote:
>>> I am wondering whether we can go further. E.g. it is not really clear to
>>> me whether we need an explicit FOLL_REMOTE when we can in fact check
>>> mm != current->mm and imply that. Maybe there are some contexts which
>>> wouldn't work, I haven't checked.
>>
>> This flag is set even when /proc/self/mem is used. I've not looked deeply 
>> into
>> this flag but perhaps accessing your own memory this way can be considered
>> 'remote' since you're not accessing it directly. On the other hand, perhaps 
>> this
>> is just mistaken in this case?
> 
> My understanding of the flag is quite limited as well. All I know it is
> related to protection keys and it is needed to bypass protection check.
> See arch_vma_access_permitted. See also 1b2ee1266ea6 ("mm/core: Do not
> enforce PKEY permissions on remote mm access").

Yeah, we need the flag to tell us when PKEYs should be applied or not.
The current task's PKRU (pkey rights register) should really only be
used to impact access to the task's memory, but has no bearing on how a
given task should access remote memory.

Re: [PATCH] e1000: fix e1000e_disable_aspm_locked() warning

2015-09-28 Thread Dave Hansen 
On 08/31/2015 02:26 PM, Dave Hansen wrote:
> From: Dave Hansen <dave.han...@linux.intel.com>
> 
> I have a .config with CONFIG_PM disabled.  I get the following whenever
> compiling the e1000 driver:
> 
> ...net/ethernet/intel/e1000e/netdev.c:6450:13: warning: 
> 'e1000e_disable_aspm_locked' defined but not used [-Wunused-function]
>  static void e1000e_disable_aspm_locked(struct pci_dev *pdev, u16 state)
> 
> Looks like we just need to move e1000e_disable_aspm_locked() to
> be underneath the CONFIG_PM #ifdef.

This patch:

[2758f9edb]: e1000e: Fix incorrect ASPM locking

established a new caller for e1000e_disable_aspm_locked() which makes my
patch useless and wrong (it breaks the compile).

I believe we should just revert my patch.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] e1000: fix e1000e_disable_aspm_locked() warning

2015-08-31 Thread Dave Hansen 

From: Dave Hansen <dave.han...@linux.intel.com>

I have a .config with CONFIG_PM disabled.  I get the following whenever
compiling the e1000 driver:

...net/ethernet/intel/e1000e/netdev.c:6450:13: warning: 
'e1000e_disable_aspm_locked' defined but not used [-Wunused-function]
 static void e1000e_disable_aspm_locked(struct pci_dev *pdev, u16 state)

Looks like we just need to move e1000e_disable_aspm_locked() to
be underneath the CONFIG_PM #ifdef.

Signed-off-by: Dave Hansen <dave.han...@linux.intel.com>
Cc: Jeff Kirsher <jeffrey.t.kirs...@intel.com> (supporter:INTEL ETHERNET 
DRIVERS)
Cc: Jesse Brandeburg <jesse.brandeb...@intel.com> (reviewer)
Cc: Shannon Nelson <shannon.nel...@intel.com> (reviewer)
Cc: Carolyn Wyborny <carolyn.wybo...@intel.com> (reviewer)
Cc: Don Skidmore <donald.c.skidm...@intel.com> (reviewer)
Cc: Matthew Vick <matthew.v...@intel.com> (reviewer)
Cc: John Ronciak <john.ronc...@intel.com> (reviewer)
Cc: Mitch Williams <mitch.a.willi...@intel.com> (reviewer)
Cc: intel-wired-...@lists.osuosl.org (open list:INTEL ETHERNET DRIVERS)
Cc: netdev@vger.kernel.org (open list:NETWORKING DRIVERS)
Cc: linux-ker...@vger.kernel.org (open list)
---

 b/drivers/net/ethernet/intel/e1000e/netdev.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -puN drivers/net/ethernet/intel/e1000e/netdev.c~fix-e1000-warning 
drivers/net/ethernet/intel/e1000e/netdev.c
--- a/drivers/net/ethernet/intel/e1000e/netdev.c~fix-e1000-warning  
2015-08-31 14:19:11.520115321 -0700
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c2015-08-31 
14:19:11.525115549 -0700
@@ -6439,6 +6439,7 @@ static void e1000e_disable_aspm(struct p
__e1000e_disable_aspm(pdev, state, 0);
 }
 
+#ifdef CONFIG_PM
 /**
  * e1000e_disable_aspm_locked   Disable ASPM states.
  * @pdev: pointer to PCI device struct
@@ -6452,7 +6453,6 @@ static void e1000e_disable_aspm_locked(s
__e1000e_disable_aspm(pdev, state, 1);
 }
 
-#ifdef CONFIG_PM
 static int __e1000_resume(struct pci_dev *pdev)
 {
struct net_device *netdev = pci_get_drvdata(pdev);
_
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier

2008-02-20 Thread Dave Hansen 
On Mon, 2008-02-18 at 11:00 +0100, Jan-Bernd Themann wrote:
 Dave Hansen [EMAIL PROTECTED] wrote on 15.02.2008 17:55:38:
 
  I've been thinking about that, and I don't think you really *need* to
  keep a comprehensive map like that. 
  
  When the memory is in a particular configuration (range of memory
  present along with unique set of holes) you get a unique ehea_bmap
  configuration.  That layout is completely predictable.
  
  So, if at any time you want to figure out what the ehea_bmap address for
  a particular *Linux* virtual address is, you just need to pretend that
  you're creating the entire ehea_bmap, use the same algorithm and figure
  out host you would have placed things, and use that result.
  
  Now, that's going to be a slow, crappy linear search (but maybe not as
  slow as recreating the silly thing).  So, you might eventually run into
  some scalability problems with a lot of packets going around.  But, I'd
  be curious if you do in practice.
 
 Up to 14 addresses translation per packet (sg_list) might be required on 
 the transmit side. On receive side it is only 1. Most packets require only 
 very few translations (1 or sometimes more)  translations. However, 
 with more then 700.000 packets per second this approach does not seem 
 reasonable from performance perspective when memory is fragmented as you
 described.

OK, but let's see the data.  *SHOW* me that it's slow. If the algorithm
works, then perhaps we can simply speed it up with a little caching and
*MUCH* less memory overhead.

-- Dave

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier

2008-02-15 Thread Dave Hansen 
On Fri, 2008-02-15 at 14:22 +0100, Christoph Raisch wrote:
 A translation from kernel to ehea_bmap space should be fast and
 predictable
 (ruling out hashes).
 If a driver doesn't know anything else about the mapping structure,
 the normal solution in kernel for this type of problem is a multi
 level
 look up table
 like pgd-pud-pmd-pte
 This doesn't sound right to be implemented in a device driver.
 
 We didn't see from the existing code that such a mapping to a
 contiguous
 space already exists.
 Maybe we've missed it.

I've been thinking about that, and I don't think you really *need* to
keep a comprehensive map like that.  

When the memory is in a particular configuration (range of memory
present along with unique set of holes) you get a unique ehea_bmap
configuration.  That layout is completely predictable.

So, if at any time you want to figure out what the ehea_bmap address for
a particular *Linux* virtual address is, you just need to pretend that
you're creating the entire ehea_bmap, use the same algorithm and figure
out host you would have placed things, and use that result.

Now, that's going to be a slow, crappy linear search (but maybe not as
slow as recreating the silly thing).  So, you might eventually run into
some scalability problems with a lot of packets going around.  But, I'd
be curious if you do in practice.

The other idea is that you create a mapping that is precisely 1:1 with
kernel memory.  Let's say you have two sections present, 0 and 100.  You
have a high_section_index of 100, and you vmalloc() a 100 entry array.

You need to create a *CONTIGUOUS* ehea map?  Create one like this:

EHEA_VADDR-Linux Section
0-0
1-0
2-0
3-0
...
100-100

It's contiguous.  Each area points to a valid Linux memory address.
It's also discernable in O(1) to what EHEA address a given Linux address
is mapped.  You just have a couple of duplicate entries.  

-- Dave

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier

2008-02-14 Thread Dave Hansen 
On Thu, 2008-02-14 at 09:46 +0100, Christoph Raisch wrote:
 Dave Hansen [EMAIL PROTECTED] wrote on 13.02.2008 18:05:00:
  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?
 
 Unfortunately this won't work. This was one of our first ideas we tossed
 out,
 but the number of MRs will not be sufficient.

Can you give a ballpark of how many there are to work with? 10? 100?
1000? 

 We understand that the add/remove area is not as
 settled in the kernel like for example f_ops ;-)
 Are there already base working assumptions which are very unlikely to
 change?

If you use good interfaces, and someone changes them, they'll likely
also fix your driver.

If you use bad interfaces, people may not even notice when they break.
As I showed you with those compile failures, you're using bad interfaces
that don't even compile on some .configs.  

 I'm a little confused here
 ...the existing add/remove code depends on sparse mem.
 Other pieces on the POWER6 version of the architecture do as well.
 So we could either chose to disable add/remove if sparsemem is not there,
 or disable the driver by Kconfig in this case.

Technically, you can do this.  But, it's not a sign of a professionally
written driver that is going to get its patches accepted into mainline.
Technically, you can also use lots of magic numbers and not obey
CodingStyle.  But, you'll probably get review comments asking you to
change it.

   - 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 [EMAIL PROTECTED] and another
  [EMAIL PROTECTED]  That's quite possible.  I think your vmalloc'd
  array will eat all of memory.
 I'm glad you mention this part. There are many algorithms out there to
 handle this problem,
 hashes/trees/... all of these trade speed for smaller memory footprint.
 We based the table decission on the existing implementations of the
 architecture.
 Do you see such a case coming along for the next generation POWER systems?

Dude.  It exists *TODAY*.  Go take a machine, add tens of gigabytes of
memory to it.  Then, remove all of the sections of memory in the middle.
You'll be left with a very sparse memory configuration that we *DO*
handle today in the core VM.  We handle it quite well, actually.

The hypervisor does not shrink memory from the top down.  It pulls
things out of the middle and shuffles things around.  In fact, a NUMA
node's memory isn't even contiguous.

Your code will OOM the machine in this case.  I consider the ehea driver
buggy in this regard.

 I would guess these drastic changes would also require changes in base
 kernel.

No, we actually solved those a couple years ago.  

 Will you provide a generic mapping system with a contiguous virtual address
 space
 like the ehea_bmap we can query? This would need to be a stable part of
 the implementation,
 including translation functions from kernel to nextgen_ehea_generic_bmap
 like virt_to_abs.

Yes, that's a real possibility, especially if some other users for it
come forward.  We could definitely add something like that to the
generic code.  But, you'll have to be convincing that what we have now
is insufficient.

Does this requirement:
- MRs cover a contiguous virtual memory block (no holes)
come from the hardware?

Is that *EACH* MR?  OR all MRs?

Where does EHEA_BUSMAP_START come from?  Is that defined in the
hardware?  Have you checked to ensure that no other users might want a
chunk of memory in that area?

Can you query the existing MRs?  Not change them in place, but can you
query their contents?

  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.
 
 We picked sections instead of PFNs because this keeps the ehea_bmap in a
 reasonable range
 on the existing systems.
 But if you provide a abstract method handling exactly the problem we
 mention
 we'll be happy to use that and dump our

Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier

2008-02-14 Thread Dave Hansen 

On Thu, 2008-02-14 at 09:36 -0800, Badari Pulavarty wrote:
 
 I am not sure what you are trying to do with walk_memory_resource().
 The
 behavior is different on ppc64. Hotplug memory usage assumes that all
 the memory resources (all system memory, not just IOMEM) are
 represented
 in /proc/iomem. Its the case with i386 and ia64. But on ppc64 is
 contains ONLY iomem related. Paulus didn't want to export all the
 system
 memory into /proc/iomem on ppc64. So I had to workaround by providing
 arch-specific walk_memory_resource() function for ppc64.

OK, let's use that one.

-- Dave

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier

2008-02-13 Thread Dave Hansen 
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 [EMAIL PROTECTED] and another
[EMAIL PROTECTED]  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

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier

2008-02-12 Thread Dave Hansen 
On Mon, 2008-02-11 at 17:24 +0100, Jan-Bernd Themann wrote:
 Drivers like eHEA need memory notifiers in order to 
 update their internal DMA memory map when memory is added
 to or removed from the system.
 
 Patch for eHEA memory hotplug support that uses these functions:
 http://www.spinics.net/lists/netdev/msg54484.html

This driver is broken pretty horribly.  It won't even compile for a
plain ppc64 kernel:

http://sr71.net/~dave/linux/ehea-is-broken.config

I know it's used for very specific hardware, but this is the symptom of
it not using the proper abstracted interfaces to the VM.

In file included from 
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_main.c:42:
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.h:44:14: 
warning: SECTION_SIZE_BITS is not defined
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.h:45:2: 
error: #error eHEA module cannot work if kernel sectionsize  ehea sectionsize
  CC  drivers/net/mii.o
make[4]: *** [drivers/net/ehea/ehea_main.o] Error 1
make[4]: *** Waiting for unfinished jobs
  CC  drivers/net/ixgb/ixgb_param.o
In file included from 
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:32:
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.h:44:14: 
warning: SECTION_SIZE_BITS is not defined
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.h:45:2: 
error: #error eHEA module cannot work if kernel sectionsize  ehea sectionsize
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c: In 
function 'ehea_create_busmap':
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:574: 
error: 'NR_MEM_SECTIONS' undeclared (first use in this function)
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:574: 
error: (Each undeclared identifier is reported only once
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:574: 
error: for each function it appears in.)
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:575: 
error: implicit declaration of function 'valid_section_nr'
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c: In 
function 'ehea_map_vaddr':
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:606: 
error: 'SECTION_SIZE_BITS' undeclared (first use in this function)
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c: In 
function 'ehea_reg_kernel_mr':
/home/dave/work/linux/2.6/23/linux-2.6.git/drivers/net/ehea/ehea_qmr.c:655: 
error: 'SECTION_SIZE_BITS' undeclared (first use in this function)

-- Dave

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier

2008-02-11 Thread Dave Hansen 
On Mon, 2008-02-11 at 17:24 +0100, Jan-Bernd Themann wrote:
 Drivers like eHEA need memory notifiers in order to 
 update their internal DMA memory map when memory is added
 to or removed from the system.
 
 Patch for eHEA memory hotplug support that uses these functions:
 http://www.spinics.net/lists/netdev/msg54484.html
 
 Signed-off-by: Jan-Bernd Themann [EMAIL PROTECTED]

How about you fix up the driver, first?  Then, this can go in.

-- Dave

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier

2008-02-11 Thread Dave Hansen 
On Mon, 2008-02-11 at 16:57 +0100, Jan-Bernd Themann wrote:
 Drivers like eHEA need memory notifiers in order to 
 update their internal DMA memory map when memory is added
 to or removed from the system.

Could you post this with the new users as well so we can make sure
they're not abusing this in some way?

-- Dave

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] drivers/base: export gpl (un)register_memory_notifier

2008-02-11 Thread Dave Hansen 
On Mon, 2008-02-11 at 17:24 +0100, Jan-Bernd Themann wrote:
  the eHEA patch belongs to a patchset that is usually
  added by Jeff Garzik once this dependency (EXPORTS)
  is resolved.

I know that's already in mainline but, man, that code is nasty.  It has
stuff indented 7 levels or so and is virtually impossible to read.

This:

 int ehea_create_busmap(void)
 {
 u64 vaddr = EHEA_BUSMAP_START;
 unsigned long high_section_index = 0;
 int i;
 
 /*
  * Sections are not in ascending order - Loop over all sections and
  * find the highest PFN to compute the required map size.
 */
 ehea_bmap.valid_sections = 0;
 
 for (i = 0; i  NR_MEM_SECTIONS; i++)
 if (valid_section_nr(i))
 high_section_index = i;

is also completely bogus for arch-independent code.  If someone ever
wants to do ppc without sparsemem (or redoes internal sparsemem
interfaces), this code will break.  

Also, keeping your own mapping of all of memory is really nasty.  With
SPARSEMEM_EXTREME/VMEMMAP you can have extremely sparse physical memory,
and this:

 ehea_bmap.entries = high_section_index + 1;
 ehea_bmap.vaddr = vmalloc(ehea_bmap.entries * 
 sizeof(*ehea_bmap.vaddr));

could theoretically consume all of memory if the sections are very sparse.

Could you please try to explain what the heck this driver is doing?
We'll try to fix up the generic interfaces so that you can deal with
things in a more generic fashion, but it can't stay this way.

Also, just ripping down and completely re-doing the entire mass of cards
every time a 16MB area of memory is added or removed seems like an
awfully big sledgehammer to me.  I would *HATE* to see anybody else
using this driver as an example to work off of?  Can't you just keep
track of which areas the driver is actually *USING* and only worry about
changing mappings if that intersects with an area having hotplug done on
it?

Can you please give it some CodingStyle love and make it so that it
doesn't indent so much?  Something like this:

diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index c051c7e..72c5652 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -2638,38 +2638,40 @@ static void ehea_rereg_mrs(struct work_struct *work)
down(dlpar_mem_lock);
ehea_info(LPAR memory enlarged - re-initializing driver);
 
-   list_for_each_entry(adapter, adapter_list, list)
-   if (adapter-active_ports) {
-   /* Shutdown all ports */
-   for (i = 0; i  EHEA_MAX_PORTS; i++) {
-   struct ehea_port *port = adapter-port[i];
-
-   if (port) {
-   struct net_device *dev = port-netdev;
-
-   if (dev-flags  IFF_UP) {
-   down(port-port_lock);
-   netif_stop_queue(dev);
-   ret = ehea_stop_qps(dev);
-   if (ret) {
-   up(port-port_lock);
-   goto out;
-   }
-   port_napi_disable(port);
-   up(port-port_lock);
-   }
-   }
-   }
-
-   /* Unregister old memory region */
-   ret = ehea_rem_mr(adapter-mr);
+   list_for_each_entry(adapter, adapter_list, list) {
+   if (!adapter-active_ports)
+   continue;
+   /* Shutdown all ports */
+   for (i = 0; i  EHEA_MAX_PORTS; i++) {
+   struct ehea_port *port = adapter-port[i];
+   struct net_device *dev;
+   if (!port)
+   continue;
+
+   dev = port-netdev;
+   if (!(dev-flags  IFF_UP))
+   continue;
+
+   down(port-port_lock);
+   netif_stop_queue(dev);
+   ret = ehea_stop_qps(dev);
if (ret) {
-   ehea_error(unregister MR failed - driver
-   inoperable!);
+   up(port-port_lock);
goto out;
}
+   port_napi_disable(port);
+   up(port-port_lock);
}
 
+   /* Unregister old memory region */
+   ret = ehea_rem_mr(adapter-mr);
+   if (ret) {
+

Re: [PATCH][RESEND] drivers/base: export (un)register_memory_notifier

2008-02-11 Thread Dave Hansen 
On Mon, 2008-02-11 at 10:49 +0100, Jan-Bernd Themann wrote:
 are you the right person to address this patch to?

You might want to check the top of the file. ;)

 --- a/drivers/base/memory.c
 +++ b/drivers/base/memory.c
 @@ -52,11 +52,13 @@ int register_memory_notifier(struct notifier_block *nb)
  {
  return blocking_notifier_chain_register(memory_chain, nb);
  }
 +EXPORT_SYMBOL(register_memory_notifier);
  
  void unregister_memory_notifier(struct notifier_block *nb)
  {
  blocking_notifier_chain_unregister(memory_chain, nb);
  }
 +EXPORT_SYMBOL(unregister_memory_notifier);

Is there a particular reason these can't be GPL?

-- Dave

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/9] network namespaces: core and device list

2006-08-16 Thread Dave Hansen 
On Tue, 2006-08-15 at 18:48 +0400, Andrey Savochkin wrote:
 
 /* Can survive without statistics */
 stats = kmalloc(sizeof(struct net_device_stats), GFP_KERNEL);
 if (stats) {
 memset(stats, 0, sizeof(struct net_device_stats));
 -   loopback_dev.priv = stats;
 -   loopback_dev.get_stats = get_stats;
 +   dev-priv = stats;
 +   dev-get_stats = get_stats;
 } 

With this much surgery it might be best to start using things that have
come along since this code was touched last, like kzalloc().

-- Dave

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 2/6] [Network namespace] Network device sharing by view

2006-06-27 Thread Dave Hansen 
On Wed, 2006-06-28 at 00:52 +0200, Herbert Poetzl wrote:
 seriously, what I think Eric meant was that it
 might be nice (especially for migration purposes)
 to keep the device namespace completely virtualized
 and not just isolated ...

It might be nice, but it is probably unneeded for an initial
implementation.  In practice, a cluster doing
checkpoint/restart/migration will already have a system in place for
assigning unique IPs or other identifiers to each container.  It could
just as easily make sure to assign unique network device names to
containers.

The issues really only come into play when you have an unstructured set
of machines and you want to migrate between them without having prepared
them with any kind of unique net device names beforehand.

It may look weird, but do application really *need* to see eth0 rather
than eth858354?

-- Dave

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Current git tree oopses in tcp_time_to_recover

2005-09-08 Thread Dave Hansen 
I've noticed two oopses with a git tree I pulled yesterday.  Last
commit: 4706df3d3c42af802597d82c8b1542c3d52eab23

Probably the same bug as the one that Andrew Morton forwarded here:

http://marc.theaimsgroup.com/?l=linux-netdevm=112618307911533w=2

I see a bunch of assertion failures, followed by a partial oops that I
got from a netconsole.  The oops occurred _just_ after I started up
gaim, and I've noticed this as the trigger each time the system froze.

KERNEL: assertion ((int)tp-lost_out = 0) failed at net/ipv4/tcp_input.c (2148)
KERNEL: assertion ((int)tp-lost_out = 0) failed at net/ipv4/tcp_input.c (1127)
KERNEL: assertion ((int)tp-lost_out = 0) failed at net/ipv4/tcp_input.c (2148)
KERNEL: assertion ((int)tp-lost_out = 0) failed at net/ipv4/tcp_input.c (1127)
KERNEL: assertion ((int)tp-lost_out = 0) failed at net/ipv4/tcp_input.c (2148)
KERNEL: assertion ((int)tp-lost_out = 0) failed at net/ipv4/tcp_input.c (2148)
KERNEL: assertion ((int)tp-sacked_out = 0) failed at net/ipv4/tcp_input.c 
(2147)
KERNEL: assertion ((int)tp-sacked_out = 0) failed at net/ipv4/tcp_input.c 
(2147)
KERNEL: assertion ((int)tp-sacked_out = 0) failed at net/ipv4/tcp_input.c 
(2147)
KERNEL: assertion ((int)tp-sacked_out = 0) failed at net/ipv4/tcp_input.c 
(2147)
KERNEL: assertion ((int)tp-sacked_out = 0) failed at net/ipv4/tcp_input.c 
(2147)
KERNEL: assertion ((int)tp-sacked_out = 0) failed at net/ipv4/tcp_input.c 
(2147)
KERNEL: assertion ((int)tp-sacked_out = 0) failed at net/ipv4/tcp_input.c 
(2147)
KERNEL: assertion ((int)tp-sacked_out = 0) failed at net/ipv4/tcp_input.c 
(2147)
KERNEL: assertion ((int)tp-sacked_out = 0) failed at net/ipv4/tcp_input.c 
(2147)
KERNEL: assertion ((int)tp-sacked_out = 0) failed at net/ipv4/tcp_input.c 
(2147)
KERNEL: assertion ((int)tp-sacked_out = 0) failed at net/ipv4/tcp_input.c 
(2147)
KERNEL: assertion ((int)tp-sacked_out = 0) failed at net/ipv4/tcp_input.c 
(2147)
KERNEL: assertion ((int)tp-sacked_out = 0) failed at net/ipv4/tcp_input.c 
(2147)
KERNEL: assertion ((int)tp-lost_out = 0) failed at net/ipv4/tcp_input.c (2148)
KERNEL: assertion ((int)tp-lost_out = 0) failed at net/ipv4/tcp_input.c (2148)
Leak l=4294967294 4
KERNEL: assertion (!skb_queue_empty(sk-sk_write_queue)) failed at 
net/ipv4/tcp_timer.c (291)
Unable to handle kernel NULL pointer dereference at virtual address 0048
 printing eip:
c032be51
*pde = 
Oops:  [#1]
PREEMPT
Modules linked in: netconsole pcmcia thermal fan button processor ac
battery nfs lockd sunrpc ipt_state iptable_filter iptable_nat ip_tables
ip_conntrack rtc ipw2200 ieee80211 ieee80211_crypt 3c59x mii e1000
yenta_socket rsrc_nonstatic pcmcia_core snd_intel8x0 snd_ac97_codec
snd_ac97_bus snd_pcm_oss snd_mixer_oss snd_pcm snd_timer snd soundcore
snd_page_alloc ehci_hcd usbhid uhci_hcd usbcore
CPU:0
EIP:0060:[c032be51]Not tainted VLI
EFLAGS: 00010246   (2.6.13-g4706df3d)
EIP is at tcp_time_to_recover+0x59/0xb0
eax: 00ceaac8   ebx: d7763000   ecx:    edx: 
esi:    edi: d7763000   ebp: 0003   esp: c04bee60
ds: 007b   es: 007b   ss: 0068

--

I'll see if I can get a better oops.  Just wanted to send this out
before I crash again :)

-- Dave

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] create sysfs device files for hostap

2005-07-15 Thread Dave Hansen 
I was writing some scripts to automatically build kismet source lines,
and I noticed that hostap devices don't have device files, unlike my
prism54 and ipw2200 cards:

$ ls -l /sys/class/net/eth0/device
/sys/class/net/eth0/device - 
../../../devices/pci:00/:00:1e.0/:02:01.0
$ ls -l /sys/class/net/wifi0
ls: /sys/class/net/wifi0/device: No such file or directory
$ ls -l /sys/class/net/wlan0
ls: /sys/class/net/wlan0/device: No such file or directory

The following (quite small) patch makes sure that both the wlan and wifi
net devices have that pointer to the bus device.

This way, I can do things like

for i in /sys/class/net/*; do
if ! [ -e $i/device/drive ]; then
continue;
fi;
driver=$(basename $(readlink $i/device/driver))
case $driver in
hostap*)
echo -- hostap,$i,$i-$driver
break; 
ipw2?00)
echo -- $driver,$i,$i-$driver
break;
prism54)
echo prism54g,$i 
esac
done

Which should generate a working set of source lines for kismet no matter
what order I plug the cards in.  

It might also be handy to have a link between the two net devices, but
that's a patch for another day.

That patch is against 2.6.13-rc1-mm1.

-- Dave

Signed-off-by: Dave Hansen [EMAIL PROTECTED]

diff -ru linux-2.6-mm.orig/drivers/net/wireless/hostap/hostap.c 
linux-2.6-mm/drivers/net/wireless/hostap/hostap.c
--- linux-2.6-mm.orig/drivers/net/wireless/hostap/hostap.c  2005-07-15 
10:57:18.0 -0700
+++ linux-2.6-mm/drivers/net/wireless/hostap/hostap.c   2005-07-15 
12:29:15.0 -0700
@@ -141,6 +141,7 @@
if (strchr(dev-name, '%'))
ret = dev_alloc_name(dev, dev-name);
 
+   SET_NETDEV_DEV(dev, mdev-class_dev.dev);
if (ret = 0)
ret = register_netdevice(dev);
 
diff -ru linux-2.6-mm.orig/drivers/net/wireless/hostap/hostap_cs.c 
linux-2.6-mm/drivers/net/wireless/hostap/hostap_cs.c
--- linux-2.6-mm.orig/drivers/net/wireless/hostap/hostap_cs.c   2005-07-15 
10:57:18.0 -0700
+++ linux-2.6-mm/drivers/net/wireless/hostap/hostap_cs.c2005-07-15 
12:30:57.0 -0700
@@ -739,7 +739,8 @@
}
 
/* Need to allocate net_device before requesting IRQ handler */
-   dev = prism2_init_local_data(prism2_pccard_funcs, 0);
+   dev = prism2_init_local_data(prism2_pccard_funcs, 0,
+handle_to_dev(link-handle));
if (dev == NULL)
goto failed;
link-priv = dev;
diff -ru linux-2.6-mm.orig/drivers/net/wireless/hostap/hostap_hw.c 
linux-2.6-mm/drivers/net/wireless/hostap/hostap_hw.c
--- linux-2.6-mm.orig/drivers/net/wireless/hostap/hostap_hw.c   2005-07-15 
10:57:18.0 -0700
+++ linux-2.6-mm/drivers/net/wireless/hostap/hostap_hw.c2005-07-15 
12:27:59.0 -0700
@@ -3268,7 +3268,8 @@
 

 static struct net_device *
-prism2_init_local_data(struct prism2_helper_functions *funcs, int card_idx)
+prism2_init_local_data(struct prism2_helper_functions *funcs, int card_idx,
+  struct device *sdev)
 {
struct net_device *dev;
struct hostap_interface *iface;
@@ -3439,6 +3440,7 @@
 
rtnl_lock();
ret = dev_alloc_name(dev, wifi%d);
+   SET_NETDEV_DEV(dev, sdev);
if (ret = 0)
ret = register_netdevice(dev);
rtnl_unlock();
diff -ru linux-2.6-mm.orig/drivers/net/wireless/hostap/hostap_pci.c 
linux-2.6-mm/drivers/net/wireless/hostap/hostap_pci.c
--- linux-2.6-mm.orig/drivers/net/wireless/hostap/hostap_pci.c  2005-07-15 
10:57:18.0 -0700
+++ linux-2.6-mm/drivers/net/wireless/hostap/hostap_pci.c   2005-07-15 
12:17:35.0 -0700
@@ -308,7 +308,8 @@
pci_set_master(pdev);
 #endif /* PRISM2_BUS_MASTER */
 
-   dev = prism2_init_local_data(prism2_pci_funcs, cards_found);
+   dev = prism2_init_local_data(prism2_pci_funcs, cards_found,
+pdev-dev);
if (dev == NULL)
goto fail;
iface = netdev_priv(dev);
diff -ru linux-2.6-mm.orig/drivers/net/wireless/hostap/hostap_plx.c 
linux-2.6-mm/drivers/net/wireless/hostap/hostap_plx.c
--- linux-2.6-mm.orig/drivers/net/wireless/hostap/hostap_plx.c  2005-07-15 
10:57:18.0 -0700
+++ linux-2.6-mm/drivers/net/wireless/hostap/hostap_plx.c   2005-07-15 
12:17:01.0 -0700
@@ -522,7 +522,8 @@
 * not present; but are there really such cards in use(?) */
}
 
-   dev = prism2_init_local_data(prism2_plx_funcs, cards_found);
+   dev = prism2_init_local_data(prism2_plx_funcs, cards_found,
+pdev-dev);
if (dev == NULL)
goto fail