* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Andy Lutomirski @ 2014-11-14 20:16 UTC (permalink / raw)
To: Tom Herbert
Cc: Eric Dumazet, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <CA+mtBx_DyUCxE6BqL_xXcqKPUDu7b3jLShV+AL6L+gCzJLriPg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Nov 14, 2014 at 11:52 AM, Tom Herbert <therbert-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, Nov 14, 2014 at 11:33 AM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Fri, 2014-11-14 at 09:17 -0800, Andy Lutomirski wrote:
>>
>>> As a heavy user of RFS (and finder of bugs in it, too), here's my
>>> question about this API:
>>>
>>> How does an application tell whether the socket represents a
>>> non-actively-steered flow? If the flow is subject to RFS, then moving
>>> the application handling to the socket's CPU seems problematic, as the
>>> socket's CPU might move as well. The current implementation in this
>>> patch seems to tell me which CPU the most recent packet came in on,
>>> which is not necessarily very useful.
>>
>> Its the cpu that hit the TCP stack, bringing dozens of cache lines in
>> its cache. This is all that matters,
>>
>>>
>>> Some possibilities:
>>>
>>> 1. Let SO_INCOMING_CPU fail if RFS or RPS are in play.
>>
>> Well, idea is to not use RFS at all. Otherwise, it is useless.
Sure, but how do I know that it'll be the same CPU next time?
>>
> Bear in mind this is only an interface to report RX CPU and in itself
> doesn't provide any functionality for changing scheduling, there is
> obviously logic needed in user space that would need to do something.
>
> If we track the interrupting CPU in skb, the interface could be easily
> extended to provide the interrupting CPU, the RPS CPU (calculated at
> reported time), and the CPU processing transport (post steering which
> is what is currently returned). That would provide the complete
> picture to control scheduling a flow from userspace, and an interface
> to selectively turn off RFS for a socket would make sense then.
I think that a turn-off-RFS interface would also want a way to figure
out where the flow would go without RFS. Can the network stack do
that (e.g. evaluate the rx indirection hash or whatever happens these
days)?
>
>> RFS is the other way around : You want the flow to follow your thread.
>>
>> RPS wont be a problem if you have sensible RPS settings.
>>
>>>
>>> 2. Change the interface a bit to report the socket's preferred CPU
>>> (where it would go without RFS, for example) and then let the
>>> application use setsockopt to tell the socket to stay put (i.e. turn
>>> off RFS and RPS for that flow).
>>>
>>> 3. Report the preferred CPU as in (2) but let the application ask for
>>> something different.
>>>
>>> For example, I have flows for which I know which CPU I want. A nice
>>> API to put the flow there would be quite useful.
>>>
>>>
>>> Also, it may be worth changing the naming to indicate that these are
>>> about the rx cpu (they are, right?). For some applications (sparse,
>>> low-latency flows, for example), it can be useful to keep the tx
>>> completion handling on a different CPU.
>>
>> SO_INCOMING_CPU is rx, like incoming ;)
>>
>>
Duh :)
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Jay Vosburgh @ 2014-11-14 20:15 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: David Miller, eric.dumazet, netdev, ogerlitz, pshelar, jesse,
discuss
In-Reply-To: <1415995451.15154.54.camel@localhost>
Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
>On Fr, 2014-11-14 at 13:38 -0500, David Miller wrote:
>> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
>> Date: Fri, 14 Nov 2014 16:46:18 +0100
>>
>> > I would still like to see the current proposed fix getting applied and
>> > we can do this on-top. The inline call after this patch reassembles a
>> > direct function call, so besides the long list of clobbers, it should
>> > still be pretty fast.
>>
>> I would rather revert the change entirely until it is implemented
>> properly.
>>
>> Also, I am strongly of the opinion that this is a mis-use of the
>> alternative call interface. It was never intended to be used for
>> things that can make real function calls.
>
>I tend to disagree. Grepping e.g. shows
>
> alternative_call_2(copy_user_generic_unrolled,
> copy_user_generic_string,
> X86_FEATURE_REP_GOOD,
> copy_user_enhanced_fast_string,
> X86_FEATURE_ERMS,
> ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from),
> "=d" (len)),
> "1" (to), "2" (from), "3" (len)
> : "memory", "rcx", "r8", "r9", "r10", "r11");
>
>
>(it has a few less clobbers because it has more output operands)
As those functions (copy_user_generic_unrolled, et al) are all
in assembly language, presumably the list of clobbered registers can be
had via inspection.
For the arch_fast_hash2 case, the functions (__intel_crc4_2_hash
and __jash2) are both written in C, so how would the clobber list be
created?
-J
>I just tried to come up with some macros which lets you abstract away
>the clobber list, but in the end it somehow has to look exactly like
>that. The double-colon syntax also makes it difficult to come up with
>something that let's us use varargs for that.
>
>> You can add a million clobbers, or a trampoline, it's still using a
>> facility in a manner for which it was not designed.
>
>The full clobber list for a function call which would always clear
>registers like we would have in a normal non-inlined function call would
>look like this:
>
>#define FUNC_CLOBBER LIST "memory", "cc", "rax", "rdi", "rsi", "rdx", "rcx", "r8", "r9", "r10", "r11"
>
>(reference in arch/x86/include/asm/calling.h).
>
>> This means a new interface with a new name and with capabilities
>> explicitly supporting this case are in order.
>
>It try to implicitly embed the clobber list, would something like that
>be ok?
>
>Thanks,
>Hannes
---
-Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Hannes Frederic Sowa @ 2014-11-14 20:10 UTC (permalink / raw)
To: David Miller
Cc: eric.dumazet, netdev, ogerlitz, pshelar, jesse, jay.vosburgh,
discuss
In-Reply-To: <1415995451.15154.54.camel@localhost>
On Fr, 2014-11-14 at 21:04 +0100, Hannes Frederic Sowa wrote:
> On Fr, 2014-11-14 at 13:38 -0500, David Miller wrote:
> > From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > Date: Fri, 14 Nov 2014 16:46:18 +0100
> >
> > > I would still like to see the current proposed fix getting applied and
> > > we can do this on-top. The inline call after this patch reassembles a
> > > direct function call, so besides the long list of clobbers, it should
> > > still be pretty fast.
> >
> > I would rather revert the change entirely until it is implemented
> > properly.
> >
> > Also, I am strongly of the opinion that this is a mis-use of the
> > alternative call interface. It was never intended to be used for
> > things that can make real function calls.
>
> I tend to disagree. Grepping e.g. shows
>
> alternative_call_2(copy_user_generic_unrolled,
> copy_user_generic_string,
> X86_FEATURE_REP_GOOD,
> copy_user_enhanced_fast_string,
> X86_FEATURE_ERMS,
> ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from),
> "=d" (len)),
> "1" (to), "2" (from), "3" (len)
> : "memory", "rcx", "r8", "r9", "r10", "r11");
Oh, I need to take this back, These are actually assembler functions.
^ permalink raw reply
* Re: [PATCH] ixgbe: Look up MAC address in Open Firmware
From: Sergei Shtylyov @ 2014-11-14 20:07 UTC (permalink / raw)
To: Martin K. Petersen, netdev; +Cc: linux.nics
In-Reply-To: <yq1r3x5vbh9.fsf@sermon.lab.mkp.net>
Hello.
On 11/14/2014 10:16 PM, Martin K. Petersen wrote:
> Attempt to look up the MAC address in Open Firmware on systems that
> support it. If the "local-mac-address" property is not valid resort to
> using the IDPROM value.
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index d2df4e3d1032..365924124fab 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
[...]
> @@ -7959,6 +7964,31 @@ int ixgbe_wol_supported(struct ixgbe_adapter *adapter, u16 device_id,
> return is_wol_supported;
> }
>
> +#ifdef CONFIG_OF
> +/**
> + * ixgbe_of_mac_addr - Look up MAC address in Open Firmware
> + * @adapter: Pointer to adapter struct
> + */
> +static void ixgbe_of_mac_addr(struct ixgbe_adapter *adapter)
> +{
> + struct device_node *dp = pci_device_to_OF_node(adapter->pdev);
> + struct ixgbe_hw *hw = &adapter->hw;
> + const unsigned char *addr;
> + int len;
> +
> + addr = of_get_property(dp, "local-mac-address", &len);
> + if (addr && len == 6) {
> + e_dev_info("Using OpenPROM MAC address\n");
> + memcpy(hw->mac.perm_addr, addr, 6);
> + }
> +
> + if (!is_valid_ether_addr(hw->mac.perm_addr)) {
> + e_dev_info("Using IDPROM MAC address\n");
> + memcpy(hw->mac.perm_addr, idprom->id_ethaddr, 6);
> + }
> +}
> +#endif
> +
> /**
> * ixgbe_probe - Device Initialization Routine
> * @pdev: PCI device information struct
> @@ -8223,6 +8253,10 @@ skip_sriov:
> goto err_sw_init;
> }
>
> +#ifdef CONFIG_OF
> + ixgbe_of_mac_addr(adapter);
> +#endif
Eww... why not define the following above instead:
#else
static inline void ixgbe_of_mac_addr(struct ixgbe_adapter *adapter) {}
#endif
This is closer to what Documentation/SubmittingPatches suggests.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Hannes Frederic Sowa @ 2014-11-14 20:04 UTC (permalink / raw)
To: David Miller
Cc: eric.dumazet, netdev, ogerlitz, pshelar, jesse, jay.vosburgh,
discuss
In-Reply-To: <20141114.133829.1437047454714311242.davem@davemloft.net>
On Fr, 2014-11-14 at 13:38 -0500, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Fri, 14 Nov 2014 16:46:18 +0100
>
> > I would still like to see the current proposed fix getting applied and
> > we can do this on-top. The inline call after this patch reassembles a
> > direct function call, so besides the long list of clobbers, it should
> > still be pretty fast.
>
> I would rather revert the change entirely until it is implemented
> properly.
>
> Also, I am strongly of the opinion that this is a mis-use of the
> alternative call interface. It was never intended to be used for
> things that can make real function calls.
I tend to disagree. Grepping e.g. shows
alternative_call_2(copy_user_generic_unrolled,
copy_user_generic_string,
X86_FEATURE_REP_GOOD,
copy_user_enhanced_fast_string,
X86_FEATURE_ERMS,
ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from),
"=d" (len)),
"1" (to), "2" (from), "3" (len)
: "memory", "rcx", "r8", "r9", "r10", "r11");
(it has a few less clobbers because it has more output operands)
I just tried to come up with some macros which lets you abstract away
the clobber list, but in the end it somehow has to look exactly like
that. The double-colon syntax also makes it difficult to come up with
something that let's us use varargs for that.
> You can add a million clobbers, or a trampoline, it's still using a
> facility in a manner for which it was not designed.
The full clobber list for a function call which would always clear
registers like we would have in a normal non-inlined function call would
look like this:
#define FUNC_CLOBBER LIST "memory", "cc", "rax", "rdi", "rsi", "rdx", "rcx", "r8", "r9", "r10", "r11"
(reference in arch/x86/include/asm/calling.h).
> This means a new interface with a new name and with capabilities
> explicitly supporting this case are in order.
It try to implicitly embed the clobber list, would something like that
be ok?
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Tom Herbert @ 2014-11-14 19:52 UTC (permalink / raw)
To: Eric Dumazet
Cc: Andy Lutomirski, Michael Kerrisk, David Miller, netdev, Ying Cai,
Willem de Bruijn, Neal Cardwell, Linux API
In-Reply-To: <1415993614.17262.55.camel@edumazet-glaptop2.roam.corp.google.com>
On Fri, Nov 14, 2014 at 11:33 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2014-11-14 at 09:17 -0800, Andy Lutomirski wrote:
>
>> As a heavy user of RFS (and finder of bugs in it, too), here's my
>> question about this API:
>>
>> How does an application tell whether the socket represents a
>> non-actively-steered flow? If the flow is subject to RFS, then moving
>> the application handling to the socket's CPU seems problematic, as the
>> socket's CPU might move as well. The current implementation in this
>> patch seems to tell me which CPU the most recent packet came in on,
>> which is not necessarily very useful.
>
> Its the cpu that hit the TCP stack, bringing dozens of cache lines in
> its cache. This is all that matters,
>
>>
>> Some possibilities:
>>
>> 1. Let SO_INCOMING_CPU fail if RFS or RPS are in play.
>
> Well, idea is to not use RFS at all. Otherwise, it is useless.
>
Bear in mind this is only an interface to report RX CPU and in itself
doesn't provide any functionality for changing scheduling, there is
obviously logic needed in user space that would need to do something.
If we track the interrupting CPU in skb, the interface could be easily
extended to provide the interrupting CPU, the RPS CPU (calculated at
reported time), and the CPU processing transport (post steering which
is what is currently returned). That would provide the complete
picture to control scheduling a flow from userspace, and an interface
to selectively turn off RFS for a socket would make sense then.
> RFS is the other way around : You want the flow to follow your thread.
>
> RPS wont be a problem if you have sensible RPS settings.
>
>>
>> 2. Change the interface a bit to report the socket's preferred CPU
>> (where it would go without RFS, for example) and then let the
>> application use setsockopt to tell the socket to stay put (i.e. turn
>> off RFS and RPS for that flow).
>>
>> 3. Report the preferred CPU as in (2) but let the application ask for
>> something different.
>>
>> For example, I have flows for which I know which CPU I want. A nice
>> API to put the flow there would be quite useful.
>>
>>
>> Also, it may be worth changing the naming to indicate that these are
>> about the rx cpu (they are, right?). For some applications (sparse,
>> low-latency flows, for example), it can be useful to keep the tx
>> completion handling on a different CPU.
>
> SO_INCOMING_CPU is rx, like incoming ;)
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/1 net-next] net: dsa: replace count*size kmalloc by kmalloc_array
From: Joe Perches @ 2014-11-14 19:42 UTC (permalink / raw)
To: Fabian Frederick; +Cc: netdev, linux-kernel, David S. Miller
In-Reply-To: <47990659.56515.1415993549555.open-xchange@webmail.nmp.skynet.be>
On Fri, 2014-11-14 at 20:32 +0100, Fabian Frederick wrote:
> On 14 November 2014 at 20:14 Joe Perches <joe@perches.com> wrote:
> > On Fri, 2014-11-14 at 20:02 +0100, Fabian Frederick wrote:
> > > On 14 November 2014 at 19:47 Joe Perches <joe@perches.com> wrote:
> > > > On Fri, 2014-11-14 at 19:36 +0100, Fabian Frederick wrote:
> > > > > kmalloc_array manages count*sizeof overflow.
> > > >
> > > > Fundamentally correct, but is this necessary or useful?
> > > > sizeof(s8) isn't often going to be anything other than 1.
> > > Absolutely, I thought it was a struct :)
> > >
> > > There must be a reason for so many cases though ...
> >
> > Some might be style symmetry for other sizeof(othertype)
> > uses in the same paths, but most of them are just overkill
> > or maybe lack of understanding.
> 95% comes from drivers tree. I guess one patch to Greg would be enough.
I think it'd be better to send patches through the
appropriate various maintainers
Likely just using the 2nd level directory would be
good enough
$ git grep -E --name-only "\*\s*sizeof\s*\(\s*[us]8\s*\)" | \
cut -f1-2 -d"/" | uniq
arch/arm
drivers/acpi
drivers/char
drivers/gpu
drivers/iio
drivers/infiniband
drivers/input
drivers/md
drivers/media
drivers/mtd
drivers/net
drivers/power
drivers/rtc
drivers/thermal
fs/compat_ioctl.c
net/dsa
> Are you interested in those patches or can I do them with some "Suggested-by" ?
Not really and no need.
cheers, Joe
^ permalink raw reply
* Re: [PATCH net-next] net: introduce SO_INCOMING_CPU
From: Eric Dumazet @ 2014-11-14 19:33 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Michael Kerrisk, David Miller, netdev, Ying Cai, Willem de Bruijn,
Neal Cardwell, Linux API
In-Reply-To: <CALCETrXmCWSoeQLC3WCqMf_sehGVE9KTbTGmsA77ZN84B-V24Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, 2014-11-14 at 09:17 -0800, Andy Lutomirski wrote:
> As a heavy user of RFS (and finder of bugs in it, too), here's my
> question about this API:
>
> How does an application tell whether the socket represents a
> non-actively-steered flow? If the flow is subject to RFS, then moving
> the application handling to the socket's CPU seems problematic, as the
> socket's CPU might move as well. The current implementation in this
> patch seems to tell me which CPU the most recent packet came in on,
> which is not necessarily very useful.
Its the cpu that hit the TCP stack, bringing dozens of cache lines in
its cache. This is all that matters,
>
> Some possibilities:
>
> 1. Let SO_INCOMING_CPU fail if RFS or RPS are in play.
Well, idea is to not use RFS at all. Otherwise, it is useless.
RFS is the other way around : You want the flow to follow your thread.
RPS wont be a problem if you have sensible RPS settings.
>
> 2. Change the interface a bit to report the socket's preferred CPU
> (where it would go without RFS, for example) and then let the
> application use setsockopt to tell the socket to stay put (i.e. turn
> off RFS and RPS for that flow).
>
> 3. Report the preferred CPU as in (2) but let the application ask for
> something different.
>
> For example, I have flows for which I know which CPU I want. A nice
> API to put the flow there would be quite useful.
>
>
> Also, it may be worth changing the naming to indicate that these are
> about the rx cpu (they are, right?). For some applications (sparse,
> low-latency flows, for example), it can be useful to keep the tx
> completion handling on a different CPU.
SO_INCOMING_CPU is rx, like incoming ;)
^ permalink raw reply
* Re: [PATCH 1/1 net-next] net: dsa: replace count*size kmalloc by kmalloc_array
From: Fabian Frederick @ 2014-11-14 19:32 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, linux-kernel, David S. Miller
In-Reply-To: <1415992486.5912.23.camel@perches.com>
> On 14 November 2014 at 20:14 Joe Perches <joe@perches.com> wrote:
>
>
> On Fri, 2014-11-14 at 20:02 +0100, Fabian Frederick wrote:
> > On 14 November 2014 at 19:47 Joe Perches <joe@perches.com> wrote:
> > > On Fri, 2014-11-14 at 19:36 +0100, Fabian Frederick wrote:
> > > > kmalloc_array manages count*sizeof overflow.
> > >
> > > Fundamentally correct, but is this necessary or useful?
> > > sizeof(s8) isn't often going to be anything other than 1.
> > Absolutely, I thought it was a struct :)
> >
> > There must be a reason for so many cases though ...
>
> Some might be style symmetry for other sizeof(othertype)
> uses in the same paths, but most of them are just overkill
> or maybe lack of understanding.
>
> s8 is char so by definition it has to be 1.
>
> I doubt any of the code dates from PDP-8/TOPS-10 days.
>
>
95% comes from drivers tree. I guess one patch to Greg would be enough.
Are you interested in those patches or can I do them with some "Suggested-by" ?
^ permalink raw reply
* Re: [linux-nics] [PATCH] ixgbe: Look up MAC address in Open Firmware
From: Jeff Kirsher @ 2014-11-14 19:26 UTC (permalink / raw)
To: Martin K. Petersen; +Cc: netdev, linux.nics
In-Reply-To: <yq1r3x5vbh9.fsf@sermon.lab.mkp.net>
[-- Attachment #1: Type: text/plain, Size: 361 bytes --]
On Fri, 2014-11-14 at 14:16 -0500, Martin K. Petersen wrote:
>
> Attempt to look up the MAC address in Open Firmware on systems that
> support it. If the "local-mac-address" property is not valid resort to
> using the IDPROM value.
>
> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Thanks Martin, I will add your patch to my queue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH] ixgbe: Look up MAC address in Open Firmware
From: Martin K. Petersen @ 2014-11-14 19:16 UTC (permalink / raw)
To: netdev; +Cc: linux.nics
Attempt to look up the MAC address in Open Firmware on systems that
support it. If the "local-mac-address" property is not valid resort to
using the IDPROM value.
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d2df4e3d1032..365924124fab 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -50,6 +50,11 @@
#include <linux/prefetch.h>
#include <scsi/fc/fc_fcoe.h>
+#ifdef CONFIG_OF
+#include <asm/idprom.h>
+#include <asm/prom.h>
+#endif
+
#include "ixgbe.h"
#include "ixgbe_common.h"
#include "ixgbe_dcb_82599.h"
@@ -7959,6 +7964,31 @@ int ixgbe_wol_supported(struct ixgbe_adapter *adapter, u16 device_id,
return is_wol_supported;
}
+#ifdef CONFIG_OF
+/**
+ * ixgbe_of_mac_addr - Look up MAC address in Open Firmware
+ * @adapter: Pointer to adapter struct
+ */
+static void ixgbe_of_mac_addr(struct ixgbe_adapter *adapter)
+{
+ struct device_node *dp = pci_device_to_OF_node(adapter->pdev);
+ struct ixgbe_hw *hw = &adapter->hw;
+ const unsigned char *addr;
+ int len;
+
+ addr = of_get_property(dp, "local-mac-address", &len);
+ if (addr && len == 6) {
+ e_dev_info("Using OpenPROM MAC address\n");
+ memcpy(hw->mac.perm_addr, addr, 6);
+ }
+
+ if (!is_valid_ether_addr(hw->mac.perm_addr)) {
+ e_dev_info("Using IDPROM MAC address\n");
+ memcpy(hw->mac.perm_addr, idprom->id_ethaddr, 6);
+ }
+}
+#endif
+
/**
* ixgbe_probe - Device Initialization Routine
* @pdev: PCI device information struct
@@ -8223,6 +8253,10 @@ skip_sriov:
goto err_sw_init;
}
+#ifdef CONFIG_OF
+ ixgbe_of_mac_addr(adapter);
+#endif
+
memcpy(netdev->dev_addr, hw->mac.perm_addr, netdev->addr_len);
if (!is_valid_ether_addr(netdev->dev_addr)) {
^ permalink raw reply related
* Re: [PATCH 1/1 net-next] net: dsa: replace count*size kmalloc by kmalloc_array
From: Joe Perches @ 2014-11-14 19:14 UTC (permalink / raw)
To: Fabian Frederick; +Cc: netdev, linux-kernel, David S. Miller
In-Reply-To: <2104595537.56209.1415991758726.open-xchange@webmail.nmp.skynet.be>
On Fri, 2014-11-14 at 20:02 +0100, Fabian Frederick wrote:
> On 14 November 2014 at 19:47 Joe Perches <joe@perches.com> wrote:
> > On Fri, 2014-11-14 at 19:36 +0100, Fabian Frederick wrote:
> > > kmalloc_array manages count*sizeof overflow.
> >
> > Fundamentally correct, but is this necessary or useful?
> > sizeof(s8) isn't often going to be anything other than 1.
> Absolutely, I thought it was a struct :)
>
> There must be a reason for so many cases though ...
Some might be style symmetry for other sizeof(othertype)
uses in the same paths, but most of them are just overkill
or maybe lack of understanding.
s8 is char so by definition it has to be 1.
I doubt any of the code dates from PDP-8/TOPS-10 days.
^ permalink raw reply
* [PATCH net-next] Revert "fast_hash: avoid indirect function calls"
From: Jay Vosburgh @ 2014-11-14 19:05 UTC (permalink / raw)
To: David Miller
Cc: hannes, eric.dumazet, netdev, ogerlitz, pshelar, jesse, discuss
In-Reply-To: <20141114.133829.1437047454714311242.davem@davemloft.net>
This reverts commit e5a2c899957659cd1a9f789bc462f9c0b35f5150.
Commit e5a2c899 introduced an alternative_call, arch_fast_hash2,
that selects between __jhash2 and __intel_crc4_2_hash based on the
X86_FEATURE_XMM4_2.
Unfortunately, the alternative_call system does not appear to be
suitable for use with C functions, as register usage is not handled
properly for the called functions. The __jhash2 function in particular
clobbers registers that are not preserved when called via
alternative_call, resulting in a panic for direct callers of
arch_fast_hash2 on older CPUs lacking sse4_2. It is possible that
__intel_crc4_2_hash works merely by chance because it uses fewer
registers.
This commit was suggested as the source of the problem by Jesse
Gross <jesse@nicira.com>.
Signed-off-by: Jay Vosburgh <jay.vosburgh@canonical.com>
---
arch/x86/include/asm/hash.h | 51 +++++----------------------------------------
arch/x86/lib/hash.c | 29 +++++++++++---------------
include/asm-generic/hash.h | 36 ++------------------------------
include/linux/hash.h | 34 ++++++++++++++++++++++++++++++
lib/Makefile | 2 +-
lib/hash.c | 39 ++++++++++++++++++++++++++++++++++
6 files changed, 93 insertions(+), 98 deletions(-)
create mode 100644 lib/hash.c
diff --git a/arch/x86/include/asm/hash.h b/arch/x86/include/asm/hash.h
index a881d78..e8c58f8 100644
--- a/arch/x86/include/asm/hash.h
+++ b/arch/x86/include/asm/hash.h
@@ -1,48 +1,7 @@
-#ifndef __ASM_X86_HASH_H
-#define __ASM_X86_HASH_H
+#ifndef _ASM_X86_HASH_H
+#define _ASM_X86_HASH_H
-#include <linux/cpufeature.h>
-#include <asm/alternative.h>
+struct fast_hash_ops;
+extern void setup_arch_fast_hash(struct fast_hash_ops *ops);
-u32 __intel_crc4_2_hash(const void *data, u32 len, u32 seed);
-u32 __intel_crc4_2_hash2(const u32 *data, u32 len, u32 seed);
-
-/*
- * non-inline versions of jhash so gcc does not need to generate
- * duplicate code in every object file
- */
-u32 __jhash(const void *data, u32 len, u32 seed);
-u32 __jhash2(const u32 *data, u32 len, u32 seed);
-
-/*
- * for documentation of these functions please look into
- * <include/asm-generic/hash.h>
- */
-
-static inline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
-{
- u32 hash;
-
- alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
-#ifdef CONFIG_X86_64
- "=a" (hash), "D" (data), "S" (len), "d" (seed));
-#else
- "=a" (hash), "a" (data), "d" (len), "c" (seed));
-#endif
- return hash;
-}
-
-static inline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
-{
- u32 hash;
-
- alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
-#ifdef CONFIG_X86_64
- "=a" (hash), "D" (data), "S" (len), "d" (seed));
-#else
- "=a" (hash), "a" (data), "d" (len), "c" (seed));
-#endif
- return hash;
-}
-
-#endif /* __ASM_X86_HASH_H */
+#endif /* _ASM_X86_HASH_H */
diff --git a/arch/x86/lib/hash.c b/arch/x86/lib/hash.c
index e143271..ff4fa51 100644
--- a/arch/x86/lib/hash.c
+++ b/arch/x86/lib/hash.c
@@ -31,13 +31,13 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
+#include <linux/hash.h>
+#include <linux/init.h>
+
#include <asm/processor.h>
#include <asm/cpufeature.h>
#include <asm/hash.h>
-#include <linux/hash.h>
-#include <linux/jhash.h>
-
static inline u32 crc32_u32(u32 crc, u32 val)
{
#ifdef CONFIG_AS_CRC32
@@ -48,7 +48,7 @@ static inline u32 crc32_u32(u32 crc, u32 val)
return crc;
}
-u32 __intel_crc4_2_hash(const void *data, u32 len, u32 seed)
+static u32 intel_crc4_2_hash(const void *data, u32 len, u32 seed)
{
const u32 *p32 = (const u32 *) data;
u32 i, tmp = 0;
@@ -71,27 +71,22 @@ u32 __intel_crc4_2_hash(const void *data, u32 len, u32 seed)
return seed;
}
-EXPORT_SYMBOL(__intel_crc4_2_hash);
-u32 __intel_crc4_2_hash2(const u32 *data, u32 len, u32 seed)
+static u32 intel_crc4_2_hash2(const u32 *data, u32 len, u32 seed)
{
+ const u32 *p32 = (const u32 *) data;
u32 i;
for (i = 0; i < len; i++)
- seed = crc32_u32(seed, *data++);
+ seed = crc32_u32(seed, *p32++);
return seed;
}
-EXPORT_SYMBOL(__intel_crc4_2_hash2);
-u32 __jhash(const void *data, u32 len, u32 seed)
+void __init setup_arch_fast_hash(struct fast_hash_ops *ops)
{
- return jhash(data, len, seed);
-}
-EXPORT_SYMBOL(__jhash);
-
-u32 __jhash2(const u32 *data, u32 len, u32 seed)
-{
- return jhash2(data, len, seed);
+ if (cpu_has_xmm4_2) {
+ ops->hash = intel_crc4_2_hash;
+ ops->hash2 = intel_crc4_2_hash2;
+ }
}
-EXPORT_SYMBOL(__jhash2);
diff --git a/include/asm-generic/hash.h b/include/asm-generic/hash.h
index 3c82760..b631284 100644
--- a/include/asm-generic/hash.h
+++ b/include/asm-generic/hash.h
@@ -1,41 +1,9 @@
#ifndef __ASM_GENERIC_HASH_H
#define __ASM_GENERIC_HASH_H
-#include <linux/jhash.h>
-
-/**
- * arch_fast_hash - Caclulates a hash over a given buffer that can have
- * arbitrary size. This function will eventually use an
- * architecture-optimized hashing implementation if
- * available, and trades off distribution for speed.
- *
- * @data: buffer to hash
- * @len: length of buffer in bytes
- * @seed: start seed
- *
- * Returns 32bit hash.
- */
-static inline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
-{
- return jhash(data, len, seed);
-}
-
-/**
- * arch_fast_hash2 - Caclulates a hash over a given buffer that has a
- * size that is of a multiple of 32bit words. This
- * function will eventually use an architecture-
- * optimized hashing implementation if available,
- * and trades off distribution for speed.
- *
- * @data: buffer to hash (must be 32bit padded)
- * @len: number of 32bit words
- * @seed: start seed
- *
- * Returns 32bit hash.
- */
-static inline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
+struct fast_hash_ops;
+static inline void setup_arch_fast_hash(struct fast_hash_ops *ops)
{
- return jhash2(data, len, seed);
}
#endif /* __ASM_GENERIC_HASH_H */
diff --git a/include/linux/hash.h b/include/linux/hash.h
index 6e8fb02..d0494c3 100644
--- a/include/linux/hash.h
+++ b/include/linux/hash.h
@@ -84,4 +84,38 @@ static inline u32 hash32_ptr(const void *ptr)
return (u32)val;
}
+struct fast_hash_ops {
+ u32 (*hash)(const void *data, u32 len, u32 seed);
+ u32 (*hash2)(const u32 *data, u32 len, u32 seed);
+};
+
+/**
+ * arch_fast_hash - Caclulates a hash over a given buffer that can have
+ * arbitrary size. This function will eventually use an
+ * architecture-optimized hashing implementation if
+ * available, and trades off distribution for speed.
+ *
+ * @data: buffer to hash
+ * @len: length of buffer in bytes
+ * @seed: start seed
+ *
+ * Returns 32bit hash.
+ */
+extern u32 arch_fast_hash(const void *data, u32 len, u32 seed);
+
+/**
+ * arch_fast_hash2 - Caclulates a hash over a given buffer that has a
+ * size that is of a multiple of 32bit words. This
+ * function will eventually use an architecture-
+ * optimized hashing implementation if available,
+ * and trades off distribution for speed.
+ *
+ * @data: buffer to hash (must be 32bit padded)
+ * @len: number of 32bit words
+ * @seed: start seed
+ *
+ * Returns 32bit hash.
+ */
+extern u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed);
+
#endif /* _LINUX_HASH_H */
diff --git a/lib/Makefile b/lib/Makefile
index 04e53dd..7512dc9 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -26,7 +26,7 @@ obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \
gcd.o lcm.o list_sort.o uuid.o flex_array.o iovec.o clz_ctz.o \
bsearch.o find_last_bit.o find_next_bit.o llist.o memweight.o kfifo.o \
- percpu-refcount.o percpu_ida.o rhashtable.o
+ percpu-refcount.o percpu_ida.o hash.o rhashtable.o
obj-y += string_helpers.o
obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
obj-y += kstrtox.o
diff --git a/lib/hash.c b/lib/hash.c
new file mode 100644
index 0000000..fea973f
--- /dev/null
+++ b/lib/hash.c
@@ -0,0 +1,39 @@
+/* General purpose hashing library
+ *
+ * That's a start of a kernel hashing library, which can be extended
+ * with further algorithms in future. arch_fast_hash{2,}() will
+ * eventually resolve to an architecture optimized implementation.
+ *
+ * Copyright 2013 Francesco Fusco <ffusco@redhat.com>
+ * Copyright 2013 Daniel Borkmann <dborkman@redhat.com>
+ * Copyright 2013 Thomas Graf <tgraf@redhat.com>
+ * Licensed under the GNU General Public License, version 2.0 (GPLv2)
+ */
+
+#include <linux/jhash.h>
+#include <linux/hash.h>
+#include <linux/cache.h>
+
+static struct fast_hash_ops arch_hash_ops __read_mostly = {
+ .hash = jhash,
+ .hash2 = jhash2,
+};
+
+u32 arch_fast_hash(const void *data, u32 len, u32 seed)
+{
+ return arch_hash_ops.hash(data, len, seed);
+}
+EXPORT_SYMBOL_GPL(arch_fast_hash);
+
+u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
+{
+ return arch_hash_ops.hash2(data, len, seed);
+}
+EXPORT_SYMBOL_GPL(arch_fast_hash2);
+
+static int __init hashlib_init(void)
+{
+ setup_arch_fast_hash(&arch_hash_ops);
+ return 0;
+}
+early_initcall(hashlib_init);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 1/1 net-next] net: dsa: replace count*size kmalloc by kmalloc_array
From: Fabian Frederick @ 2014-11-14 19:02 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, linux-kernel, David S. Miller
In-Reply-To: <1415990835.5912.20.camel@perches.com>
> On 14 November 2014 at 19:47 Joe Perches <joe@perches.com> wrote:
>
>
> On Fri, 2014-11-14 at 19:36 +0100, Fabian Frederick wrote:
> > kmalloc_array manages count*sizeof overflow.
>
> Fundamentally correct, but is this necessary or useful?
> sizeof(s8) isn't often going to be anything other than 1.
Absolutely, I thought it was a struct :)
There must be a reason for so many cases though ...
Regards,
Fabian
>
> Would the kernel even work without that assumption?
>
>
> > diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> []
> > @@ -526,7 +526,8 @@ static int dsa_of_setup_routing_table(struct
> > dsa_platform_data *pd,
> >
> > /* First time routing table allocation */
> > if (!cd->rtable) {
> > - cd->rtable = kmalloc(pd->nr_chips * sizeof(s8), GFP_KERNEL);
> > + cd->rtable = kmalloc_array(pd->nr_chips, sizeof(s8),
> > + GFP_KERNEL);
> > if (!cd->rtable)
> > return -ENOMEM;
> >
>
> Maybe all of these could be simplified
>
> $ git grep -E "\*\s*sizeof\s*\(\s*[us]8\s*\)"
> arch/arm/common/edma.c: (edma_cc->num_tc +
> 1) * sizeof(s8),
> drivers/acpi/utils.c: (element->buffer.length *
> sizeof(u8));
> drivers/acpi/utils.c: tail += element->buffer.length
> * sizeof(u8);
> drivers/char/tpm/tpm_i2c_stm_st33.c: kmalloc(TPM_BUFSIZE * sizeof(u8),
> GFP_KERNEL);
> drivers/char/tpm/tpm_i2c_stm_st33.c: kmalloc(TPM_BUFSIZE * sizeof(u8),
> GFP_KERNEL);
> drivers/gpu/drm/r128/r128_state.c: mask_size = depth->n * sizeof(u8);
> drivers/gpu/drm/r128/r128_state.c: mask_size = depth->n *
> sizeof(u8);
> drivers/iio/common/st_sensors/st_sensors_spi.c: memcpy(data, tb->rx_buf,
> len*sizeof(u8));
> drivers/infiniband/hw/amso1100/c2_mq.h: u8 pad[64 - sizeof(u16) - 2 *
> sizeof(u8) - sizeof(u32) - sizeof(u16)];
> drivers/input/tablet/aiptek.c: const int sizeof_buf = 3 * sizeof(u8);
> drivers/input/tablet/aiptek.c: const int sizeof_buf = 3 * sizeof(u8);
> drivers/md/dm-crypt.c: memset(&cc->key, 0, cc->key_size * sizeof(u8));
> drivers/md/dm-crypt.c: cc = kzalloc(sizeof(*cc) + key_size * sizeof(u8),
> GFP_KERNEL);
> drivers/media/dvb-frontends/dib7000p.c: tx = kzalloc(2*sizeof(u8),
> GFP_KERNEL);
> drivers/media/dvb-frontends/dib7000p.c: rx = kzalloc(2*sizeof(u8),
> GFP_KERNEL);
> drivers/media/dvb-frontends/dib8000.c: client.i2c_write_buffer = kzalloc(4 *
> sizeof(u8), GFP_KERNEL);
> drivers/media/dvb-frontends/dib8000.c: client.i2c_read_buffer = kzalloc(4 *
> sizeof(u8), GFP_KERNEL);
> drivers/media/dvb-frontends/dib9000.c: client.i2c_write_buffer = kzalloc(4 *
> sizeof(u8), GFP_KERNEL);
> drivers/media/dvb-frontends/dib9000.c: client.i2c_read_buffer = kzalloc(4 *
> sizeof(u8), GFP_KERNEL);
> drivers/media/pci/ttpci/av7110_ipack.c: if (!(p->buf =
> vmalloc(size*sizeof(u8)))) {
> drivers/mtd/inftlmount.c: s->nb_blocks *
> sizeof(u8));
> drivers/net/wireless/ath/ath10k/htt.h: * b) num_chars * sizeof(u8) aligned
> to 4bytes */
> drivers/net/wireless/b43/ppr.c: BUILD_BUG_ON(sizeof(struct b43_ppr) !=
> B43_PPR_RATES_NUM * sizeof(u8));
> drivers/net/wireless/iwlwifi/pcie/trans.c:
> trans_pcie->n_no_reclaim_cmds * sizeof(u8));
> drivers/net/wireless/rtlwifi/efuse.c: memset(data, 0xff, PGPKT_DATA_SIZE *
> sizeof(u8));
> drivers/net/wireless/rtlwifi/efuse.c: memset(tmpdata, 0xff, PGPKT_DATA_SIZE
> * sizeof(u8));
> drivers/net/wireless/rtlwifi/efuse.c: u8 originaldata[8 * sizeof(u8)];
> drivers/net/wireless/rtlwifi/efuse.c: u8 originaldata[8 * sizeof(u8)];
> drivers/net/wireless/rtlwifi/efuse.c: memset(originaldata, 0xff, 8
> * sizeof(u8));
> drivers/net/wireless/rtlwifi/efuse.c: memset(target_pkt.data, 0xFF, 8 *
> sizeof(u8));
> drivers/power/ds2781_battery.c: ret = w1_ds2781_read(dev_info, val,
> DS2781_VOLT_MSB, 2 * sizeof(u8));
> drivers/power/ds2781_battery.c: ret = w1_ds2781_read(dev_info, val,
> DS2781_TEMP_MSB, 2 * sizeof(u8));
> drivers/rtc/rtc-pcf2123.c: ret = spi_write(spi, txbuf, 2 * sizeof(u8));
> drivers/rtc/rtc-pcf2123.c: ret = spi_write(spi, txbuf, 2 * sizeof(u8));
> drivers/rtc/rtc-pcf2123.c: ret = spi_write_then_read(spi, txbuf, 1 *
> sizeof(u8),
> drivers/rtc/rtc-pcf2123.c: rxbuf, 2 *
> sizeof(u8));
> drivers/thermal/x86_pkg_temp_thermal.c: (max_phy_id+1) *
> sizeof(u8), GFP_ATOMIC);
> fs/compat_ioctl.c: if (__copy_in_user(&tdata->read_write,
> &udata->read_write, 2 * sizeof(u8)))
> net/dsa/dsa.c: cd->rtable = kmalloc(pd->nr_chips * sizeof(s8),
> GFP_KERNEL);
> net/dsa/dsa.c: memset(cd->rtable, -1, pd->nr_chips * sizeof(s8));
>
>
^ permalink raw reply
* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Cong Wang @ 2014-11-14 19:02 UTC (permalink / raw)
To: David Miller
Cc: Hannes Frederic Sowa, Eric Dumazet, netdev, Or Gerlitz,
Pravin B Shelar, Jesse Gross, jay.vosburgh, discuss
In-Reply-To: <20141114.133829.1437047454714311242.davem@davemloft.net>
On Fri, Nov 14, 2014 at 10:38 AM, David Miller <davem@davemloft.net> wrote:
>
> Also, I am strongly of the opinion that this is a mis-use of the
> alternative call interface. It was never intended to be used for
> things that can make real function calls.
>
> You can add a million clobbers, or a trampoline, it's still using a
> facility in a manner for which it was not designed.
>
> This means a new interface with a new name and with capabilities
> explicitly supporting this case are in order.
>
I am wondering, compare with alternative call, how slower is just testing
cpu_has_xmm4_2?
^ permalink raw reply
* Re: [PATCH 1/1 net-next] net: dsa: replace count*size kmalloc by kmalloc_array
From: Joe Perches @ 2014-11-14 18:47 UTC (permalink / raw)
To: Fabian Frederick; +Cc: linux-kernel, David S. Miller, netdev
In-Reply-To: <1415990202-28673-1-git-send-email-fabf@skynet.be>
On Fri, 2014-11-14 at 19:36 +0100, Fabian Frederick wrote:
> kmalloc_array manages count*sizeof overflow.
Fundamentally correct, but is this necessary or useful?
sizeof(s8) isn't often going to be anything other than 1.
Would the kernel even work without that assumption?
> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
[]
> @@ -526,7 +526,8 @@ static int dsa_of_setup_routing_table(struct dsa_platform_data *pd,
>
> /* First time routing table allocation */
> if (!cd->rtable) {
> - cd->rtable = kmalloc(pd->nr_chips * sizeof(s8), GFP_KERNEL);
> + cd->rtable = kmalloc_array(pd->nr_chips, sizeof(s8),
> + GFP_KERNEL);
> if (!cd->rtable)
> return -ENOMEM;
>
Maybe all of these could be simplified
$ git grep -E "\*\s*sizeof\s*\(\s*[us]8\s*\)"
arch/arm/common/edma.c: (edma_cc->num_tc + 1) * sizeof(s8),
drivers/acpi/utils.c: (element->buffer.length * sizeof(u8));
drivers/acpi/utils.c: tail += element->buffer.length * sizeof(u8);
drivers/char/tpm/tpm_i2c_stm_st33.c: kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
drivers/char/tpm/tpm_i2c_stm_st33.c: kmalloc(TPM_BUFSIZE * sizeof(u8), GFP_KERNEL);
drivers/gpu/drm/r128/r128_state.c: mask_size = depth->n * sizeof(u8);
drivers/gpu/drm/r128/r128_state.c: mask_size = depth->n * sizeof(u8);
drivers/iio/common/st_sensors/st_sensors_spi.c: memcpy(data, tb->rx_buf, len*sizeof(u8));
drivers/infiniband/hw/amso1100/c2_mq.h: u8 pad[64 - sizeof(u16) - 2 * sizeof(u8) - sizeof(u32) - sizeof(u16)];
drivers/input/tablet/aiptek.c: const int sizeof_buf = 3 * sizeof(u8);
drivers/input/tablet/aiptek.c: const int sizeof_buf = 3 * sizeof(u8);
drivers/md/dm-crypt.c: memset(&cc->key, 0, cc->key_size * sizeof(u8));
drivers/md/dm-crypt.c: cc = kzalloc(sizeof(*cc) + key_size * sizeof(u8), GFP_KERNEL);
drivers/media/dvb-frontends/dib7000p.c: tx = kzalloc(2*sizeof(u8), GFP_KERNEL);
drivers/media/dvb-frontends/dib7000p.c: rx = kzalloc(2*sizeof(u8), GFP_KERNEL);
drivers/media/dvb-frontends/dib8000.c: client.i2c_write_buffer = kzalloc(4 * sizeof(u8), GFP_KERNEL);
drivers/media/dvb-frontends/dib8000.c: client.i2c_read_buffer = kzalloc(4 * sizeof(u8), GFP_KERNEL);
drivers/media/dvb-frontends/dib9000.c: client.i2c_write_buffer = kzalloc(4 * sizeof(u8), GFP_KERNEL);
drivers/media/dvb-frontends/dib9000.c: client.i2c_read_buffer = kzalloc(4 * sizeof(u8), GFP_KERNEL);
drivers/media/pci/ttpci/av7110_ipack.c: if (!(p->buf = vmalloc(size*sizeof(u8)))) {
drivers/mtd/inftlmount.c: s->nb_blocks * sizeof(u8));
drivers/net/wireless/ath/ath10k/htt.h: * b) num_chars * sizeof(u8) aligned to 4bytes */
drivers/net/wireless/b43/ppr.c: BUILD_BUG_ON(sizeof(struct b43_ppr) != B43_PPR_RATES_NUM * sizeof(u8));
drivers/net/wireless/iwlwifi/pcie/trans.c: trans_pcie->n_no_reclaim_cmds * sizeof(u8));
drivers/net/wireless/rtlwifi/efuse.c: memset(data, 0xff, PGPKT_DATA_SIZE * sizeof(u8));
drivers/net/wireless/rtlwifi/efuse.c: memset(tmpdata, 0xff, PGPKT_DATA_SIZE * sizeof(u8));
drivers/net/wireless/rtlwifi/efuse.c: u8 originaldata[8 * sizeof(u8)];
drivers/net/wireless/rtlwifi/efuse.c: u8 originaldata[8 * sizeof(u8)];
drivers/net/wireless/rtlwifi/efuse.c: memset(originaldata, 0xff, 8 * sizeof(u8));
drivers/net/wireless/rtlwifi/efuse.c: memset(target_pkt.data, 0xFF, 8 * sizeof(u8));
drivers/power/ds2781_battery.c: ret = w1_ds2781_read(dev_info, val, DS2781_VOLT_MSB, 2 * sizeof(u8));
drivers/power/ds2781_battery.c: ret = w1_ds2781_read(dev_info, val, DS2781_TEMP_MSB, 2 * sizeof(u8));
drivers/rtc/rtc-pcf2123.c: ret = spi_write(spi, txbuf, 2 * sizeof(u8));
drivers/rtc/rtc-pcf2123.c: ret = spi_write(spi, txbuf, 2 * sizeof(u8));
drivers/rtc/rtc-pcf2123.c: ret = spi_write_then_read(spi, txbuf, 1 * sizeof(u8),
drivers/rtc/rtc-pcf2123.c: rxbuf, 2 * sizeof(u8));
drivers/thermal/x86_pkg_temp_thermal.c: (max_phy_id+1) * sizeof(u8), GFP_ATOMIC);
fs/compat_ioctl.c: if (__copy_in_user(&tdata->read_write, &udata->read_write, 2 * sizeof(u8)))
net/dsa/dsa.c: cd->rtable = kmalloc(pd->nr_chips * sizeof(s8), GFP_KERNEL);
net/dsa/dsa.c: memset(cd->rtable, -1, pd->nr_chips * sizeof(s8));
^ permalink raw reply
* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: David Miller @ 2014-11-14 18:38 UTC (permalink / raw)
To: hannes
Cc: eric.dumazet, netdev, ogerlitz, pshelar, jesse, jay.vosburgh,
discuss
In-Reply-To: <1415979978.15154.41.camel@localhost>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Fri, 14 Nov 2014 16:46:18 +0100
> I would still like to see the current proposed fix getting applied and
> we can do this on-top. The inline call after this patch reassembles a
> direct function call, so besides the long list of clobbers, it should
> still be pretty fast.
I would rather revert the change entirely until it is implemented
properly.
Also, I am strongly of the opinion that this is a mis-use of the
alternative call interface. It was never intended to be used for
things that can make real function calls.
You can add a million clobbers, or a trampoline, it's still using a
facility in a manner for which it was not designed.
This means a new interface with a new name and with capabilities
explicitly supporting this case are in order.
Thanks.
^ permalink raw reply
* [PATCH 1/1 net-next] net: dsa: replace count*size kzalloc by kcalloc
From: Fabian Frederick @ 2014-11-14 18:38 UTC (permalink / raw)
To: linux-kernel; +Cc: Fabian Frederick, David S. Miller, netdev
kcalloc manages count*sizeof overflow.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/dsa/dsa.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 4648f12..e84b656 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -606,8 +606,8 @@ static int dsa_of_probe(struct platform_device *pdev)
if (pd->nr_chips > DSA_MAX_SWITCHES)
pd->nr_chips = DSA_MAX_SWITCHES;
- pd->chip = kzalloc(pd->nr_chips * sizeof(struct dsa_chip_data),
- GFP_KERNEL);
+ pd->chip = kcalloc(pd->nr_chips, sizeof(struct dsa_chip_data),
+ GFP_KERNEL);
if (!pd->chip) {
ret = -ENOMEM;
goto out_free;
--
1.9.3
^ permalink raw reply related
* [PATCH 1/1 net-next] net: dsa: replace count*size kmalloc by kmalloc_array
From: Fabian Frederick @ 2014-11-14 18:36 UTC (permalink / raw)
To: linux-kernel; +Cc: Fabian Frederick, David S. Miller, netdev
kmalloc_array manages count*sizeof overflow.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/dsa/dsa.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 4648f12..c00cca3 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -526,7 +526,8 @@ static int dsa_of_setup_routing_table(struct dsa_platform_data *pd,
/* First time routing table allocation */
if (!cd->rtable) {
- cd->rtable = kmalloc(pd->nr_chips * sizeof(s8), GFP_KERNEL);
+ cd->rtable = kmalloc_array(pd->nr_chips, sizeof(s8),
+ GFP_KERNEL);
if (!cd->rtable)
return -ENOMEM;
--
1.9.3
^ permalink raw reply related
* [PATCH 1/1 net-next] Bluetooth: hidp: replace kzalloc/copy_from_user by memdup_user
From: Fabian Frederick @ 2014-11-14 18:35 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, Marcel Holtmann, Gustavo Padovan, Johan Hedberg,
David S. Miller, linux-bluetooth, netdev
use memdup_user for rd_data import.
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/bluetooth/hidp/core.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 1b7d605..cc25d0b 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -736,14 +736,10 @@ static int hidp_setup_hid(struct hidp_session *session,
struct hid_device *hid;
int err;
- session->rd_data = kzalloc(req->rd_size, GFP_KERNEL);
- if (!session->rd_data)
- return -ENOMEM;
+ session->rd_data = memdup_user(req->rd_data, req->rd_size);
+ if (IS_ERR(session->rd_data))
+ return PTR_ERR(session->rd_data);
- if (copy_from_user(session->rd_data, req->rd_data, req->rd_size)) {
- err = -EFAULT;
- goto fault;
- }
session->rd_size = req->rd_size;
hid = hid_allocate_device();
--
1.9.3
^ permalink raw reply related
* [PATCH 1/1 net-next] openvswitch: use PTR_ERR_OR_ZERO
From: Fabian Frederick @ 2014-11-14 18:32 UTC (permalink / raw)
To: linux-kernel
Cc: Fabian Frederick, Pravin Shelar, David S. Miller, dev, netdev
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
net/openvswitch/flow_netlink.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index c0d066d..c8fccdd 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -1425,10 +1425,8 @@ static int add_action(struct sw_flow_actions **sfa, int attrtype,
struct nlattr *a;
a = __add_action(sfa, attrtype, data, len, log);
- if (IS_ERR(a))
- return PTR_ERR(a);
- return 0;
+ return PTR_ERR_OR_ZERO(a);
}
static inline int add_nested_action_start(struct sw_flow_actions **sfa,
--
1.9.3
^ permalink raw reply related
* Re: [PATCH net-next] icmp: Remove some spurious dropped packet profile hits from the ICMP path
From: Rick Jones @ 2014-11-14 18:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, davem
In-Reply-To: <1415984329.17262.47.camel@edumazet-glaptop2.roam.corp.google.com>
On 11/14/2014 08:58 AM, Eric Dumazet wrote:
> On Fri, 2014-11-14 at 08:16 -0800, Rick Jones wrote:
>
>> I thought the point of the drop profiling was to show where the drops
>> were happening. Leaving the kfree_skb() up in icmp_rcv() does not
>> improve showing where the drops happened. That is why I've pushed it
>> down into the routines called by icmp_rcv().
>
> OK, but we drop an icmp message, and that really should be enough.
>
> The point is that most normal icmp messages wont be dropped, but
> consumed.
>
> I am not sure we want to bloat the kernel for such minor problem.
I can certainly rework the patch that way, but one thing I have noticed
when the system is the initiator of pings rather than the target, is
that I get (or at least I think I do) drop profile hits in ping_rcv():
# To display the perf.data header info, please use
--header/--header-only options.
#
# Samples: 997K of event 'skb:kfree_skb'
# Event count (approx.): 997789
#
# Children Self Symbol Shared
Object
# ........ ........ ..........................................
...........................
#
100.00% 100.00% [k] kfree_skb
[kernel.kallsyms]
|
|--100.00%-- ping_rcv
| icmp_rcv
| ip_local_deliver_finish
| ip_local_deliver
| ip_rcv_finish
| ip_rcv
| __netif_receive_skb_core
| __netif_receive_skb
| netif_receive_skb_internal
| napi_gro_receive
| e1000_receive_skb
| e1000_clean_rx_irq
| e1000e_poll
| net_rx_action
| __do_softirq
I don't have an explanation for it though. Perhaps it is just confusion
on my part.
That is from:
raj@raj-8510w:~$ sudo ~/net-next/tools/perf/perf record -a -g -e
skb:kfree_skb ping -n -f -q tardy.usa.hp.com -c 1000000
PING tardy.usa.hp.com (16.103.148.51) 56(84) bytes of data.
--- tardy.usa.hp.com ping statistics ---
1000000 packets transmitted, 1000000 received, 0% packet loss, time 65751ms
rtt min/avg/max/mdev = 0.036/0.053/0.170/0.008 ms, ipg/ewma 0.065/0.052 ms
[ perf record: Woken up 1037 times to write data ]
[ perf record: Captured and wrote 259.456 MB perf.data (~11335798 samples) ]
Warning:
Processed 1002854 events and lost 2 chunks!
Check IO/CPU overload!
^ permalink raw reply
* Re: [PATCH 1/1] drivers: net: cpsw: Fix TX_IN_SEL offset
From: Mugunthan V N @ 2014-11-14 18:23 UTC (permalink / raw)
To: John Ogness, linux-kernel
Cc: davem, balbi, george.cherian, jhovold, mpa, bhutchings, zonque,
tklauser, netdev
In-Reply-To: <87sihlj11v.fsf@linutronix.de>
On Friday 14 November 2014 08:12 PM, John Ogness wrote:
> The TX_IN_SEL offset for the CPSW_PORT/TX_IN_CTL register was
> incorrect. This caused the Dual MAC mode to never get set when
> it should. It also caused possible unintentional setting of a
> bit in the CPSW_PORT/TX_BLKS_REM register.
>
> The purpose of setting the Dual MAC mode for this register is to:
>
> "... allow packets from both ethernet ports to be written into
> the FIFO without one port starving the other port."
> - AM335x ARM TRM
>
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>
Regards
Mugunthan V N
^ permalink raw reply
* Re: [PATCH] e100: fix typo in MDI/MDI-X eeprom check in e100_phy_init
From: John W. Linville @ 2014-11-14 17:58 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: netdev, Dave Miller, Auke Kok, Malli Chilakala
In-Reply-To: <CAL3LdT5yEg1MqDyigzFpfPkpFv8ST09uPYSEwqaKAypEvDwHeQ@mail.gmail.com>
On Fri, Nov 14, 2014 at 09:17:39AM -0800, Jeff Kirsher wrote:
> On Fri, Nov 14, 2014 at 7:59 AM, John W. Linville
> <linville@tuxdriver.com> wrote:
> > Although it doesn't explicitly say so, commit 60ffa478759f39a2 ("e100:
> > Fix MDIO/MDIO-X") appears to be intended to revert the earlier commit
> > 648951451e6d2d53 ("e100: fixed e100 MDI/MDI-X issues"). However,
> > careful examination reveals that the attempted revert actually
> > _inverted_ the test for eeprom_mdix_enabled. That is bound to program
> > a few PHYs incorrectly...
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=1156417
> >
> > Signed-off-by: John W. Linville <linville@tuxdriver.com>
> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Cc: Auke Kok <auke-jan.h.kok@intel.com>
> > Cc: Malli Chilakala <mallikarjuna.chilakala@intel.com>
> > ---
> > Wow, an 8 year old bug in e100 -- woohoo!! :-)
> >
> > This was causing some serious flakiness for a large cash register
> > deployment in Europe. Testing with this one-line (really,
> > one-character) patch seems to have resolved the issue.
> >
> > drivers/net/ethernet/intel/e100.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Weird, I did not get this mail. Anyway, thanks John, I have added
> your patch to my queue.
I got a "no such address" bounce on all the @intel.com addresses,
but they all seem OK to me -- not sure what the issue is...?
Anyway, glad you got it! :-)
John
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [PATCH net-next v3] fast_hash: clobber registers correctly for inline function use
From: Jay Vosburgh @ 2014-11-14 17:57 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev, ogerlitz, pshelar, jesse, discuss
In-Reply-To: <6751d6af4301f283134a419385f65dfcf92a44ab.1415978153.git.hannes@stressinduktion.org>
Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:
>In case the arch_fast_hash call gets inlined we need to tell gcc which
>registers are clobbered with. rhashtable was fine, because it used
>arch_fast_hash via function pointer and thus the compiler took care of
>that. In case of openvswitch the call got inlined and arch_fast_hash
>touched registeres which gcc didn't know about.
>
>Also don't use conditional compilation inside arguments, as this confuses
>sparse.
>
>Fixes: e5a2c899957659c ("fast_hash: avoid indirect function calls")
>Reported-by: Jay Vosburgh <jay.vosburgh@canonical.com>
>Cc: Pravin Shelar <pshelar@nicira.com>
>Cc: Jesse Gross <jesse@nicira.com>
>Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>---
>
>
>v2)
>After studying gcc documentation again, it occured to me that I need to
>specificy all input operands in the clobber section, too. Otherwise gcc
>can expect that the inline assembler section won't modify the inputs,
>which is not true.
>
>v3)
>added Fixes tag
This patch does not compile for me when applied to today's
net-next:
CC [M] fs/nfsd/nfs4state.o
In file included from ./arch/x86/include/asm/bitops.h:16:0,
from include/linux/bitops.h:33,
from include/linux/kernel.h:10,
from include/linux/list.h:8,
from include/linux/wait.h:6,
from include/linux/fs.h:6,
from fs/nfsd/nfs4state.c:36:
fs/nfsd/nfs4state.c: In function ‘nfsd4_cb_recall_prepare’:
./arch/x86/include/asm/alternative.h:185:2: error: ‘asm’ operand has impossible constraints
asm volatile (ALTERNATIVE("call %P[old]", "call %P[new]", feature) \
^
./arch/x86/include/asm/hash.h:27:2: note: in expansion of macro ‘alternative_call’
alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
^
make[2]: *** [fs/nfsd/nfs4state.o] Error 1
make[1]: *** [fs/nfsd] Error 2
make: *** [fs] Error 2
-J
>Bye,
>Hannes
>
> arch/x86/include/asm/hash.h | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
>diff --git a/arch/x86/include/asm/hash.h b/arch/x86/include/asm/hash.h
>index a881d78..a25c45a 100644
>--- a/arch/x86/include/asm/hash.h
>+++ b/arch/x86/include/asm/hash.h
>@@ -23,11 +23,15 @@ static inline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
> {
> u32 hash;
>
>- alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
> #ifdef CONFIG_X86_64
>- "=a" (hash), "D" (data), "S" (len), "d" (seed));
>+ alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
>+ "=a" (hash), "D" (data), "S" (len), "d" (seed)
>+ : "rdi", "rsi", "rdx", "rcx", "r8", "r9", "r10", "r11",
>+ "cc", "memory");
> #else
>- "=a" (hash), "a" (data), "d" (len), "c" (seed));
>+ alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
>+ "=a" (hash), "a" (data), "d" (len), "c" (seed)
>+ : "edx", "ecx", "cc", "memory");
> #endif
> return hash;
> }
>@@ -36,11 +40,15 @@ static inline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
> {
> u32 hash;
>
>- alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
> #ifdef CONFIG_X86_64
>- "=a" (hash), "D" (data), "S" (len), "d" (seed));
>+ alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
>+ "=a" (hash), "D" (data), "S" (len), "d" (seed)
>+ : "rdi", "rsi", "rdx", "rcx", "r8", "r9", "r10", "r11",
>+ "cc", "memory");
> #else
>- "=a" (hash), "a" (data), "d" (len), "c" (seed));
>+ alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
>+ "=a" (hash), "a" (data), "d" (len), "c" (seed)
>+ : "edx", "ecx", "cc", "memory");
> #endif
> return hash;
> }
>--
>1.9.3
---
-Jay Vosburgh, jay.vosburgh@canonical.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox