* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
From: Neil Horman @ 2017-01-23 14:53 UTC (permalink / raw)
To: David Laight
Cc: 'Xin Long', network dev, linux-sctp@vger.kernel.org,
Marcelo Ricardo Leitner, Vlad Yasevich, davem@davemloft.net
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB026B563@AcuExch.aculab.com>
On Mon, Jan 23, 2017 at 11:25:56AM +0000, David Laight wrote:
> From: Xin Long
> > Sent: 19 January 2017 17:19
> > This patch is to implement Sender-Side Procedures for the Add
> > Outgoing and Incoming Streams Request Parameter described in
> > rfc6525 section 5.1.5-5.1.6.
> >
> > It is also to add sockopt SCTP_ADD_STREAMS in rfc6525 section
> > 6.3.4 for users.
> ...
> > + out = params->sas_outstrms;
> > + in = params->sas_instrms;
> > +
> > + if (!out && !in)
> > + goto out;
> > +
> > + if (out) {
> > + __u16 nums = stream->outcnt + out;
>
> Make nums 'unsigned int', the code will be smaller and you can
> use the value for the overflow check.
>
> > + /* Check for overflow, can't use nums here */
> > + if (stream->outcnt + out > SCTP_MAX_STREAM)
> > + goto out;
> > +
> > + /* Use ksize to check if stream array really needs to realloc */
> > + if (ksize(stream->out) / sizeof(*stream->out) < nums) {
> > + struct sctp_stream_out *streamout;
> > +
> > + streamout = kcalloc(nums, sizeof(*streamout),
> > + GFP_KERNEL);
> > + if (!streamout) {
> > + retval = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + memcpy(streamout, stream->out,
> > + sizeof(*streamout) * stream->outcnt);
> > +
> > + kfree(stream->out);
> > + stream->out = streamout;
> > + }
>
> Does kcalloc() zero the entire area, or just the length you ask for?
> If the latter you need to zero the rest here.
Better still, just use krealloc. You still need to zero out any space beyond
the old length, but it will make the code shorter, and avoid the need for
additional temporary variables.
Neil
> ...
>
> David
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" 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 net-next 1/2] net: phy: Fix typo for MDIO module boilerplate comment
From: Andrew Lunn @ 2017-01-23 15:00 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, davem, vivien.didelot
In-Reply-To: <20170123051733.10316-2-f.fainelli@gmail.com>
On Sun, Jan 22, 2017 at 09:17:32PM -0800, Florian Fainelli wrote:
> The module boilerplate macro is named mdio_module_driver and not
> module_mdio_driver, fix that.
>
> Fixes: a9049e0c513c ("mdio: Add support for mdio drivers.")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: dsa: b53: Utilize mdio_module_driver
From: Andrew Lunn @ 2017-01-23 15:00 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, davem, vivien.didelot
In-Reply-To: <20170123051733.10316-3-f.fainelli@gmail.com>
On Sun, Jan 22, 2017 at 09:17:33PM -0800, Florian Fainelli wrote:
> Eliminate a bit of boilerplate code.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* [PATCH 1/3] ath10k: remove multiple defines of DIAG_TRANSFER_LIMIT
From: Srinivas Kandagatla @ 2017-01-23 15:04 UTC (permalink / raw)
To: Kalle Valo
Cc: ath10k, linux-wireless, netdev, linux-kernel, Srinivas Kandagatla
DIAG_TRANSFER_LIMIT is redefined with same value and comments
just below this entry, remove this duplicate entry.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
drivers/net/wireless/ath/ath10k/pci.h | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 9854ad5..c76789d 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -25,11 +25,6 @@
#include "ahb.h"
/*
- * maximum number of bytes that can be handled atomically by DiagRead/DiagWrite
- */
-#define DIAG_TRANSFER_LIMIT 2048
-
-/*
* maximum number of bytes that can be
* handled atomically by DiagRead/DiagWrite
*/
--
2.10.1
^ permalink raw reply related
* [PATCH 2/3] ath10k: use dma_zalloc_coherent()
From: Srinivas Kandagatla @ 2017-01-23 15:04 UTC (permalink / raw)
To: Kalle Valo
Cc: ath10k, linux-wireless, netdev, linux-kernel, Srinivas Kandagatla
In-Reply-To: <1485183876-27080-1-git-send-email-srinivas.kandagatla@linaro.org>
use dma_zalloc_coherent() instead of dma_alloc_coherent and memset().
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
drivers/net/wireless/ath/ath10k/ce.c | 9 +--------
drivers/net/wireless/ath/ath10k/pci.c | 3 +--
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 0b4d796..c2b388f 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -958,7 +958,7 @@ ath10k_ce_alloc_dest_ring(struct ath10k *ar, unsigned int ce_id,
* coherent DMA are unsupported
*/
dest_ring->base_addr_owner_space_unaligned =
- dma_alloc_coherent(ar->dev,
+ dma_zalloc_coherent(ar->dev,
(nentries * sizeof(struct ce_desc) +
CE_DESC_RING_ALIGN),
&base_addr, GFP_KERNEL);
@@ -969,13 +969,6 @@ ath10k_ce_alloc_dest_ring(struct ath10k *ar, unsigned int ce_id,
dest_ring->base_addr_ce_space_unaligned = base_addr;
- /*
- * Correctly initialize memory to 0 to prevent garbage
- * data crashing system when download firmware
- */
- memset(dest_ring->base_addr_owner_space_unaligned, 0,
- nentries * sizeof(struct ce_desc) + CE_DESC_RING_ALIGN);
-
dest_ring->base_addr_owner_space = PTR_ALIGN(
dest_ring->base_addr_owner_space_unaligned,
CE_DESC_RING_ALIGN);
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index b541a1c..855e3de 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -896,7 +896,7 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar, u32 address, void *data,
*/
alloc_nbytes = min_t(unsigned int, nbytes, DIAG_TRANSFER_LIMIT);
- data_buf = (unsigned char *)dma_alloc_coherent(ar->dev,
+ data_buf = (unsigned char *)dma_zalloc_coherent(ar->dev,
alloc_nbytes,
&ce_data_base,
GFP_ATOMIC);
@@ -905,7 +905,6 @@ static int ath10k_pci_diag_read_mem(struct ath10k *ar, u32 address, void *data,
ret = -ENOMEM;
goto done;
}
- memset(data_buf, 0, alloc_nbytes);
remaining_bytes = nbytes;
ce_data = ce_data_base;
--
2.10.1
^ permalink raw reply related
* [PATCH 3/3] ath10k: fix typo in addr calculation
From: Srinivas Kandagatla @ 2017-01-23 15:04 UTC (permalink / raw)
To: Kalle Valo
Cc: ath10k, linux-wireless, netdev, linux-kernel, Srinivas Kandagatla
In-Reply-To: <1485183876-27080-1-git-send-email-srinivas.kandagatla@linaro.org>
CORE_CTRL_ADDRESS is offset in register address space, it does not
make sense to OR it to derive the final address. It looks like its
a typo, so fix it.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
drivers/net/wireless/ath/ath10k/pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 855e3de..023ab10 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1936,7 +1936,7 @@ static int ath10k_pci_wake_target_cpu(struct ath10k *ar)
{
u32 addr, val;
- addr = SOC_CORE_BASE_ADDRESS | CORE_CTRL_ADDRESS;
+ addr = SOC_CORE_BASE_ADDRESS + CORE_CTRL_ADDRESS;
val = ath10k_pci_read32(ar, addr);
val |= CORE_CTRL_CPU_INTR_MASK;
ath10k_pci_write32(ar, addr, val);
--
2.10.1
^ permalink raw reply related
* [PULL] vhost: cleanups and fixes
From: Michael S. Tsirkin @ 2017-01-23 15:05 UTC (permalink / raw)
To: Linus Torvalds
Cc: gcampana, kvm, mst, netdev, pmorel, linux-kernel, virtualization,
dan.carpenter, colin.king, bhumirks, silbe
The following changes since commit 49def1853334396f948dcb4cedb9347abb318df5:
Linux 4.10-rc4 (2017-01-15 16:21:59 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
for you to fetch changes up to 0db1dba5dfaf70fb3baf07973996db2078528cde:
virtio/s390: virtio: constify virtio_config_ops structures (2017-01-19 23:46:34 +0200)
Note: ARM DMA fixes missed this pull, will be in the next one.
----------------------------------------------------------------
virtio, vhost: fixes, cleanups
Random fixes and cleanups that accumulated over the time.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
----------------------------------------------------------------
Bhumika Goyal (2):
vhost: scsi: constify target_core_fabric_ops structures
virtio/s390: virtio: constify virtio_config_ops structures
Colin Ian King (1):
virtio/s390: add missing \n to end of dev_err message
Dan Carpenter (1):
vhost/scsi: silence uninitialized variable warning
G. Campana (1):
virtio_console: fix a crash in config_work_handler
Halil Pasic (2):
tools/virtio/ringtest: fix run-on-all.sh for offline cpus
tools/virtio/ringtest: tweaks for s390
Pierre Morel (1):
virtio/s390: support READ_STATUS command for virtio-ccw
drivers/char/virtio_console.c | 2 +-
drivers/s390/virtio/virtio_ccw.c | 29 +++++++++++++++++++++++++++--
drivers/vhost/scsi.c | 4 ++--
tools/virtio/ringtest/main.h | 12 ++++++++++++
tools/virtio/ringtest/run-on-all.sh | 5 +++--
5 files changed, 45 insertions(+), 7 deletions(-)
^ permalink raw reply
* Re: [PATCH v1] net: phy: micrel: add KSZ8795 ethernet switch
From: Andrew Lunn @ 2017-01-23 15:12 UTC (permalink / raw)
To: Sean Nyekjaer; +Cc: netdev
In-Reply-To: <20170123075846.12888-1-sean.nyekjaer@prevas.dk>
On Mon, Jan 23, 2017 at 08:58:46AM +0100, Sean Nyekjaer wrote:
> This is add support for the PHYs in the KSZ8795 5port managed switch.
>
> It will allow to detect the link between the switch and the soc
> and uses the same read_status functions as the KSZ8873MLL switch.
>
> This ethernet switch have unfortunately the same phy id as KSZ8051.
>
> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
> ---
> drivers/net/phy/micrel.c | 14 ++++++++++++++
> include/linux/micrel_phy.h | 2 ++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index ea92d524d5a8..fa158ae5115b 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -1014,6 +1014,20 @@ static struct phy_driver ksphy_driver[] = {
> .get_stats = kszphy_get_stats,
> .suspend = genphy_suspend,
> .resume = genphy_resume,
> +}, {
> + .phy_id = PHY_ID_KSZ8795,
> + .phy_id_mask = MICREL_PHY_ID_MASK,
> + .name = "Micrel KSZ8795 Switch",
This name is confusing. You are adding support for the PHYs embedded
in the switch, not the switch itself. The phylib has no idea these
PHYs are inside a switch. It does not matter. It is just a PHY. Please
drop the word "Switch".
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH] phy: marvell: remove conflicting initializer
From: Andrew Lunn @ 2017-01-23 15:19 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Florian Fainelli, David S. Miller, Charles-Antoine Couret,
Clemens Gruber, netdev, linux-kernel
In-Reply-To: <20170123121905.3245589-1-arnd@arndb.de>
On Mon, Jan 23, 2017 at 01:18:41PM +0100, Arnd Bergmann wrote:
> One line was apparently pasted incorrectly during a new feature patch:
>
> drivers/net/phy/marvell.c:2090:15: error: initialized field overwritten [-Werror=override-init]
> .features = PHY_GBIT_FEATURES,
>
> I'm removing the extraneous line here to avoid the W=1 warning and restore
> the previous flags value, and I'm slightly reordering the lines for consistency
> to make it less likely to happen again in the future. The ordering in the
> array is still not the same as in the structure definition, instead I picked
> the order that is most common in this file and that seems to make more sense
> here.
>
> Fixes: 0b04680fdae4 ("phy: marvell: Add support for temperature sensor")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Hi Arnd
Thanks for this. I found one bug in my code while testing due to
duplicate probe initialisation. I was surprised at the time the
compiler did not warn me. I didn't think to try W=1. Is there any
legitimate need to allow multiple initialisation of a field? Can this
warning be made always on instead of only W=1?
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [RFC PATCH net-next 4/5] bridge: vlan lwt and dst_metadata netlink support
From: Roopa Prabhu @ 2017-01-23 15:39 UTC (permalink / raw)
To: Rosen, Rami
Cc: netdev@vger.kernel.org, davem@davemloft.net,
stephen@networkplumber.org, nikolay@cumulusnetworks.com,
tgraf@suug.ch, hannes@stressinduktion.org, jbenc@redhat.com,
pshelar@ovn.org, dsa@cumulusnetworks.com, hadi@mojatatu.com
In-Reply-To: <9B0331B6EBBD0E4684FBFAEDA55776F93D495F16@HASMSX110.ger.corp.intel.com>
On 1/22/17, 4:22 PM, Rosen, Rami wrote:
> Hi, Roopa,
>
> Two minor comments:
>
> The parameter br is not used in the br_add_vlan_tunnel_info() method, it should be removed:
Thanks Rami. will take care of this in the next version.
^ permalink raw reply
* Re: [PATCH net] net/mlx5e: Do not recycle pages from emergency reserve
From: David Miller @ 2017-01-23 15:45 UTC (permalink / raw)
To: brouer; +Cc: saeedm, tom, saeedm, netdev, tariqt, eric.dumazet
In-Reply-To: <20170123093940.5d69539e@redhat.com>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Mon, 23 Jan 2017 09:39:40 +0100
> BUT there are other advantages of using a guaranteed recycle pool
> facility (like the page_pool). Namely, (1) DMA-overhead: keeping page
> DMA mapped to counter DMA+IOMMU overhead, (2) RX-zero-copy: opens up
> for a software memory model solution for pre-VMA-mapping pages to
> userspace (See: [1] for argument how this avoids leaking kernel mem,
> but only expose/leak packet-data mem)
+1
^ permalink raw reply
* Re: [PATCH 0/4] Make xfrm usable by 32-bit programs
From: David Miller @ 2017-01-23 15:47 UTC (permalink / raw)
To: steffen.klassert
Cc: cernekee, herbert, paul, sds, eparis, linux-kernel, netdev,
selinux, fw, fan.du, dianders, dtor
In-Reply-To: <20170123093517.GM3541@gauss.secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Mon, 23 Jan 2017 10:35:17 +0100
> So this creates new incompatibilities what is another argument against
> this approach. If you really need this, try to implement a full compat
> layer. I think this is the only sane solution for this.
A full compat layer is the only reasonable solution to this problem.
^ permalink raw reply
* [PATCH net-next] net: stmicro: dwmac4 comments enhancement
From: Joao Pinto @ 2017-01-23 15:56 UTC (permalink / raw)
To: davem; +Cc: peppe.cavallaro, alexandre.torgue, netdev, Joao Pinto
This patch adds clarification about the HW Controller that the dwmac4 driver
package is targeting.
Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 4 ++--
drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 4 ++--
drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h | 2 +-
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c | 4 ++--
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 202216c..d893da9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -1,6 +1,6 @@
/*
- * This is the driver for the GMAC on-chip Ethernet controller for ST SoCs.
- * DWC Ether MAC version 4.00 has been used for developing this code.
+ * This is the driver for the DesignWare Ethernet QoS Controller.
+ * For historical reasons it started in 4.x version.
*
* This only implements the mac core functions for this chip.
*
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index 8816515..1f94bd6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -1,6 +1,6 @@
/*
- * This contains the functions to handle the descriptors for DesignWare databook
- * 4.xx.
+ * This contains the functions to handle the descriptors for the DesignWare
+ * Ethernet QoS Controller. For historical reasons it started in 4.x version.
*
* Copyright (C) 2015 STMicroelectronics Ltd
*
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h
index 9736c50..cfeb08a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h
@@ -1,6 +1,6 @@
/*
* Header File to describe the DMA descriptors and related definitions specific
- * for DesignWare databook 4.xx.
+ * for the DesignWare Ethernet QoS Controller
*
* Copyright (C) 2015 STMicroelectronics Ltd
*
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
index 377d1b4..301d01f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.c
@@ -1,6 +1,6 @@
/*
- * This is the driver for the GMAC on-chip Ethernet controller for ST SoCs.
- * DWC Ether MAC version 4.xx has been used for developing this code.
+ * This is the driver for DesignWare Ethernet QoS Controller.
+ * For historical reasons it started in 4.x version.
*
* This contains the functions to handle the dma.
*
--
2.9.3
^ permalink raw reply related
* Re: [PATCHv3 net-next 3/4] sctp: add support for generating stream reconf add incoming/outgoing streams request chunk
From: David Miller @ 2017-01-23 15:58 UTC (permalink / raw)
To: David.Laight
Cc: marcelo.leitner, lucien.xin, netdev, linux-sctp, nhorman,
vyasevich
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB026B8E0@AcuExch.aculab.com>
From: David Laight <David.Laight@ACULAB.COM>
Date: Mon, 23 Jan 2017 12:26:12 +0000
> From: Marcelo Ricardo Leitner > Sent: 20 January 2017 16:39
>> To: David Laight
>> On Fri, Jan 20, 2017 at 02:50:01PM +0000, David Laight wrote:
>> > From: Xin Long
>> > > Sent: 19 January 2017 17:19
>> > > This patch is to define Add Incoming/Outgoing Streams Request
>> > > Parameter described in rfc6525 section 4.5 and 4.6. They can
>> > > be in one same chunk trunk as rfc6525 section 3.1-7 describes,
>> > > so make them in one function.
>> > ...
>> > > +struct sctp_strreset_addstrm {
>> > > + sctp_paramhdr_t param_hdr;
>> > > + __u32 request_seq;
>> > > + __u16 number_of_streams;
>> > > + __u16 reserved;
>> > > +} __packed;
>> > ...
>> > > + addstrm.param_hdr.type = SCTP_PARAM_RESET_ADD_OUT_STREAMS;
>> > > + addstrm.param_hdr.length = htons(size);
>> > > + addstrm.number_of_streams = htons(out);
>> > > + addstrm.request_seq = htonl(asoc->strreset_outseq);
>> > > + addstrm.reserved = 0;
>> > > +
>> > > + sctp_addto_chunk(retval, size, &addstrm);
>> >
>> > Since you allocate the sctp_strreset_addstrm structure on stack
>> > there is no requirement for it to be packed.
>>
>> It shouldn't matter that it's allocated on stack. Why should it?
>> We need it to be packed as this is a header that will be sent out to
>> another peer, so there can't be any padding on it.
>
> That isn't what __packed means.
> It means that the compiler must assume that the structure can be
> misaligned in memory and must use byte memory accesses on systems
> that fault misaligned memory accesses.
Also, for the types involved there will not be any padding at all.
Check if you do not believe me.
If this is "so critical" for end to end communication, why the heck
do you not see __packed sprinkled all over our definitions for IPV4,
IPV6, TCP, UDP, etc. headers?
Do you know why? Because it's completely unnecessary...
Can I seriously, strongly, state that people should use __packed as
the last possible resort when writing code?
Don't add it unless you run into a problem which fundamentally cannot
be solved by either using different data types, a different ordering
of data items, or similar.
And when __packed is actually legitimate and used I am now going to
require a HUGE FRIGGIN' COMMENT explaining why no other approach
whatsoever can solve the problem.
__packed is going to emit incredibly inefficient code on some cpus,
I really don't think people truly understand the enormous negative
ramifications of using __packed.
It is almost never, ever, necessary.
And I'm tired of writing this tirade every half year or so.
__packed, just say no...
^ permalink raw reply
* Re: [PATCHv3 net-next 3/4] sctp: add support for generating stream reconf add incoming/outgoing streams request chunk
From: David Miller @ 2017-01-23 16:00 UTC (permalink / raw)
To: marcelo.leitner
Cc: David.Laight, lucien.xin, netdev, linux-sctp, nhorman, vyasevich
In-Reply-To: <20170123123628.GX3781@localhost.localdomain>
From: "'Marcelo Ricardo Leitner'" <marcelo.leitner@gmail.com>
Date: Mon, 23 Jan 2017 10:36:28 -0200
> So, no padding. A field just after the other, which is what we want on a
> network header.
It isn't necessary!
Show me a case where it is required when you use properly fixed sized
types and a proper ordering of the struct members. No padding is
going in there, go and check.
Do we splatter __packed all over our ipv4/ipv6 header, TCP header, UDP
header, etc. structures? No, we don't because it's totally unecessary.
I will not accept __packed being used unless it is absolutely, provably,
the only way to solve a particular problem. And when that does happen,
I am going to require a huge comment explaining in detail why this is
the case, and why no other approach or solution solved the problem.
^ permalink raw reply
* Re: [PATCHv3 net-next 4/4] sctp: implement sender-side procedures for Add Incoming/Outgoing Streams Request Parameter
From: Marcelo Ricardo Leitner @ 2017-01-23 16:02 UTC (permalink / raw)
To: Neil Horman
Cc: David Laight, 'Xin Long', network dev,
linux-sctp@vger.kernel.org, Vlad Yasevich, davem@davemloft.net
In-Reply-To: <20170123145347.GB31445@hmswarspite.think-freely.org>
On Mon, Jan 23, 2017 at 09:53:47AM -0500, Neil Horman wrote:
> On Mon, Jan 23, 2017 at 11:25:56AM +0000, David Laight wrote:
> > From: Xin Long
> > > Sent: 19 January 2017 17:19
> > > This patch is to implement Sender-Side Procedures for the Add
> > > Outgoing and Incoming Streams Request Parameter described in
> > > rfc6525 section 5.1.5-5.1.6.
> > >
> > > It is also to add sockopt SCTP_ADD_STREAMS in rfc6525 section
> > > 6.3.4 for users.
> > ...
> > > + out = params->sas_outstrms;
> > > + in = params->sas_instrms;
> > > +
> > > + if (!out && !in)
> > > + goto out;
> > > +
> > > + if (out) {
> > > + __u16 nums = stream->outcnt + out;
> >
> > Make nums 'unsigned int', the code will be smaller and you can
> > use the value for the overflow check.
> >
> > > + /* Check for overflow, can't use nums here */
> > > + if (stream->outcnt + out > SCTP_MAX_STREAM)
> > > + goto out;
> > > +
> > > + /* Use ksize to check if stream array really needs to realloc */
> > > + if (ksize(stream->out) / sizeof(*stream->out) < nums) {
> > > + struct sctp_stream_out *streamout;
> > > +
> > > + streamout = kcalloc(nums, sizeof(*streamout),
> > > + GFP_KERNEL);
> > > + if (!streamout) {
> > > + retval = -ENOMEM;
> > > + goto out;
> > > + }
> > > +
> > > + memcpy(streamout, stream->out,
> > > + sizeof(*streamout) * stream->outcnt);
> > > +
> > > + kfree(stream->out);
> > > + stream->out = streamout;
> > > + }
> >
> > Does kcalloc() zero the entire area, or just the length you ask for?
> > If the latter you need to zero the rest here.
> Better still, just use krealloc. You still need to zero out any space beyond
> the old length, but it will make the code shorter, and avoid the need for
> additional temporary variables.
Seems if we pass gfp | __GFP_ZERO to krealloc it will end up zeroing the
slab for us before doing the memcpy.
I didn't follow all paths but in slab_alloc_node it will end up calling:
if (unlikely(gfpflags & __GFP_ZERO) && object)
memset(object, 0, s->object_size);
So I would expect that other paths also do it.
Marcelo
>
> Neil
>
> > ...
> >
> > David
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" 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] phy: marvell: remove conflicting initializer
From: Arnd Bergmann @ 2017-01-23 16:09 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, David S. Miller, Charles-Antoine Couret,
Clemens Gruber, netdev, linux-kernel
In-Reply-To: <20170123151941.GG10895@lunn.ch>
On Monday, January 23, 2017 4:19:41 PM CET Andrew Lunn wrote:
> On Mon, Jan 23, 2017 at 01:18:41PM +0100, Arnd Bergmann wrote:
> > One line was apparently pasted incorrectly during a new feature patch:
> >
> > drivers/net/phy/marvell.c:2090:15: error: initialized field overwritten [-Werror=override-init]
> > .features = PHY_GBIT_FEATURES,
> >
> > I'm removing the extraneous line here to avoid the W=1 warning and restore
> > the previous flags value, and I'm slightly reordering the lines for consistency
> > to make it less likely to happen again in the future. The ordering in the
> > array is still not the same as in the structure definition, instead I picked
> > the order that is most common in this file and that seems to make more sense
> > here.
> >
> > Fixes: 0b04680fdae4 ("phy: marvell: Add support for temperature sensor")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Hi Arnd
>
> Thanks for this. I found one bug in my code while testing due to
> duplicate probe initialisation. I was surprised at the time the
> compiler did not warn me. I didn't think to try W=1. Is there any
> legitimate need to allow multiple initialisation of a field? Can this
> warning be made always on instead of only W=1?
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>
Great question! I have sent out patches for a couple of actual bugs that
came from this warning in the past few months.
I still have a couple of patches that I have not sent out so far,
I think they are all false-positives, and I have not found a good
workaround for most of them other than disabling the warning locally.
Once we get the patches below (or some other workaround) into the
kernel, we should definitely enable the warning by default.
Arnd
arch/arm64/kernel/sys.c | 1 +
arch/arm64/kernel/sys32.c | 1 +
arch/arm64/kernel/traps.c | 1 +
arch/x86/kernel/cpu/common.c | 1 +
drivers/block/drbd/drbd_main.c | 7 +++----
drivers/gpu/drm/gma500/Makefile | 1 +
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/omapdrm/omap_fbdev.c | 3 +++
drivers/net/ethernet/qlogic/qede/qede_dcbnl.c | 1 -
drivers/net/ethernet/renesas/sh_eth.c | 1 +
fs/proc/task_mmu.c | 2 ++
include/linux/libata.h | 1 +
include/linux/perf/arm_pmu.h | 1 +
kernel/bpf/core.c | 2 ++
net/wimax/stack.c | 6 +++---
sound/usb/quirks-table.h | 3 ++-
17 files changed, 30 insertions(+), 16 deletions(-)
>From b85020cd2c0da34c940b6a7d5d04a39e01fa3bf9 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 24 Jun 2016 15:04:14 +0200
Subject: [PATCH] arm64: turn off -Woverride-init false positives
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/arch/arm64/kernel/sys.c b/arch/arm64/kernel/sys.c
index 26fe8ea93ea2..ce84c7b821f9 100644
--- a/arch/arm64/kernel/sys.c
+++ b/arch/arm64/kernel/sys.c
@@ -55,6 +55,7 @@ asmlinkage long sys_rt_sigreturn_wrapper(void);
#undef __SYSCALL
#define __SYSCALL(nr, sym) [nr] = sym,
+#pragma GCC diagnostic ignored "-Woverride-init"
/*
* The sys_call_table array must be 4K aligned to be accessible from
* kernel/entry.S.
diff --git a/arch/arm64/kernel/sys32.c b/arch/arm64/kernel/sys32.c
index a40b1343b819..78c768c9fe23 100644
--- a/arch/arm64/kernel/sys32.c
+++ b/arch/arm64/kernel/sys32.c
@@ -42,6 +42,7 @@ asmlinkage long compat_sys_mmap2_wrapper(void);
#undef __SYSCALL
#define __SYSCALL(nr, sym) [nr] = sym,
+#pragma GCC diagnostic ignored "-Woverride-init"
/*
* The sys_call_table array must be 4K aligned to be accessible from
* kernel/entry.S.
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index ff290d569f41..d7c4e1088d49 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -558,6 +558,7 @@ asmlinkage long do_ni_syscall(struct pt_regs *regs)
return sys_ni_syscall();
}
+#pragma GCC diagnostic ignored "-Woverride-init"
static const char *esr_class_str[] = {
[0 ... ESR_ELx_EC_MAX] = "UNRECOGNIZED EC",
[ESR_ELx_EC_UNKNOWN] = "Unknown/Uncategorized",
>From 975b03637d5362bbf99ad46d59dbf94b495cb78f Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 3 Jan 2017 15:35:07 +0100
Subject: [PATCH] drm: fix override-init warnings
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/gpu/drm/gma500/Makefile b/drivers/gpu/drm/gma500/Makefile
index 190e55f2f891..3ff5f58db4f4 100644
--- a/drivers/gpu/drm/gma500/Makefile
+++ b/drivers/gpu/drm/gma500/Makefile
@@ -2,6 +2,7 @@
# KMS driver for the GMA500
#
ccflags-y += -I$(srctree)/include/drm
+CFLAGS_framebuffer.o += $(call cc-disable-warning, override-init)
gma500_gfx-y += \
accel_2d.o \
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 74ca2e8b2494..fb1342d59a7c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -5,6 +5,7 @@
subdir-ccflags-$(CONFIG_DRM_I915_WERROR) := -Werror
subdir-ccflags-y += \
$(call as-instr,movntdqa (%eax)$(comma)%xmm0,-DCONFIG_AS_MOVNTDQA)
+subdir-ccflags-y += $(call cc-disable-warning, override-init)
# Please keep these build lists sorted!
>From af178eec23da3e549b7ac5360c213955059d9f16 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 15 Nov 2016 14:10:37 +0100
Subject: [PATCH] [HACK] drm: omapdrm: field override
../drivers/gpu/drm/omapdrm/omap_fbdev.c:96:20: error: initialized field overwritten [-Werror=override-init]
.fb_pan_display = omap_fbdev_pan_display,
^~~~~~~~~~~~~~~~~~~~~~
../drivers/gpu/drm/omapdrm/omap_fbdev.c:96:20: note: (near initialization for 'omap_fb_ops.fb_pan_display')
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
index 2a839956dae6..d953186142d1 100644
--- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
+++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
@@ -80,6 +80,8 @@ static int omap_fbdev_pan_display(struct fb_var_screeninfo *var,
return drm_fb_helper_pan_display(var, fbi);
}
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Woverride-init"
static struct fb_ops omap_fb_ops = {
.owner = THIS_MODULE,
DRM_FB_HELPER_DEFAULT_OPS,
@@ -95,6 +97,7 @@ static struct fb_ops omap_fb_ops = {
.fb_pan_display = omap_fbdev_pan_display,
};
+#pragma GCC diagnostic pop
static int omap_fbdev_create(struct drm_fb_helper *helper,
struct drm_fb_helper_surface_size *sizes)
>From a9c01cba0336e2907d78953226a5d985edcf1ee1 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 8 Jun 2016 13:44:51 +0200
Subject: [PATCH] procfs: disable -Woverride-init warning
This warning finds some actual bugs, but the usage in procfs is intended,
so we get a false positive output many lines like:
fs/proc/task_mmu.c: In function 'show_smap_vma_flags':
fs/proc/task_mmu.c:629:2403: error: initialized field overwritten [-Werror=override-init]
fs/proc/task_mmu.c:629:2403: note: (near initialization for 'mnemonics[0]')
fs/proc/task_mmu.c:630:2403: error: initialized field overwritten [-Werror=override-init]
fs/proc/task_mmu.c:630:2403: note: (near initialization for 'mnemonics[1]')
fs/proc/task_mmu.c:631:2403: error: initialized field overwritten [-Werror=override-init]
fs/proc/task_mmu.c:631:2403: note: (near initialization for 'mnemonics[2]')
This uses a #pragma to turn off the warning, as a preparation for
eventually enabling the gcc flag by default for the kernel.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index ee3efb229ef6..661dca5a0042 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -616,6 +616,7 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
return 0;
}
+#pragma GCC diagnostic ignored "-Woverride-init"
static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
{
/*
@@ -682,6 +683,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
}
seq_putc(m, '\n');
}
+#pragma GCC diagnostic warning "-Woverride-init"
#ifdef CONFIG_HUGETLB_PAGE
static int smaps_hugetlb_range(pte_t *pte, unsigned long hmask,
>From 85cbf32994e826a9b3f4b0e6c481d85d77c7ad19 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 8 Jun 2016 13:08:55 +0200
Subject: [PATCH] sh_eth: disable -Woverride-init warning
-Woverride-init is part of a 'make W=1' build and it might be nice
to enable it in normal builds as it catches real bugs, but it
causes hundreds of identical false positive warnings in the sh_eth
driver:
ethernet/renesas/sh_eth.c:63:11: error: initialized field overwritten [-Werror=override-init]
ethernet/renesas/sh_eth.c:63:11: note: (near initialization for 'sh_eth_offset_gigabit[0]')
ethernet/renesas/sh_eth.c:64:11: error: initialized field overwritten [-Werror=override-init]
ethernet/renesas/sh_eth.c:64:11: note: (near initialization for 'sh_eth_offset_gigabit[1]')
ethernet/renesas/sh_eth.c:65:12: error: initialized field overwritten [-Werror=override-init]
ethernet/renesas/sh_eth.c:65:12: note: (near initialization for 'sh_eth_offset_gigabit[2]')
ethernet/renesas/sh_eth.c:66:12: error: initialized field overwritten [-Werror=override-init]
ethernet/renesas/sh_eth.c:66:12: note: (near initialization for 'sh_eth_offset_gigabit[3]')
ethernet/renesas/sh_eth.c:67:11: error: initialized field overwritten [-Werror=override-init]
ethernet/renesas/sh_eth.c:67:11: note: (near initialization for 'sh_eth_offset_gigabit[4]')
This turns off the particular warning for this file, overriding the
C flags.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 45a7a6ba7644..1980cbcef066 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -57,6 +57,7 @@
#define SH_ETH_OFFSET_DEFAULTS \
[0 ... SH_ETH_MAX_REGISTER_OFFSET - 1] = SH_ETH_OFFSET_INVALID
+#pragma GCC diagnostic ignored "-Woverride-init"
static const u16 sh_eth_offset_gigabit[SH_ETH_MAX_REGISTER_OFFSET] = {
SH_ETH_OFFSET_DEFAULTS,
>From 14e1d7313bb030decc1a7ac0618c4c95474fc116 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 8 Jun 2016 10:46:40 +0200
Subject: [PATCH] perf: arm-pmu: shut up -Woverride-init warning
gcc -Woverride-init provides useful warnings about bugs in some drivers,
but it warns about an intentional duplicate inialization in the ARM
pmu header:
arch/arm/kernel/perf_event_v7.c:159:31: error: initialized field overwritten [-Werror=override-init]
arch/arm/kernel/perf_event_v7.c:159:31: note: (near initialization for 'armv7_a8_perf_map[0]')
arch/arm/kernel/perf_event_v7.c:160:33: error: initialized field overwritten [-Werror=override-init]
arch/arm/kernel/perf_event_v7.c:160:33: note: (near initialization for 'armv7_a8_perf_map[1]')
arch/arm/kernel/perf_event_v7.c:161:37: error: initialized field overwritten [-Werror=override-init]
arch/arm/kernel/perf_event_v7.c:161:37: note: (near initialization for 'armv7_a8_perf_map[2]')
arch/arm/kernel/perf_event_v7.c:162:33: error: initialized field overwritten [-Werror=override-init]
arch/arm/kernel/perf_event_v7.c:162:33: note: (near initialization for 'armv7_a8_perf_map[3]')
arch/arm/kernel/perf_event_v7.c:163:40: error: initialized field overwritten [-Werror=override-init]
arch/arm/kernel/perf_event_v7.c:163:40: note: (near initialization for 'armv7_a8_perf_map[4]')
...
This uses a #pragma to shut up that warning, as a preparation for enabling
the warning by default in the future.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index 8462da266089..f61462cd362a 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -41,6 +41,7 @@ struct arm_pmu_platdata {
#define C(_x) PERF_COUNT_HW_CACHE_##_x
#define CACHE_OP_UNSUPPORTED 0xFFFF
+#pragma GCC diagnostic ignored "-Woverride-init"
#define PERF_MAP_ALL_UNSUPPORTED \
[0 ... PERF_COUNT_HW_MAX - 1] = HW_OP_UNSUPPORTED
>From e2f45688890614c67fef699a8612283ad4c44b06 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 8 Jun 2016 10:46:40 +0200
Subject: [PATCH] bpf: shut up -Woverride-init warning
gcc -Woverride-init provides useful warnings about bugs in some drivers,
but it warns about an intentional duplicate inialization in the bpf
code:
kernel/bpf/core.c: In function '__bpf_prog_run':
kernel/bpf/core.c:476:26: error: initialized field overwritten [-Werror=override-init]
kernel/bpf/core.c:476:26: note: (near initialization for 'jumptable[12]')
kernel/bpf/core.c:477:26: error: initialized field overwritten [-Werror=override-init]
kernel/bpf/core.c:477:26: note: (near initialization for 'jumptable[4]')
kernel/bpf/core.c:478:26: error: initialized field overwritten [-Werror=override-init]
kernel/bpf/core.c:478:26: note: (near initialization for 'jumptable[28]')
kernel/bpf/core.c:479:26: error: initialized field overwritten [-Werror=override-init]
kernel/bpf/core.c:479:26: note: (near initialization for 'jumptable[20]')
kernel/bpf/core.c:480:26: error: initialized field overwritten [-Werror=override-init]
kernel/bpf/core.c:480:26: note: (near initialization for 'jumptable[92]')
kernel/bpf/core.c:481:26: error: initialized field overwritten [-Werror=override-init]
This uses a #pragma to shut up that warning, as a preparation for enabling
the warning by default in the future.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 503d4211988a..8944778d9c13 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -547,6 +547,7 @@ EXPORT_SYMBOL_GPL(__bpf_call_base);
*
* Decode and execute eBPF instructions.
*/
+#pragma GCC diagnostic ignored "-Woverride-init"
static unsigned int __bpf_prog_run(void *ctx, const struct bpf_insn *insn)
{
u64 stack[MAX_BPF_STACK / sizeof(u64)];
@@ -1003,6 +1004,7 @@ static unsigned int __bpf_prog_run(void *ctx, const struct bpf_insn *insn)
WARN_RATELIMIT(1, "unknown opcode %02x\n", insn->code);
return 0;
}
+#pragma GCC diagnostic warning "-Woverride-init"
STACK_FRAME_NON_STANDARD(__bpf_prog_run); /* jump table */
bool bpf_prog_array_compatible(struct bpf_array *array,
>From 092549ed4782679e220357a3990bb5087e7d277f Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 8 Jun 2016 10:41:06 +0200
Subject: [PATCH] libata: shut up -Woverride-init warning
In general, the -Woverride-init is helpful to catch bugs with drivers that
unintentially initialize the same field in a structure more than once.
In case of libata, it conflicts with the intended use of the ATA_BASE_SHT()
macro, causing warnings about code that works as designed:
drivers/ata/ahci.c:107:555: error: initialized field overwritten [-Werror=override-init]
drivers/ata/ahci.c:107:685: error: initialized field overwritten [-Werror=override-init]
drivers/ata/acard-ahci.c:86:567: error: initialized field overwritten [-Werror=override-init]
drivers/ata/ahci_platform.c:37:555: error: initialized field overwritten [-Werror=override-init]
drivers/ata/sata_sil24.c:388:15: error: initialized field overwritten [-Werror=override-init]
drivers/ata/sata_highbank.c:456:573: error: initialized field overwritten [-Werror=override-init]
drivers/ata/ahci_brcmstb.c:282:565: error: initialized field overwritten [-Werror=override-init]
drivers/ata/ahci_ceva.c:162:565: error: initialized field overwritten [-Werror=override-init]
drivers/ata/ahci_imx.c:526:563: error: initialized field overwritten [-Werror=override-init]
drivers/ata/ahci_mvebu.c:98:567: error: initialized field overwritten [-Werror=override-init]
drivers/ata/ahci_sunxi.c:175:567: error: initialized field overwritten [-Werror=override-init]
drivers/ata/ahci_st.c:145:561: error: initialized field overwritten [-Werror=override-init]
drivers/ata/ahci_tegra.c:295:567: error: initialized field overwritten [-Werror=override-init]
drivers/ata/ahci_xgene.c:731:567: error: initialized field overwritten [-Werror=override-init]
drivers/ata/ahci_qoriq.c:148:567: error: initialized field overwritten [-Werror=override-init]
drivers/ata/sata_mv.c:683:15: error: initialized field overwritten [-Werror=override-init]
drivers/ata/sata_nv.c:395:15: error: initialized field overwritten [-Werror=override-init]
drivers/ata/sata_nv.c:398:21: error: initialized field overwritten [-Werror=override-init]
drivers/ata/sata_nv.c:403:15: error: initialized field overwritten [-Werror=override-init]
drivers/ata/sata_nv.c:406:21: error: initialized field overwritten [-Werror=override-init]
drivers/ata/pata_atiixp.c:254:18: error: initialized field overwritten [-Werror=override-init]
drivers/ata/pata_cs5520.c:107:18: error: initialized field overwritten [-Werror=override-init]
drivers/ata/pata_sc1200.c:210:18: error: initialized field overwritten [-Werror=override-init]
drivers/ata/pata_serverworks.c:256:18: error: initialized field overwritten [-Werror=override-init]
This adds a #pragma GCC diagnostic ignored "-Woverride-init" next to the
definition to let gcc know that it shouldn't warn about duplicate initializers
in files that include this header.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 0e8a8000b45f..038b1ee7485f 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1342,6 +1342,7 @@ extern struct device_attribute *ata_common_sdev_attrs[];
* edge driver's module reference, otherwise the driver can be unloaded
* even if the scsi_device is being accessed.
*/
+#pragma GCC diagnostic ignored "-Woverride-init"
#define ATA_BASE_SHT(drv_name) \
.module = THIS_MODULE, \
.name = drv_name, \
>From bafaf130a03ac21377fe7c142ad7ce7417ebcfe3 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 9 Jun 2016 23:20:58 +0200
Subject: [PATCH] qede: remove duplicate initializer
../drivers/net/ethernet/qlogic/qede/qede_dcbnl.c:327:13: error: initialized field overwritten [-Werror=override-init]
.getdcbx = qede_dcbnl_getdcbx,
^~~~~~~~~~~~~~~~~~
../drivers/net/ethernet/qlogic/qede/qede_dcbnl.c:327:13: note: (near initialization for 'qede_dcbnl_ops.getdcbx')
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/net/ethernet/qlogic/qede/qede_dcbnl.c b/drivers/net/ethernet/qlogic/qede/qede_dcbnl.c
index 03e8c0212433..318f0cbd58ed 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_dcbnl.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_dcbnl.c
@@ -308,7 +308,6 @@ static const struct dcbnl_rtnl_ops qede_dcbnl_ops = {
.ieee_setets = qede_dcbnl_ieee_setets,
.ieee_getapp = qede_dcbnl_ieee_getapp,
.ieee_setapp = qede_dcbnl_ieee_setapp,
- .getdcbx = qede_dcbnl_getdcbx,
.ieee_peer_getpfc = qede_dcbnl_ieee_peer_getpfc,
.ieee_peer_getets = qede_dcbnl_ieee_peer_getets,
.getstate = qede_dcbnl_getstate,
>From 657078acc9b9e8abc17aa7eb2d93877194e085ef Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 8 Jun 2016 10:31:59 +0200
Subject: [PATCH] [RFC] wimax: remove duplicate entries in wimax_gnl_policy
gcc -Wextra warns about duplicate initializations here:
net/wimax/stack.c:408:29: error: initialized field overwritten [-Werror=override-init]
net/wimax/stack.c:408:29: note: (near initialization for 'wimax_gnl_policy[1]')
net/wimax/stack.c:412:28: error: initialized field overwritten [-Werror=override-init]
net/wimax/stack.c:412:28: note: (near initialization for 'wimax_gnl_policy[1]')
net/wimax/stack.c:413:26: error: initialized field overwritten [-Werror=override-init]
net/wimax/stack.c:413:26: note: (near initialization for 'wimax_gnl_policy[1]')
This was introduced by b61a5eea5904 ("wimax: use
genl_register_family_with_ops()"), which consolidated four existing
genl_ops tables into one as a simplification.
I do not understand the reasoning behind that earlier patch, as it seems
to apply the superset of the four policy to all commands. If it was
indeed correct, we can either remove or comment out the duplicate entries
as I do here.
It's also possible that we want a partial revert of the patch that
introduced this, or a broader rework.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/net/wimax/stack.c b/net/wimax/stack.c
index 5db731512014..1c118def3713 100644
--- a/net/wimax/stack.c
+++ b/net/wimax/stack.c
@@ -404,12 +404,12 @@ void wimax_dev_init(struct wimax_dev *wimax_dev)
EXPORT_SYMBOL_GPL(wimax_dev_init);
static const struct nla_policy wimax_gnl_policy[WIMAX_GNL_ATTR_MAX + 1] = {
- [WIMAX_GNL_RESET_IFIDX] = { .type = NLA_U32, },
- [WIMAX_GNL_RFKILL_IFIDX] = { .type = NLA_U32, },
+/* [WIMAX_GNL_RESET_IFIDX] = { .type = NLA_U32, }, */
+/* [WIMAX_GNL_RFKILL_IFIDX] = { .type = NLA_U32, }, */
[WIMAX_GNL_RFKILL_STATE] = {
.type = NLA_U32 /* enum wimax_rf_state */
},
- [WIMAX_GNL_STGET_IFIDX] = { .type = NLA_U32, },
+/* [WIMAX_GNL_STGET_IFIDX] = { .type = NLA_U32, }, */
[WIMAX_GNL_MSG_IFIDX] = { .type = NLA_U32, },
[WIMAX_GNL_MSG_DATA] = {
.type = NLA_UNSPEC, /* libnl doesn't grok BINARY yet */
>From ad1e6c0987c3e03fe9177d6f1a5aaeb9732f80f5 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 8 Jun 2016 10:23:51 +0200
Subject: [PATCH] drbd: sanitize cmdnames table
gcc -Wextra warns about a duplicate entry in cmdnames[]:
drivers/block/drbd/drbd_main.c: In function 'cmdname':
drivers/block/drbd/drbd_main.c:3680:21: error: initialized field overwritten [-Werror=override-init]
drivers/block/drbd/drbd_main.c:3680:21: note: (near initialization for 'cmdnames[44]')
Apparently, there are two entries for P_RETRY_WRITE, so I'm removing the
one that ended up unused. I also checked the table to ensure that all
other entries are there and sorted them by numeric value.
I see that there is no entry for P_TRIM, so I've added that
but left it commented out. Please uncomment if that entry should
be there.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index f6fbdac2391d..5aec73fbf2ba 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -3719,7 +3719,6 @@ const char *cmdname(enum drbd_packet cmd)
static const char *cmdnames[] = {
[P_DATA] = "Data",
[P_WSAME] = "WriteSame",
- [P_TRIM] = "Trim",
[P_DATA_REPLY] = "DataReply",
[P_RS_DATA_REPLY] = "RSDataReply",
[P_BARRIER] = "Barrier",
@@ -3730,7 +3729,6 @@ const char *cmdname(enum drbd_packet cmd)
[P_DATA_REQUEST] = "DataRequest",
[P_RS_DATA_REQUEST] = "RSDataRequest",
[P_SYNC_PARAM] = "SyncParam",
- [P_SYNC_PARAM89] = "SyncParam89",
[P_PROTOCOL] = "ReportProtocol",
[P_UUIDS] = "ReportUUIDs",
[P_SIZES] = "ReportSizes",
@@ -3738,6 +3736,7 @@ const char *cmdname(enum drbd_packet cmd)
[P_SYNC_UUID] = "ReportSyncUUID",
[P_AUTH_CHALLENGE] = "AuthChallenge",
[P_AUTH_RESPONSE] = "AuthResponse",
+ [P_STATE_CHG_REQ] = "StateChgRequest",
[P_PING] = "Ping",
[P_PING_ACK] = "PingAck",
[P_RECV_ACK] = "RecvAck",
@@ -3748,22 +3747,22 @@ const char *cmdname(enum drbd_packet cmd)
[P_NEG_DREPLY] = "NegDReply",
[P_NEG_RS_DREPLY] = "NegRSDReply",
[P_BARRIER_ACK] = "BarrierAck",
- [P_STATE_CHG_REQ] = "StateChgRequest",
[P_STATE_CHG_REPLY] = "StateChgReply",
[P_OV_REQUEST] = "OVRequest",
[P_OV_REPLY] = "OVReply",
[P_OV_RESULT] = "OVResult",
[P_CSUM_RS_REQUEST] = "CsumRSRequest",
[P_RS_IS_IN_SYNC] = "CsumRSIsInSync",
+ [P_SYNC_PARAM89] = "SyncParam89",
[P_COMPRESSED_BITMAP] = "CBitmap",
[P_DELAY_PROBE] = "DelayProbe",
[P_OUT_OF_SYNC] = "OutOfSync",
- [P_RETRY_WRITE] = "RetryWrite",
[P_RS_CANCEL] = "RSCancel",
[P_CONN_ST_CHG_REQ] = "conn_st_chg_req",
[P_CONN_ST_CHG_REPLY] = "conn_st_chg_reply",
[P_RETRY_WRITE] = "retry_write",
[P_PROTOCOL_UPDATE] = "protocol_update",
+ [P_TRIM] = "Trim",
[P_RS_THIN_REQ] = "rs_thin_req",
[P_RS_DEALLOCATED] = "rs_deallocated",
>From 860d00e7a5220f9bc44bf8d6529c04dbb8856451 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 15 Jun 2016 09:10:07 +0200
Subject: [PATCH] x86: don't warn about duplicate initializion of stack sizes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
arch/x86/include/asm/page_64_types.h:18:21: error: initialized field overwritten [-Werror=override-init]
#define DEBUG_STKSZ (PAGE_SIZE << DEBUG_STACK_ORDER)
^
arch/x86/kernel/cpu/common.c:1304:26: note: in expansion of macro ‘DEBUG_STKSZ’
[DEBUG_STACK - 1] = DEBUG_STKSZ
^
arch/x86/include/asm/page_64_types.h:18:21: note: (near initialization for ‘exception_stack_sizes[2]’)
#define DEBUG_STKSZ (PAGE_SIZE << DEBUG_STACK_ORDER)
^
arch/x86/kernel/cpu/common.c:1304:26: note: in expansion of macro ‘DEBUG_STKSZ’
[DEBUG_STACK - 1] = DEBUG_STKSZ
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 5ba74f280448..ebc909203a38 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1268,6 +1268,7 @@ EXPORT_PER_CPU_SYMBOL(__preempt_count);
* limit), all of them are 4K, except the debug stack which
* is 8K.
*/
+#pragma GCC diagnostic ignored "-Woverride-init"
static const unsigned int exception_stack_sizes[N_EXCEPTION_STACKS] = {
[0 ... N_EXCEPTION_STACKS - 1] = EXCEPTION_STKSZ,
[DEBUG_STACK - 1] = DEBUG_STKSZ
>From 01fc760fe7fbee1163f4a0c25cd722014054de70 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 8 Jun 2016 10:22:51 +0200
Subject: [PATCH] alsa: usb: avoid duplicate initializers in quirks table
sound/usb/quirks-table.h:2892:125: error: initialized field overwritten [-Werror=override-init]
sound/usb/quirks-table.h:2892:125: note: (near initialization for 'usb_audio_ids[230].match_flags')
sound/usb/quirks-table.h:2892:181: error: initialized field overwritten [-Werror=override-init]
sound/usb/quirks-table.h:2892:181: note: (near initialization for 'usb_audio_ids[230].bInterfaceClass')
sound/usb/quirks-table.h:2893:125: error: initialized field overwritten [-Werror=override-init]
sound/usb/quirks-table.h:2893:125: note: (near initialization for 'usb_audio_ids[231].match_flags')
sound/usb/quirks-table.h:2893:181: error: initialized field overwritten [-Werror=override-init]
sound/usb/quirks-table.h:2893:181: note: (near initialization for 'usb_audio_ids[231].bInterfaceClass')
sound/usb/quirks-table.h:2894:125: error: initialized field overwritten [-Werror=override-init]
sound/usb/quirks-table.h:2894:125: note: (near initialization for 'usb_audio_ids[232].match_flags')
sound/usb/quirks-table.h:2894:181: error: initialized field overwritten [-Werror=override-init]
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h
index 8a59d4782a0f..8d302ef764a3 100644
--- a/sound/usb/quirks-table.h
+++ b/sound/usb/quirks-table.h
@@ -2875,10 +2875,11 @@ YAMAHA_DEVICE(0x7010, "UB99"),
*/
#define AU0828_DEVICE(vid, pid, vname, pname) { \
- USB_DEVICE_VENDOR_SPEC(vid, pid), \
.match_flags = USB_DEVICE_ID_MATCH_DEVICE | \
USB_DEVICE_ID_MATCH_INT_CLASS | \
USB_DEVICE_ID_MATCH_INT_SUBCLASS, \
+ .idVendor = vid, \
+ .idProduct = pid, \
.bInterfaceClass = USB_CLASS_AUDIO, \
.bInterfaceSubClass = USB_SUBCLASS_AUDIOCONTROL, \
.driver_info = (unsigned long) &(const struct snd_usb_audio_quirk) { \
^ permalink raw reply related
* Re: [RFC PATCH net-next 0/5] bridge: per vlan lwt and dst_metadata support
From: Roopa Prabhu @ 2017-01-23 16:13 UTC (permalink / raw)
To: Jiri Benc
Cc: Jiri Pirko, netdev, davem, stephen, nikolay, tgraf, hannes,
pshelar, dsa, hadi
In-Reply-To: <20170123095130.59ddcf34@griffin>
On 1/23/17, 12:51 AM, Jiri Benc wrote:
> On Mon, 23 Jan 2017 09:08:05 +0100, Jiri Pirko wrote:
>> Sat, Jan 21, 2017 at 06:46:51AM CET, roopa@cumulusnetworks.com wrote:
>>> Other approaches tried and vetoed:
>>> - tc vlan push/pop and tunnel metadata dst:
>>> - posses a tc rule scalability problem (2 rules per vni)
>> Why it is a problem?
> Wanted to ask exactly the same question.
>
>>> - cannot handle the case where a packet needs to be replicated to
>>> multiple vxlan remote tunnel end-points.. which the vxlan driver
>>> can do today by having multiple remote destinations per fdb.
>> Can't you just extend the tc to support this?
> +1
>
>> To me, looks like the tc is the correct place to hangle this. Then, the
>> user can use it for multiple cases of forwarding, including bridge,
>> tc-mirred, ovs and others. Putting this in bridge somehow seems wrong in
>> this light. Also, the bridge code is polluted enough as it is. I this we
>> should be super-picky to add another code there.
> Completely agreed.
>
The problem is, When you use the Linux bridge for vlan configuration and vlan filtering, having
additional vlan config in some other subsystem is a bit awkward. Its the same argument where
tc and netfilter subsystems have so much overlap...but they do because, each subsystem has to
have the missing functionality for completeness....cannot expect the user to configure a few rules
in tc and a few others in netfilter. In this case, I cannot expect the user/app to configure vlan filtering
in one place and have additional vlan to tunnel filtering in another subsystem. Its duplicating vlan
configuration in multiple places.
Also, the goal is to reduce the number of vxlan devices from say 4k to 1. I don't think replacing
it with 8k (egress + ingress) rules is going in the right direction.
bigger picture/context... With bgp now being deployed as a controller for
l2 ethernet vpn solutions (https://tools.ietf.org/html/draft-ietf-bess-evpn-overlay-07), popular routing
suites like quagga, are looking at using the Linux api for L2 configuration.
And, a 'vlan-to-tunid' mapping is a very common configuration in L2 ethernet vpn configurations.
With the bridge driver being the center of vlan configuration in such bridged networks,
having all vlan configuration in one place makes sense. Also, quagga now has a single api
to get the 'vlan-to-tunid' mapping. Telling quagga to look at tc filtering rules to derive this
mapping is not inline with the rest of the L2 api ..(when you use the Linux bridge ..).
Regarding piling this on to the bridge driver:
- It is using existing dst metadata infra + two hooks disabled by default.
- I started this with vlan-to-vxlan map in the vxlan driver (regret spending time on it)..
I ended up duplicating a lot of vlan handling code that the bridge driver all-ready had in the vxlan driver.
Hence bridge driver is the right place for this ...when you are using the bridge driver for vlan filtering.
- Besides, having it in the bridge driver ..enables the bridge driver for future other
l2 evpn dataplanes (vxlan just happens to be one of them i am working on currently).
^ permalink raw reply
* Re: [PATCHv3 net-next 3/4] sctp: add support for generating stream reconf add incoming/outgoing streams request chunk
From: marcelo.leitner @ 2017-01-23 16:14 UTC (permalink / raw)
To: David Miller
Cc: David.Laight, lucien.xin, netdev, linux-sctp, nhorman, vyasevich
In-Reply-To: <20170123.110047.447365739282173577.davem@davemloft.net>
On Mon, Jan 23, 2017 at 11:00:47AM -0500, David Miller wrote:
> From: "'Marcelo Ricardo Leitner'" <marcelo.leitner@gmail.com>
> Date: Mon, 23 Jan 2017 10:36:28 -0200
>
> > So, no padding. A field just after the other, which is what we want on a
> > network header.
>
> It isn't necessary!
>
> Show me a case where it is required when you use properly fixed sized
> types and a proper ordering of the struct members. No padding is
> going in there, go and check.
>
> Do we splatter __packed all over our ipv4/ipv6 header, TCP header, UDP
> header, etc. structures? No, we don't because it's totally unecessary.
Err, sure, right.
>
> I will not accept __packed being used unless it is absolutely, provably,
> the only way to solve a particular problem. And when that does happen,
> I am going to require a huge comment explaining in detail why this is
> the case, and why no other approach or solution solved the problem.
Would this be a candidate for checkpatch.pl?
Marcelo
^ permalink raw reply
* Re: [PATCHv3 net-next 3/4] sctp: add support for generating stream reconf add incoming/outgoing streams request chunk
From: David Miller @ 2017-01-23 16:17 UTC (permalink / raw)
To: marcelo.leitner
Cc: David.Laight, lucien.xin, netdev, linux-sctp, nhorman, vyasevich
In-Reply-To: <20170123161458.GZ3781@localhost.localdomain>
From: marcelo.leitner@gmail.com
Date: Mon, 23 Jan 2017 14:14:58 -0200
> On Mon, Jan 23, 2017 at 11:00:47AM -0500, David Miller wrote:
>> I will not accept __packed being used unless it is absolutely, provably,
>> the only way to solve a particular problem. And when that does happen,
>> I am going to require a huge comment explaining in detail why this is
>> the case, and why no other approach or solution solved the problem.
>
> Would this be a candidate for checkpatch.pl?
If this helps developers, sure.
^ permalink raw reply
* Re: [PATCH net-next] bnx2x: avoid two atomic ops per page on x86
From: David Miller @ 2017-01-23 16:18 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, krisman, Yuval.Mintz, ariel.elior
In-Reply-To: <1484929534.16328.134.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 20 Jan 2017 08:25:34 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> Commit 4cace675d687 ("bnx2x: Alloc 4k fragment for each rx ring buffer
> element") added extra put_page() and get_page() calls on arches where
> PAGE_SIZE=4K like x86
>
> Reorder things to avoid this overhead.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Gabriel Krisman Bertazi <krisman@linux.vnet.ibm.com>
> Cc: Yuval Mintz <Yuval.Mintz@cavium.com>
> Cc: Ariel Elior <ariel.elior@cavium.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v6 1/1] net sched actions: Add support for user cookies
From: Daniel Borkmann @ 2017-01-23 16:18 UTC (permalink / raw)
To: Simon Horman, Jamal Hadi Salim
Cc: davem, netdev, jiri, paulb, john.fastabend, mrv, hadarh, ogerlitz,
roid, xiyou.wangcong
In-Reply-To: <20170123125838.GD31958@penelope.horms.nl>
On 01/23/2017 01:58 PM, Simon Horman wrote:
> Hi Jamal,
>
> On Sun, Jan 22, 2017 at 03:25:50PM -0500, Jamal Hadi Salim wrote:
>
> ...
>
>> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
>> index cd08df9..58cf1c5 100644
>> --- a/net/sched/act_api.c
>> +++ b/net/sched/act_api.c
>> @@ -24,6 +24,7 @@
>> #include <net/net_namespace.h>
>> #include <net/sock.h>
>> #include <net/sch_generic.h>
>> +#include <net/pkt_cls.h>
>> #include <net/act_api.h>
>> #include <net/netlink.h>
>>
>> @@ -33,6 +34,8 @@ static void free_tcf(struct rcu_head *head)
>>
>> free_percpu(p->cpu_bstats);
>> free_percpu(p->cpu_qstats);
>> + kfree(p->act_cookie->data);
>
> Does the above need to be protected by a check for p->act_cookie being non-NULL?
Yep, that would be a NULL-deref. Why not just embedd tc_cookie as
suggested earlier, the struct is rather small anyway ...
>> + kfree(p->act_cookie);
>> kfree(p);
>> }
>>
>
> ...
>
>> @@ -575,6 +584,33 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla,
>> if (err < 0)
>> goto err_mod;
>>
>> + if (tb[TCA_ACT_COOKIE]) {
>> + int cklen = nla_len(tb[TCA_ACT_COOKIE]);
>> +
>> + if (cklen > TC_COOKIE_MAX_SIZE) {
>> + err = -EINVAL;
>> + tcf_hash_release(a, bind);
>> + goto err_mod;
>> + }
>> +
>> + a->act_cookie = kzalloc(sizeof(*a->act_cookie), GFP_KERNEL);
>> + if (!a->act_cookie) {
>> + err = -ENOMEM;
>> + tcf_hash_release(a, bind);
>> + goto err_mod;
>> + }
>> +
>> + a->act_cookie->data = nla_memdup(tb[TCA_ACT_COOKIE],
>> + GFP_KERNEL);
>> + if (!a->act_cookie->data) {
>> + err = -ENOMEM;
>> + kfree(a->act_cookie);
>> + tcf_hash_release(a, bind);
>> + goto err_mod;
>> + }
>> + a->act_cookie->len = cklen;
>
> FWIW, the above looks correct but it also looks like the error handling
> could be done less verbosely if the logic was moved to a separate function.
>
>> + }
>> +
>> /* module count goes up only when brand new policy is created
>> * if it exists and is only bound to in a_o->init() then
>> * ACT_P_CREATED is not returned (a zero is).
>> --
>> 1.9.1
>>
^ permalink raw reply
* Re: [PATCH net-next] net: stmicro: dwmac4 comments enhancement
From: David Miller @ 2017-01-23 16:20 UTC (permalink / raw)
To: Joao.Pinto; +Cc: peppe.cavallaro, alexandre.torgue, netdev
In-Reply-To: <4b0ae13cd5076f1830bae79e1ad9bd0542d0e2bf.1485186686.git.jpinto@synopsys.com>
From: Joao Pinto <Joao.Pinto@synopsys.com>
Date: Mon, 23 Jan 2017 15:56:24 +0000
> + * This is the driver for the DesignWare Ethernet QoS Controller.
This is fine.
> + * For historical reasons it started in 4.x version.
This provides no value whatsoever.
Please resist the urge to fidget and adjust random things, just get to
the point and make the minimum change necessary to achieve a specific,
well defined, constrained, objective.
Please!
I'm really getting irritated with how much effort is being expended
to make dilly-dally style changes that provide little or no value
to the driver.
Fix bugs and contribute new support for new chips, because this stuff
I'm seeing here is just eating up everyone's time rather than improving
the driver.
Thank you.
^ permalink raw reply
* Re: [RFC PATCH net-next 0/5] bridge: per vlan lwt and dst_metadata support
From: Jiri Benc @ 2017-01-23 16:24 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Jiri Pirko, netdev, davem, stephen, nikolay, tgraf, hannes,
pshelar, dsa, hadi
In-Reply-To: <58862BAA.4060001@cumulusnetworks.com>
On Mon, 23 Jan 2017 08:13:30 -0800, Roopa Prabhu wrote:
> And, a 'vlan-to-tunid' mapping is a very common configuration in L2 ethernet vpn configurations.
You have one particular and narrow use case in mind and are proposing a
rather large patchset to add support for that (and only that) single
use case, while we already have a generic mechanism in place to address
this and many similar (and dissimilar, too) use cases. That doesn't
sound right.
If the current generic mechanisms have bottlenecks for your use case,
let's work on removing those bottlenecks. That way, everybody benefits,
not just a single use case.
Thanks,
Jiri
^ permalink raw reply
* Re: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
From: Alexander Duyck @ 2017-01-23 16:28 UTC (permalink / raw)
To: David Laight, Tushar Dave
Cc: David Miller, maowenan@huawei.com, netdev@vger.kernel.org,
jeffrey.t.kirsher@intel.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB0268E25@AcuExch.aculab.com>
On Mon, Jan 23, 2017 at 1:44 AM, David Laight <David.Laight@aculab.com> wrote:
> Alexander Duyck
>> Sent: 19 January 2017 15:55
> ...
>> >> The Relaxed Ordering attribute doesn't get applied across the board.
>> >> It ends up being limited to a subset of the transactions if I recall
>> >> correctly. In this case it is the Tx descriptor write back, and the
>> >> Rx data write back. We don't apply the RO bit to any other
>> >> transactions.
>> >>
>> >> In the case of Tx descriptor there is no harm in allowing it to be
>> >> reordered because we only really read the DD bit so we don't care
>> >> about the ordering of the write back. In the case of the Rx data the
>> >> Rx descriptor essentially acts as a flush since it is sent without the
>> >> RO bit set. So all the writes before it must be completed before the
>> >> Rx descriptor write back.
>> >
>> > In which case why not set it unconditionally for all architectures?
>> >
>> > I'm surprised (I often am) that allowing those re-orderings makes
>> > any significant difference.
>> > Unfortunately you need a PCIe analyser to see what is really happening
>> > and they don't come cheap.
>> >
>> > What I do vaguely remember is that some hosts don't always implement
>> > the 'normal' re-ordering of reads and read completions.
>> > Re-ordering of reads allows descriptor reads to overtake transmit
>> > traffic which is likely to make a difference.
>>
>> I think part of the issue, at least in the case of SPARC, is that the
>> handling of the memory writes in the PCIe root complex is impacted by
>> the RO attribute. On the bus itself it doesn't matter much, but at
>> the root complex it can become expensive to have to wait on a partial
>> write to complete while there are other writes pending. This is why
>> the IOMMU for SPARC now has a WEAK_ORDERING attribute you can add so
>> that it can write the data in whatever order it wants in relation to
>> other writes in that region.
>
> I hope the IOMMU only ever reorders writes that have the RO bit set.
I'm assuming it only applies to DMA regions mapped with
DMA_ATTR_WEAK_ORDERING. Since drivers have to specify that attribute
it likely is only going to apply to DMA regions that could have the RO
bit set.
> Has anyone tried cache invalidates on the rx buffers?
> Might make the writes less expensive.
> Or is the issue with NUMA rather than cache.
I don't know. This is all very SPARC specific and I haven't done any
of the work on it. You might try checking with those responsible for
introducing DMA_ATTR_WEAK_ORDERING for the SPARC architecture.
- Alex
^ 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