Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1335540 > unrolled thread
| Started by | Gregory CLEMENT <gregory.clement@free-electrons.com> |
|---|---|
| First post | 2016-02-16 16:40 +0100 |
| Last post | 2016-02-18 12:50 +0100 |
| Articles | 6 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH v2 net-next 0/8] API set for HW Buffer management Gregory CLEMENT <gregory.clement@free-electrons.com> - 2016-02-16 16:40 +0100
[PATCH v2 4/8] ARM: dts: armada-xp: enable buffer manager support on Armada XP boards Gregory CLEMENT <gregory.clement@free-electrons.com> - 2016-02-16 16:40 +0100
Re: [PATCH v2 net-next 0/8] API set for HW Buffer management Willy Tarreau <w@1wt.eu> - 2016-02-18 00:10 +0100
Re: [PATCH v2 net-next 0/8] API set for HW Buffer management Gregory CLEMENT <gregory.clement@free-electrons.com> - 2016-02-18 18:40 +0100
Re: [PATCH v2 net-next 6/8] net: mvneta: bm: add support for hardware buffer management David Miller <davem@davemloft.net> - 2016-02-18 05:50 +0100
Re: [PATCH v2 net-next 6/8] net: mvneta: bm: add support for hardware buffer management Marcin Wojtas <mw@semihalf.com> - 2016-02-18 12:50 +0100
| From | Gregory CLEMENT <gregory.clement@free-electrons.com> |
|---|---|
| Date | 2016-02-16 16:40 +0100 |
| Subject | [PATCH v2 net-next 0/8] API set for HW Buffer management |
| Message-ID | <r2PTj-1fi-3@gated-at.bofh.it> |
Hello,
A few weeks ago I sent a proposal for a API set for HW Buffer
management, to have a better view of the motivation for this API see
the cover letter of this proposal:
http://thread.gmane.org/gmane.linux.kernel/2125152
Since this version I took into account the review from Florian:
- The hardware buffer management helpers are no more built by default
and now depend on a hidden config symbol which has to be selected
by the driver if needed
- The hwbm_pool_refill() and hwbm_pool_add() now receive a gfp_t as
argument allowing the caller to specify the flag it needs.
- buf_num is now tested to ensure there is no wrapping
- A spinlock has been added to protect the hwbm_pool_add() function in
SMP or irq context.
I also used pr_warn instead of pr_debug in case of errors.
I fixed the mvneta implementation by returning the buffer to the pool
at various place instead of ignoring it.
About the series itself I tried to make this series easier to merge:
- Squashed "bus: mvenus-mbus: Fix size test for
mvebu_mbus_get_dram_win_info" into bus: mvebu-mbus: provide api for
obtaining IO and DRAM window information.
- Added my signed-otf-by on all the patches as submitter of the series.
- Renamed the dts patches with the pattern "ARM: dts: platform:"
- Removed the patch "ARM: mvebu: enable SRAM support in
mvebu_v7_defconfig" of this series and already applied it
- Rodified the order of the patches.
In order to ease the test the branch mvneta-BM-framework-v2 is
available at git@github.com:MISL-EBU-System-SW/mainline-public.git.
David,
I would like to merge the 4 first patches myself once you will have
take the 4 last patches of the series. Patch 5 do not modify files in
the net subsystem, but as there is a build dependency it is better if
you merge it in the same time as the 3 other ones.
Thanks,
Gregory
Gregory CLEMENT (2):
net: add a hardware buffer management helper API
net: mvneta: Use the new hwbm framework
Marcin Wojtas (6):
ARM: dts: armada-38x: add buffer manager nodes
ARM: dts: armada-38x: enable buffer manager support on Armada 38x
boards
ARM: dts: armada-xp: add buffer manager nodes
ARM: dts: armada-xp: enable buffer manager support on Armada XP boards
bus: mvebu-mbus: provide api for obtaining IO and DRAM window
information
net: mvneta: bm: add support for hardware buffer management
.../bindings/net/marvell-armada-370-neta.txt | 19 +-
.../devicetree/bindings/net/marvell-neta-bm.txt | 49 +++
arch/arm/boot/dts/armada-385-db-ap.dts | 20 +-
arch/arm/boot/dts/armada-388-clearfog.dts | 6 +
arch/arm/boot/dts/armada-388-db.dts | 17 +-
arch/arm/boot/dts/armada-388-gp.dts | 17 +-
.../arm/boot/dts/armada-38x-solidrun-microsom.dtsi | 15 +-
arch/arm/boot/dts/armada-38x.dtsi | 18 +
arch/arm/boot/dts/armada-xp-db.dts | 19 +-
arch/arm/boot/dts/armada-xp-gp.dts | 19 +-
arch/arm/boot/dts/armada-xp.dtsi | 18 +
drivers/bus/mvebu-mbus.c | 51 +++
drivers/net/ethernet/marvell/Kconfig | 15 +
drivers/net/ethernet/marvell/Makefile | 1 +
drivers/net/ethernet/marvell/mvneta.c | 430 +++++++++++++++---
drivers/net/ethernet/marvell/mvneta_bm.c | 486 +++++++++++++++++++++
drivers/net/ethernet/marvell/mvneta_bm.h | 162 +++++++
include/linux/mbus.h | 3 +
include/net/hwbm.h | 21 +
net/Kconfig | 3 +
net/core/Makefile | 1 +
net/core/hwbm.c | 87 ++++
22 files changed, 1415 insertions(+), 62 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/marvell-neta-bm.txt
create mode 100644 drivers/net/ethernet/marvell/mvneta_bm.c
create mode 100644 drivers/net/ethernet/marvell/mvneta_bm.h
create mode 100644 include/net/hwbm.h
create mode 100644 net/core/hwbm.c
--
2.5.0
[toc] | [next] | [standalone]
| From | Gregory CLEMENT <gregory.clement@free-electrons.com> |
|---|---|
| Date | 2016-02-16 16:40 +0100 |
| Subject | [PATCH v2 4/8] ARM: dts: armada-xp: enable buffer manager support on Armada XP boards |
| Message-ID | <r2PTl-1fi-47@gated-at.bofh.it> |
| In reply to | #1335540 |
From: Marcin Wojtas <mw@semihalf.com>
Since mvneta driver supports using hardware buffer management (BM), in
order to use it, board files have to be adjusted accordingly. This commit
enables BM on AXP-DB and AXP-GP in same manner - because number of ports
on those boards is the same as number of possible pools, each port is
supposed to use single pool for all kind of packets.
Moreover appropriate entry is added to 'soc' node ranges, as well as "okay"
status for 'bm' and 'bm-bppi' (internal SRAM) nodes.
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
arch/arm/boot/dts/armada-xp-db.dts | 19 ++++++++++++++++++-
arch/arm/boot/dts/armada-xp-gp.dts | 19 ++++++++++++++++++-
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-db.dts b/arch/arm/boot/dts/armada-xp-db.dts
index f774101416a5..30657302305d 100644
--- a/arch/arm/boot/dts/armada-xp-db.dts
+++ b/arch/arm/boot/dts/armada-xp-db.dts
@@ -77,7 +77,8 @@
MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000
MBUS_ID(0x01, 0x2f) 0 0 0xf0000000 0x1000000
MBUS_ID(0x09, 0x09) 0 0 0xf8100000 0x10000
- MBUS_ID(0x09, 0x05) 0 0 0xf8110000 0x10000>;
+ MBUS_ID(0x09, 0x05) 0 0 0xf8110000 0x10000
+ MBUS_ID(0x0c, 0x04) 0 0 0xf1200000 0x100000>;
devbus-bootcs {
status = "okay";
@@ -181,21 +182,33 @@
status = "okay";
phy = <&phy0>;
phy-mode = "rgmii-id";
+ buffer-manager = <&bm>;
+ bm,pool-long = <0>;
};
ethernet@74000 {
status = "okay";
phy = <&phy1>;
phy-mode = "rgmii-id";
+ buffer-manager = <&bm>;
+ bm,pool-long = <1>;
};
ethernet@30000 {
status = "okay";
phy = <&phy2>;
phy-mode = "sgmii";
+ buffer-manager = <&bm>;
+ bm,pool-long = <2>;
};
ethernet@34000 {
status = "okay";
phy = <&phy3>;
phy-mode = "sgmii";
+ buffer-manager = <&bm>;
+ bm,pool-long = <3>;
+ };
+
+ bm@c0000 {
+ status = "okay";
};
mvsdio@d4000 {
@@ -230,5 +243,9 @@
};
};
};
+
+ bm-bppi {
+ status = "okay";
+ };
};
};
diff --git a/arch/arm/boot/dts/armada-xp-gp.dts b/arch/arm/boot/dts/armada-xp-gp.dts
index 4878d7353069..a1ded01d0c07 100644
--- a/arch/arm/boot/dts/armada-xp-gp.dts
+++ b/arch/arm/boot/dts/armada-xp-gp.dts
@@ -96,7 +96,8 @@
MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000
MBUS_ID(0x01, 0x2f) 0 0 0xf0000000 0x1000000
MBUS_ID(0x09, 0x09) 0 0 0xf8100000 0x10000
- MBUS_ID(0x09, 0x05) 0 0 0xf8110000 0x10000>;
+ MBUS_ID(0x09, 0x05) 0 0 0xf8110000 0x10000
+ MBUS_ID(0x0c, 0x04) 0 0 0xf1200000 0x100000>;
devbus-bootcs {
status = "okay";
@@ -196,21 +197,29 @@
status = "okay";
phy = <&phy0>;
phy-mode = "qsgmii";
+ buffer-manager = <&bm>;
+ bm,pool-long = <0>;
};
ethernet@74000 {
status = "okay";
phy = <&phy1>;
phy-mode = "qsgmii";
+ buffer-manager = <&bm>;
+ bm,pool-long = <1>;
};
ethernet@30000 {
status = "okay";
phy = <&phy2>;
phy-mode = "qsgmii";
+ buffer-manager = <&bm>;
+ bm,pool-long = <2>;
};
ethernet@34000 {
status = "okay";
phy = <&phy3>;
phy-mode = "qsgmii";
+ buffer-manager = <&bm>;
+ bm,pool-long = <3>;
};
/* Front-side USB slot */
@@ -235,6 +244,10 @@
};
};
+ bm@c0000 {
+ status = "okay";
+ };
+
nand@d0000 {
status = "okay";
num-cs = <1>;
@@ -243,5 +256,9 @@
nand-on-flash-bbt;
};
};
+
+ bm-bppi {
+ status = "okay";
+ };
};
};
--
2.5.0
[toc] | [prev] | [next] | [standalone]
| From | Willy Tarreau <w@1wt.eu> |
|---|---|
| Date | 2016-02-18 00:10 +0100 |
| Message-ID | <r3jol-4YQ-9@gated-at.bofh.it> |
| In reply to | #1335540 |
Hi Gregory, On Tue, Feb 16, 2016 at 04:33:35PM +0100, Gregory CLEMENT wrote: > Hello, > > A few weeks ago I sent a proposal for a API set for HW Buffer > management, to have a better view of the motivation for this API see > the cover letter of this proposal: > http://thread.gmane.org/gmane.linux.kernel/2125152 > > Since this version I took into account the review from Florian: > - The hardware buffer management helpers are no more built by default > and now depend on a hidden config symbol which has to be selected > by the driver if needed > - The hwbm_pool_refill() and hwbm_pool_add() now receive a gfp_t as > argument allowing the caller to specify the flag it needs. > - buf_num is now tested to ensure there is no wrapping > - A spinlock has been added to protect the hwbm_pool_add() function in > SMP or irq context. > > I also used pr_warn instead of pr_debug in case of errors. > > I fixed the mvneta implementation by returning the buffer to the pool > at various place instead of ignoring it. > > About the series itself I tried to make this series easier to merge: > - Squashed "bus: mvenus-mbus: Fix size test for > mvebu_mbus_get_dram_win_info" into bus: mvebu-mbus: provide api for > obtaining IO and DRAM window information. > - Added my signed-otf-by on all the patches as submitter of the series. > - Renamed the dts patches with the pattern "ARM: dts: platform:" > - Removed the patch "ARM: mvebu: enable SRAM support in > mvebu_v7_defconfig" of this series and already applied it > - Rodified the order of the patches. > > In order to ease the test the branch mvneta-BM-framework-v2 is > available at git@github.com:MISL-EBU-System-SW/mainline-public.git. Well, I tested this patch series on top of latest master (from today) on my fresh new clearfog board. I compared carefully with and without the patchset. My workload was haproxy receiving connections and forwarding them to my PC via the same port. I tested both with short connections (HTTP GET of an empty file) and long ones (1 MB or more). No trouble was detected at all, which is pretty good. I noticed a very tiny performance drop which is more noticeable on short connections (high packet rates), my forwarded connection rate went down from 17500/s to 17300/s. But I have not checked yet what can be tuned when using the BM, nor did I compare CPU usage. I remember having run some tests in the past, I guess it was on the XP-GP board, and noticed that the BM could save a significant amount of CPU and improve cache efficiency, so if this is the case here, we don't really care about a possible 1% performance drop. I'll try to provide more results as time permits. In the mean time if you want (or plan to submit a next batch), feel free to add a Tested-by: Willy Tarreau <w@1wt.eu>. cheers, Willy
[toc] | [prev] | [next] | [standalone]
| From | Gregory CLEMENT <gregory.clement@free-electrons.com> |
|---|---|
| Date | 2016-02-18 18:40 +0100 |
| Message-ID | <r3AIx-yZ-7@gated-at.bofh.it> |
| In reply to | #1336860 |
Hi Willy, On mer., févr. 17 2016, Willy Tarreau <w@1wt.eu> wrote: > Hi Gregory, > > On Tue, Feb 16, 2016 at 04:33:35PM +0100, Gregory CLEMENT wrote: >> Hello, >> >> A few weeks ago I sent a proposal for a API set for HW Buffer >> management, to have a better view of the motivation for this API see >> the cover letter of this proposal: >> http://thread.gmane.org/gmane.linux.kernel/2125152 >> >> Since this version I took into account the review from Florian: >> - The hardware buffer management helpers are no more built by default >> and now depend on a hidden config symbol which has to be selected >> by the driver if needed >> - The hwbm_pool_refill() and hwbm_pool_add() now receive a gfp_t as >> argument allowing the caller to specify the flag it needs. >> - buf_num is now tested to ensure there is no wrapping >> - A spinlock has been added to protect the hwbm_pool_add() function in >> SMP or irq context. >> >> I also used pr_warn instead of pr_debug in case of errors. >> >> I fixed the mvneta implementation by returning the buffer to the pool >> at various place instead of ignoring it. >> >> About the series itself I tried to make this series easier to merge: >> - Squashed "bus: mvenus-mbus: Fix size test for >> mvebu_mbus_get_dram_win_info" into bus: mvebu-mbus: provide api for >> obtaining IO and DRAM window information. >> - Added my signed-otf-by on all the patches as submitter of the series. >> - Renamed the dts patches with the pattern "ARM: dts: platform:" >> - Removed the patch "ARM: mvebu: enable SRAM support in >> mvebu_v7_defconfig" of this series and already applied it >> - Rodified the order of the patches. >> >> In order to ease the test the branch mvneta-BM-framework-v2 is >> available at git@github.com:MISL-EBU-System-SW/mainline-public.git. > > Well, I tested this patch series on top of latest master (from today) > on my fresh new clearfog board. I compared carefully with and without > the patchset. My workload was haproxy receiving connections and forwarding > them to my PC via the same port. I tested both with short connections > (HTTP GET of an empty file) and long ones (1 MB or more). No trouble > was detected at all, which is pretty good. I noticed a very tiny > performance drop which is more noticeable on short connections (high > packet rates), my forwarded connection rate went down from 17500/s to > 17300/s. But I have not checked yet what can be tuned when using the > BM, nor did I compare CPU usage. I remember having run some tests in > the past, I guess it was on the XP-GP board, and noticed that the BM > could save a significant amount of CPU and improve cache efficiency, > so if this is the case here, we don't really care about a possible 1% > performance drop. > > I'll try to provide more results as time permits. > > In the mean time if you want (or plan to submit a next batch), feel > free to add a Tested-by: Willy Tarreau <w@1wt.eu>. Great! thanks for testing. Gregory > > cheers, > Willy > -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
[toc] | [prev] | [next] | [standalone]
| From | David Miller <davem@davemloft.net> |
|---|---|
| Date | 2016-02-18 05:50 +0100 |
| Subject | Re: [PATCH v2 net-next 6/8] net: mvneta: bm: add support for hardware buffer management |
| Message-ID | <r3oHo-fk-11@gated-at.bofh.it> |
| In reply to | #1335540 |
From: Gregory CLEMENT <gregory.clement@free-electrons.com>
Date: Tue, 16 Feb 2016 16:33:41 +0100
> pp->dev = dev;
> SET_NETDEV_DEV(dev, &pdev->dev);
>
> + dev->features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO;
> + dev->hw_features |= dev->features;
> + dev->vlan_features |= dev->features;
> + dev->priv_flags |= IFF_UNICAST_FLT;
> + dev->gso_max_segs = MVNETA_MAX_TSO_SEGS;
> +
> + err = register_netdev(dev);
> + if (err < 0) {
> + dev_err(&pdev->dev, "failed to register\n");
> + goto err_free_stats;
> + }
> +
> + pp->id = dev->ifindex;
> +
> + /* Obtain access to BM resources if enabled and already initialized */
> + bm_node = of_parse_phandle(dn, "buffer-manager", 0);
> + if (bm_node && bm_node->data) {
This set of changes has a lot of problems.
First, the exact moment you call register_netdev() your device must be
fully initialized because ->open() can be invoked immediately. This
means you must take care of all of this buffer manager stuff before
calling register_netdev().
It must precisely be the last thing you invoke in your probe function
for this reason.
Also you are now adding conditionalized code to every fastpath in your
driver, that is rediculous and is going to hurt performance.
Add seperate code paths for the HWBM vs SWBM, and register a unique
set of netdev_ops as appropriate.
[toc] | [prev] | [next] | [standalone]
| From | Marcin Wojtas <mw@semihalf.com> |
|---|---|
| Date | 2016-02-18 12:50 +0100 |
| Subject | Re: [PATCH v2 net-next 6/8] net: mvneta: bm: add support for hardware buffer management |
| Message-ID | <r3vfQ-4SJ-5@gated-at.bofh.it> |
| In reply to | #1337025 |
Hi David,
2016-02-18 5:43 GMT+01:00 David Miller <davem@davemloft.net>:
> From: Gregory CLEMENT <gregory.clement@free-electrons.com>
> Date: Tue, 16 Feb 2016 16:33:41 +0100
>
>> pp->dev = dev;
>> SET_NETDEV_DEV(dev, &pdev->dev);
>>
>> + dev->features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO;
>> + dev->hw_features |= dev->features;
>> + dev->vlan_features |= dev->features;
>> + dev->priv_flags |= IFF_UNICAST_FLT;
>> + dev->gso_max_segs = MVNETA_MAX_TSO_SEGS;
>> +
>> + err = register_netdev(dev);
>> + if (err < 0) {
>> + dev_err(&pdev->dev, "failed to register\n");
>> + goto err_free_stats;
>> + }
>> +
>> + pp->id = dev->ifindex;
>> +
>> + /* Obtain access to BM resources if enabled and already initialized */
>> + bm_node = of_parse_phandle(dn, "buffer-manager", 0);
>> + if (bm_node && bm_node->data) {
>
> This set of changes has a lot of problems.
>
> First, the exact moment you call register_netdev() your device must be
> fully initialized because ->open() can be invoked immediately. This
> means you must take care of all of this buffer manager stuff before
> calling register_netdev().
>
> It must precisely be the last thing you invoke in your probe function
> for this reason.
Ok. I shifted register_netdev in order to obtain port id dynamically
from netdev's ifindex (needed to control port <-> pool mapping). If
this order of registration is problematic, I will add an ID property
to DT.
>
> Also you are now adding conditionalized code to every fastpath in your
> driver, that is rediculous and is going to hurt performance.
>
> Add seperate code paths for the HWBM vs SWBM, and register a unique
> set of netdev_ops as appropriate.
TX is untouched and BM support affects only open, stop and change_mtu
- whose execution is not problematic in terms of performance. However
there are a couple new conditions in mvneta_rx(). It can be reduced to
a single condition check, moved to NAPI callback. I'll try to refactor
code in a way to avoid code duplication. Please bear in mind I don't
want to register to different NAPI functions (exactly the same apart
from one line), as the driver can fall back to SWBM after e.g.
unsuccessful mtu change.
Best regards,
Marcin
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web