* [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
@ 2017-01-09 5:32 Mao Wenan
2017-01-09 12:07 ` maowenan
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Mao Wenan @ 2017-01-09 5:32 UTC (permalink / raw)
To: netdev, jeffrey.t.kirsher, alexander.duyck
Relax ordering(RO) is one feature of 82599 NIC, to enable this feature can
enhance the performance for some cpu architecure, such as SPARC and so on.
Currently it only supports one special cpu architecture(SPARC) in 82599
driver to enable RO feature, this is not very common for other cpu architecture
which really needs RO feature.
This patch add one common config CONFIG_ARCH_WANT_RELAX_ORDER to set RO feature,
and should define CONFIG_ARCH_WANT_RELAX_ORDER in sparc Kconfig firstly.
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
arch/Kconfig | 3 +++
arch/sparc/Kconfig | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 2 +-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 99839c2..bd04eac 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -781,4 +781,7 @@ config VMAP_STACK
the stack to map directly to the KASAN shadow map using a formula
that is incorrect if the stack is in vmalloc space.
+config ARCH_WANT_RELAX_ORDER
+ bool
+
source "kernel/gcov/Kconfig"
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index cf4034c..68ac5c7 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -44,6 +44,7 @@ config SPARC
select CPU_NO_EFFICIENT_FFS
select HAVE_ARCH_HARDENED_USERCOPY
select PROVE_LOCKING_SMALL if PROVE_LOCKING
+ select ARCH_WANT_RELAX_ORDER
config SPARC32
def_bool !64BIT
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
index 094e1d6..c38d50c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
@@ -350,7 +350,7 @@ s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
}
IXGBE_WRITE_FLUSH(hw);
-#ifndef CONFIG_SPARC
+#ifndef CONFIG_ARCH_WANT_RELAX_ORDER
/* Disable relaxed ordering */
for (i = 0; i < hw->mac.max_tx_queues; i++) {
u32 regval;
--
2.7.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* RE: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
2017-01-09 5:32 [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering Mao Wenan
@ 2017-01-09 12:07 ` maowenan
2017-01-16 4:40 ` David Miller
2017-01-17 19:15 ` David Miller
2 siblings, 0 replies; 12+ messages in thread
From: maowenan @ 2017-01-09 12:07 UTC (permalink / raw)
To: maowenan, netdev@vger.kernel.org, jeffrey.t.kirsher@intel.com,
alexander.duyck@gmail.com
Cc: Dingtianhong, weiyongjun (A)
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Mao Wenan
> Sent: Monday, January 09, 2017 1:33 PM
> To: netdev@vger.kernel.org; jeffrey.t.kirsher@intel.com;
> alexander.duyck@gmail.com
> Subject: [PATCH v2 net-next] net:add one common config
> ARCH_WANT_RELAX_ORDER to support relax ordering.
>
> Relax ordering(RO) is one feature of 82599 NIC, to enable this feature can
> enhance the performance for some cpu architecure, such as SPARC and so on.
> Currently it only supports one special cpu architecture(SPARC) in 82599 driver
> to enable RO feature, this is not very common for other cpu architecture which
> really needs RO feature.
> This patch add one common config CONFIG_ARCH_WANT_RELAX_ORDER to
> set RO feature, and should define CONFIG_ARCH_WANT_RELAX_ORDER in
> sparc Kconfig firstly.
>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
> arch/Kconfig | 3 +++
> arch/sparc/Kconfig | 1 +
> drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 2 +-
> 3 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig index 99839c2..bd04eac 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -781,4 +781,7 @@ config VMAP_STACK
> the stack to map directly to the KASAN shadow map using a formula
> that is incorrect if the stack is in vmalloc space.
>
> +config ARCH_WANT_RELAX_ORDER
> + bool
> +
> source "kernel/gcov/Kconfig"
> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig index cf4034c..68ac5c7
> 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -44,6 +44,7 @@ config SPARC
> select CPU_NO_EFFICIENT_FFS
> select HAVE_ARCH_HARDENED_USERCOPY
> select PROVE_LOCKING_SMALL if PROVE_LOCKING
> + select ARCH_WANT_RELAX_ORDER
>
> config SPARC32
> def_bool !64BIT
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> index 094e1d6..c38d50c 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> @@ -350,7 +350,7 @@ s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
> }
> IXGBE_WRITE_FLUSH(hw);
>
> -#ifndef CONFIG_SPARC
> +#ifndef CONFIG_ARCH_WANT_RELAX_ORDER
> /* Disable relaxed ordering */
> for (i = 0; i < hw->mac.max_tx_queues; i++) {
> u32 regval;
> --
> 2.7.0
>
Hi Alex, Is there any improvement for this patch?
@Jeff, do you have any comments about this patch?
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
2017-01-09 5:32 [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering Mao Wenan
2017-01-09 12:07 ` maowenan
@ 2017-01-16 4:40 ` David Miller
2017-01-17 19:15 ` David Miller
2 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2017-01-16 4:40 UTC (permalink / raw)
To: maowenan; +Cc: netdev, jeffrey.t.kirsher, alexander.duyck
From: Mao Wenan <maowenan@huawei.com>
Date: Mon, 9 Jan 2017 13:32:34 +0800
> Relax ordering(RO) is one feature of 82599 NIC, to enable this feature can
> enhance the performance for some cpu architecure, such as SPARC and so on.
> Currently it only supports one special cpu architecture(SPARC) in 82599
> driver to enable RO feature, this is not very common for other cpu architecture
> which really needs RO feature.
> This patch add one common config CONFIG_ARCH_WANT_RELAX_ORDER to set RO feature,
> and should define CONFIG_ARCH_WANT_RELAX_ORDER in sparc Kconfig firstly.
>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
Alexander and Jeff, please review.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
2017-01-09 5:32 [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering Mao Wenan
2017-01-09 12:07 ` maowenan
2017-01-16 4:40 ` David Miller
@ 2017-01-17 19:15 ` David Miller
2017-01-17 19:27 ` Alexander Duyck
2017-01-18 16:22 ` David Laight
2 siblings, 2 replies; 12+ messages in thread
From: David Miller @ 2017-01-17 19:15 UTC (permalink / raw)
To: maowenan; +Cc: netdev, jeffrey.t.kirsher, alexander.duyck
From: Mao Wenan <maowenan@huawei.com>
Date: Mon, 9 Jan 2017 13:32:34 +0800
> Relax ordering(RO) is one feature of 82599 NIC, to enable this feature can
> enhance the performance for some cpu architecure, such as SPARC and so on.
> Currently it only supports one special cpu architecture(SPARC) in 82599
> driver to enable RO feature, this is not very common for other cpu architecture
> which really needs RO feature.
> This patch add one common config CONFIG_ARCH_WANT_RELAX_ORDER to set RO feature,
> and should define CONFIG_ARCH_WANT_RELAX_ORDER in sparc Kconfig firstly.
>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
Since no-one has reviewed this patch, and I do not feel comfortable with applying
it without such review, I am tossing this patch.
If someone eventually reviews it, repost this patch.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
2017-01-17 19:15 ` David Miller
@ 2017-01-17 19:27 ` Alexander Duyck
2017-01-18 1:07 ` maowenan
2017-01-18 16:22 ` David Laight
1 sibling, 1 reply; 12+ messages in thread
From: Alexander Duyck @ 2017-01-17 19:27 UTC (permalink / raw)
To: David Miller; +Cc: maowenan, Netdev, Jeff Kirsher
On Tue, Jan 17, 2017 at 11:15 AM, David Miller <davem@davemloft.net> wrote:
> From: Mao Wenan <maowenan@huawei.com>
> Date: Mon, 9 Jan 2017 13:32:34 +0800
>
>> Relax ordering(RO) is one feature of 82599 NIC, to enable this feature can
>> enhance the performance for some cpu architecure, such as SPARC and so on.
>> Currently it only supports one special cpu architecture(SPARC) in 82599
>> driver to enable RO feature, this is not very common for other cpu architecture
>> which really needs RO feature.
>> This patch add one common config CONFIG_ARCH_WANT_RELAX_ORDER to set RO feature,
>> and should define CONFIG_ARCH_WANT_RELAX_ORDER in sparc Kconfig firstly.
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>
> Since no-one has reviewed this patch, and I do not feel comfortable with applying
> it without such review, I am tossing this patch.
>
> If someone eventually reviews it, repost this patch.
Mao,
Go ahead and repost the patch and feel free to add my Reviewed-by.
Sorry I didn't reply to this earlier but I have been getting over the
flu for the last week or so.
- Alex
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
2017-01-17 19:27 ` Alexander Duyck
@ 2017-01-18 1:07 ` maowenan
0 siblings, 0 replies; 12+ messages in thread
From: maowenan @ 2017-01-18 1:07 UTC (permalink / raw)
To: Alexander Duyck, David Miller; +Cc: Netdev, Jeff Kirsher
> -----Original Message-----
> From: Alexander Duyck [mailto:alexander.duyck@gmail.com]
> Sent: Wednesday, January 18, 2017 3:28 AM
> To: David Miller
> Cc: maowenan; Netdev; Jeff Kirsher
> Subject: Re: [PATCH v2 net-next] net:add one common config
> ARCH_WANT_RELAX_ORDER to support relax ordering.
>
> On Tue, Jan 17, 2017 at 11:15 AM, David Miller <davem@davemloft.net>
> wrote:
> > From: Mao Wenan <maowenan@huawei.com>
> > Date: Mon, 9 Jan 2017 13:32:34 +0800
> >
> >> Relax ordering(RO) is one feature of 82599 NIC, to enable this
> >> feature can enhance the performance for some cpu architecure, such as
> SPARC and so on.
> >> Currently it only supports one special cpu architecture(SPARC) in
> >> 82599 driver to enable RO feature, this is not very common for other
> >> cpu architecture which really needs RO feature.
> >> This patch add one common config CONFIG_ARCH_WANT_RELAX_ORDER to
> set
> >> RO feature, and should define CONFIG_ARCH_WANT_RELAX_ORDER in
> sparc Kconfig firstly.
> >>
> >> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> >
> > Since no-one has reviewed this patch, and I do not feel comfortable
> > with applying it without such review, I am tossing this patch.
> >
> > If someone eventually reviews it, repost this patch.
>
> Mao,
>
> Go ahead and repost the patch and feel free to add my Reviewed-by.
> Sorry I didn't reply to this earlier but I have been getting over the flu for the last
> week or so.
>
> - Alex
Hi Alex,
I have reposted the patch(V3), thanks a lot.
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
2017-01-17 19:15 ` David Miller
2017-01-17 19:27 ` Alexander Duyck
@ 2017-01-18 16:22 ` David Laight
2017-01-18 17:25 ` Alexander Duyck
1 sibling, 1 reply; 12+ messages in thread
From: David Laight @ 2017-01-18 16:22 UTC (permalink / raw)
To: 'David Miller', maowenan@huawei.com
Cc: netdev@vger.kernel.org, jeffrey.t.kirsher@intel.com,
alexander.duyck@gmail.com
From: David Miller
> Sent: 17 January 2017 19:16
> > Relax ordering(RO) is one feature of 82599 NIC, to enable this feature can
> > enhance the performance for some cpu architecure, such as SPARC and so on.
> > Currently it only supports one special cpu architecture(SPARC) in 82599
> > driver to enable RO feature, this is not very common for other cpu architecture
> > which really needs RO feature.
> > This patch add one common config CONFIG_ARCH_WANT_RELAX_ORDER to set RO feature,
> > and should define CONFIG_ARCH_WANT_RELAX_ORDER in sparc Kconfig firstly.
> >
> > Signed-off-by: Mao Wenan <maowenan@huawei.com>
>
> Since no-one has reviewed this patch, and I do not feel comfortable with applying
> it without such review, I am tossing this patch.
>
> If someone eventually reviews it, repost this patch.
Having re-read parts of the PCIe spec I think I'd like someone to
explain exactly which transfers are affected by the 'relaxed ordering'
bit and why any re-ordered transactions aren't a problem.
In particular I believe RO allows the write to update the receive
descriptor ring to overtake a write of receive packet data.
That could lead to the network stack processing a receive frame
before it has actually been written.
David
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
2017-01-18 16:22 ` David Laight
@ 2017-01-18 17:25 ` Alexander Duyck
2017-01-19 11:43 ` David Laight
0 siblings, 1 reply; 12+ messages in thread
From: Alexander Duyck @ 2017-01-18 17:25 UTC (permalink / raw)
To: David Laight
Cc: David Miller, maowenan@huawei.com, netdev@vger.kernel.org,
jeffrey.t.kirsher@intel.com
On Wed, Jan 18, 2017 at 8:22 AM, David Laight <David.Laight@aculab.com> wrote:
> From: David Miller
>> Sent: 17 January 2017 19:16
>> > Relax ordering(RO) is one feature of 82599 NIC, to enable this feature can
>> > enhance the performance for some cpu architecure, such as SPARC and so on.
>> > Currently it only supports one special cpu architecture(SPARC) in 82599
>> > driver to enable RO feature, this is not very common for other cpu architecture
>> > which really needs RO feature.
>> > This patch add one common config CONFIG_ARCH_WANT_RELAX_ORDER to set RO feature,
>> > and should define CONFIG_ARCH_WANT_RELAX_ORDER in sparc Kconfig firstly.
>> >
>> > Signed-off-by: Mao Wenan <maowenan@huawei.com>
>>
>> Since no-one has reviewed this patch, and I do not feel comfortable with applying
>> it without such review, I am tossing this patch.
>>
>> If someone eventually reviews it, repost this patch.
>
> Having re-read parts of the PCIe spec I think I'd like someone to
> explain exactly which transfers are affected by the 'relaxed ordering'
> bit and why any re-ordered transactions aren't a problem.
>
> In particular I believe RO allows the write to update the receive
> descriptor ring to overtake a write of receive packet data.
> That could lead to the network stack processing a receive frame
> before it has actually been written.
>
> David
>
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.
- Alex
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
2017-01-18 17:25 ` Alexander Duyck
@ 2017-01-19 11:43 ` David Laight
2017-01-19 15:54 ` Alexander Duyck
0 siblings, 1 reply; 12+ messages in thread
From: David Laight @ 2017-01-19 11:43 UTC (permalink / raw)
To: 'Alexander Duyck'
Cc: David Miller, maowenan@huawei.com, netdev@vger.kernel.org,
jeffrey.t.kirsher@intel.com
From: Alexander Duyck
> Sent: 18 January 2017 17:25
> On Wed, Jan 18, 2017 at 8:22 AM, David Laight <David.Laight@aculab.com> wrote:
> > From: David Miller
> >> Sent: 17 January 2017 19:16
> >> > Relax ordering(RO) is one feature of 82599 NIC, to enable this feature can
> >> > enhance the performance for some cpu architecure, such as SPARC and so on.
> >> > Currently it only supports one special cpu architecture(SPARC) in 82599
> >> > driver to enable RO feature, this is not very common for other cpu architecture
> >> > which really needs RO feature.
> >> > This patch add one common config CONFIG_ARCH_WANT_RELAX_ORDER to set RO feature,
> >> > and should define CONFIG_ARCH_WANT_RELAX_ORDER in sparc Kconfig firstly.
> >> >
> >> > Signed-off-by: Mao Wenan <maowenan@huawei.com>
> >>
> >> Since no-one has reviewed this patch, and I do not feel comfortable with applying
> >> it without such review, I am tossing this patch.
> >>
> >> If someone eventually reviews it, repost this patch.
> >
> > Having re-read parts of the PCIe spec I think I'd like someone to
> > explain exactly which transfers are affected by the 'relaxed ordering'
> > bit and why any re-ordered transactions aren't a problem.
> >
> > In particular I believe RO allows the write to update the receive
> > descriptor ring to overtake a write of receive packet data.
> > That could lead to the network stack processing a receive frame
> > before it has actually been written.
> >
> > David
> >
>
> 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.
David
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
2017-01-19 11:43 ` David Laight
@ 2017-01-19 15:54 ` Alexander Duyck
2017-01-23 9:44 ` David Laight
0 siblings, 1 reply; 12+ messages in thread
From: Alexander Duyck @ 2017-01-19 15:54 UTC (permalink / raw)
To: David Laight
Cc: David Miller, maowenan@huawei.com, netdev@vger.kernel.org,
jeffrey.t.kirsher@intel.com
On Thu, Jan 19, 2017 at 3:43 AM, David Laight <David.Laight@aculab.com> wrote:
> From: Alexander Duyck
>> Sent: 18 January 2017 17:25
>> On Wed, Jan 18, 2017 at 8:22 AM, David Laight <David.Laight@aculab.com> wrote:
>> > From: David Miller
>> >> Sent: 17 January 2017 19:16
>> >> > Relax ordering(RO) is one feature of 82599 NIC, to enable this feature can
>> >> > enhance the performance for some cpu architecure, such as SPARC and so on.
>> >> > Currently it only supports one special cpu architecture(SPARC) in 82599
>> >> > driver to enable RO feature, this is not very common for other cpu architecture
>> >> > which really needs RO feature.
>> >> > This patch add one common config CONFIG_ARCH_WANT_RELAX_ORDER to set RO feature,
>> >> > and should define CONFIG_ARCH_WANT_RELAX_ORDER in sparc Kconfig firstly.
>> >> >
>> >> > Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> >>
>> >> Since no-one has reviewed this patch, and I do not feel comfortable with applying
>> >> it without such review, I am tossing this patch.
>> >>
>> >> If someone eventually reviews it, repost this patch.
>> >
>> > Having re-read parts of the PCIe spec I think I'd like someone to
>> > explain exactly which transfers are affected by the 'relaxed ordering'
>> > bit and why any re-ordered transactions aren't a problem.
>> >
>> > In particular I believe RO allows the write to update the receive
>> > descriptor ring to overtake a write of receive packet data.
>> > That could lead to the network stack processing a receive frame
>> > before it has actually been written.
>> >
>> > David
>> >
>>
>> 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.
- Alex
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
2017-01-19 15:54 ` Alexander Duyck
@ 2017-01-23 9:44 ` David Laight
2017-01-23 16:28 ` Alexander Duyck
0 siblings, 1 reply; 12+ messages in thread
From: David Laight @ 2017-01-23 9:44 UTC (permalink / raw)
To: 'Alexander Duyck'
Cc: David Miller, maowenan@huawei.com, netdev@vger.kernel.org,
jeffrey.t.kirsher@intel.com
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.
Has anyone tried cache invalidates on the rx buffers?
Might make the writes less expensive.
Or is the issue with NUMA rather than cache.
David
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering.
2017-01-23 9:44 ` David Laight
@ 2017-01-23 16:28 ` Alexander Duyck
0 siblings, 0 replies; 12+ messages in thread
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
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 [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-01-23 16:28 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-09 5:32 [PATCH v2 net-next] net:add one common config ARCH_WANT_RELAX_ORDER to support relax ordering Mao Wenan
2017-01-09 12:07 ` maowenan
2017-01-16 4:40 ` David Miller
2017-01-17 19:15 ` David Miller
2017-01-17 19:27 ` Alexander Duyck
2017-01-18 1:07 ` maowenan
2017-01-18 16:22 ` David Laight
2017-01-18 17:25 ` Alexander Duyck
2017-01-19 11:43 ` David Laight
2017-01-19 15:54 ` Alexander Duyck
2017-01-23 9:44 ` David Laight
2017-01-23 16:28 ` Alexander Duyck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).