* [PATCH] rds: rds-stress show all zeros after few minutes
@ 2016-03-31 0:50 shamir rabinovitch
2016-03-31 13:13 ` Sergei Shtylyov
2016-04-01 3:59 ` santosh.shilimkar
0 siblings, 2 replies; 6+ messages in thread
From: shamir rabinovitch @ 2016-03-31 0:50 UTC (permalink / raw)
To: rds-devel, netdev; +Cc: davem, shamir.rabinovitch
Issue can be seen on platforms that use 8K and above page size
while rds fragment size is 4K. On those platforms single page is
shared between 2 or more rds fragments. Each fragment has it's own
offeset and rds cong map code need to take this offset to account.
Not taking this offset to account lead to reading the data fragment
as congestion map fragment and hang of the rds transmit due to far
cong map corruption.
Reviewed-by: Wengang Wang <wen.gang.wang@oracle.com>
Reviewed-by: Ajaykumar Hotchandani <ajaykumar.hotchandani@oracle.com>
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
Tested-by: Anand Bibhuti <anand.bibhuti@oracle.com>
Signed-off-by: shamir rabinovitch <shamir.rabinovitch@oracle.com>
---
net/rds/ib_recv.c | 2 +-
net/rds/iw_recv.c | 2 +-
net/rds/page.c | 5 +++--
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
index 977fb86..abc8cc8 100644
--- a/net/rds/ib_recv.c
+++ b/net/rds/ib_recv.c
@@ -796,7 +796,7 @@ static void rds_ib_cong_recv(struct rds_connection *conn,
addr = kmap_atomic(sg_page(&frag->f_sg));
- src = addr + frag_off;
+ src = addr + frag->f_sg.offset + frag_off;
dst = (void *)map->m_page_addrs[map_page] + map_off;
for (k = 0; k < to_copy; k += 8) {
/* Record ports that became uncongested, ie
diff --git a/net/rds/iw_recv.c b/net/rds/iw_recv.c
index a66d179..62a1738 100644
--- a/net/rds/iw_recv.c
+++ b/net/rds/iw_recv.c
@@ -585,7 +585,7 @@ static void rds_iw_cong_recv(struct rds_connection *conn,
addr = kmap_atomic(frag->f_page);
- src = addr + frag_off;
+ src = addr + frag->f_offset + frag_off;
dst = (void *)map->m_page_addrs[map_page] + map_off;
for (k = 0; k < to_copy; k += 8) {
/* Record ports that became uncongested, ie
diff --git a/net/rds/page.c b/net/rds/page.c
index 5a14e6d..715cbaa 100644
--- a/net/rds/page.c
+++ b/net/rds/page.c
@@ -135,8 +135,9 @@ int rds_page_remainder_alloc(struct scatterlist *scat, unsigned long bytes,
if (rem->r_offset != 0)
rds_stats_inc(s_page_remainder_hit);
- rem->r_offset += bytes;
- if (rem->r_offset == PAGE_SIZE) {
+ /* some hw (e.g. sparc) require aligned memory */
+ rem->r_offset += ALIGN(bytes, 8);
+ if (rem->r_offset >= PAGE_SIZE) {
__free_page(rem->r_page);
rem->r_page = NULL;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] rds: rds-stress show all zeros after few minutes
2016-03-31 0:50 [PATCH] rds: rds-stress show all zeros after few minutes shamir rabinovitch
@ 2016-03-31 13:13 ` Sergei Shtylyov
2016-03-31 14:20 ` Shamir Rabinovitch
2016-04-01 3:59 ` santosh.shilimkar
1 sibling, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2016-03-31 13:13 UTC (permalink / raw)
To: shamir rabinovitch, rds-devel, netdev; +Cc: davem
Hello.
On 3/31/2016 3:50 AM, shamir rabinovitch wrote:
> Issue can be seen on platforms that use 8K and above page size
> while rds fragment size is 4K. On those platforms single page is
> shared between 2 or more rds fragments. Each fragment has it's own
Its.
> offeset and rds cong map code need to take this offset to account.
Offset. What is "cong", congestion?
> Not taking this offset to account lead to reading the data fragment
> as congestion map fragment and hang of the rds transmit due to far
> cong map corruption.
>
> Reviewed-by: Wengang Wang <wen.gang.wang@oracle.com>
> Reviewed-by: Ajaykumar Hotchandani <ajaykumar.hotchandani@oracle.com>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
> Tested-by: Anand Bibhuti <anand.bibhuti@oracle.com>
These should be after your sign-off, not before.
> Signed-off-by: shamir rabinovitch <shamir.rabinovitch@oracle.com>
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rds: rds-stress show all zeros after few minutes
2016-03-31 13:13 ` Sergei Shtylyov
@ 2016-03-31 14:20 ` Shamir Rabinovitch
2016-03-31 14:23 ` Sergei Shtylyov
0 siblings, 1 reply; 6+ messages in thread
From: Shamir Rabinovitch @ 2016-03-31 14:20 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: rds-devel, netdev, davem
On Thu, Mar 31, 2016 at 04:13:35PM +0300, Sergei Shtylyov wrote:
> Hello.
>
> On 3/31/2016 3:50 AM, shamir rabinovitch wrote:
>
> >Issue can be seen on platforms that use 8K and above page size
> >while rds fragment size is 4K. On those platforms single page is
> >shared between 2 or more rds fragments. Each fragment has it's own
>
> Its.
Fixed in next version.
Thanks.
>
> >offeset and rds cong map code need to take this offset to account.
>
> Offset. What is "cong", congestion?
'Offset' is in middle of the sentence so it is OK as-is.
Cong is short hand of congestion.
It will be replaces
with the full word in next version.
>
> >Not taking this offset to account lead to reading the data fragment
> >as congestion map fragment and hang of the rds transmit due to far
> >cong map corruption.
> >
> >Reviewed-by: Wengang Wang <wen.gang.wang@oracle.com>
> >Reviewed-by: Ajaykumar Hotchandani <ajaykumar.hotchandani@oracle.com>
> >Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
> >Tested-by: Anand Bibhuti <anand.bibhuti@oracle.com>
>
> These should be after your sign-off, not before.
Thanks for the comment.
Will be fixed in next version.
>
> >Signed-off-by: shamir rabinovitch <shamir.rabinovitch@oracle.com>
> [...]
>
> MBR, Sergei
>
Thanks for the review.
BR, Shamir
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rds: rds-stress show all zeros after few minutes
2016-03-31 14:20 ` Shamir Rabinovitch
@ 2016-03-31 14:23 ` Sergei Shtylyov
2016-03-31 14:25 ` Shamir Rabinovitch
0 siblings, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2016-03-31 14:23 UTC (permalink / raw)
To: Shamir Rabinovitch; +Cc: rds-devel, netdev, davem
On 3/31/2016 5:20 PM, Shamir Rabinovitch wrote:
>>> Issue can be seen on platforms that use 8K and above page size
>>> while rds fragment size is 4K. On those platforms single page is
>>> shared between 2 or more rds fragments. Each fragment has it's own
>>
>> Its.
>
> Fixed in next version.
> Thanks.
>
>>
>>> offeset and rds cong map code need to take this offset to account.
>>
>> Offset. What is "cong", congestion?
>
> 'Offset' is in middle of the sentence so it is OK as-is.
No, "offeset" is not OK. :-)
[...]
>>> Signed-off-by: shamir rabinovitch <shamir.rabinovitch@oracle.com>
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rds: rds-stress show all zeros after few minutes
2016-03-31 14:23 ` Sergei Shtylyov
@ 2016-03-31 14:25 ` Shamir Rabinovitch
0 siblings, 0 replies; 6+ messages in thread
From: Shamir Rabinovitch @ 2016-03-31 14:25 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: rds-devel, netdev, davem
On Thu, Mar 31, 2016 at 05:23:30PM +0300, Sergei Shtylyov wrote:
> On 3/31/2016 5:20 PM, Shamir Rabinovitch wrote:
>
> >>>Issue can be seen on platforms that use 8K and above page size
> >>>while rds fragment size is 4K. On those platforms single page is
> >>>shared between 2 or more rds fragments. Each fragment has it's own
> >>
> >> Its.
> >
> >Fixed in next version.
> >Thanks.
> >
> >>
> >>>offeset and rds cong map code need to take this offset to account.
> >>
> >> Offset. What is "cong", congestion?
> >
> >'Offset' is in middle of the sentence so it is OK as-is.
>
> No, "offeset" is not OK. :-)
Oh. Typo. Sorry. Will fix! :-)
>
> [...]
>
> >>>Signed-off-by: shamir rabinovitch <shamir.rabinovitch@oracle.com>
> [...]
>
> MBR, Sergei
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rds: rds-stress show all zeros after few minutes
2016-03-31 0:50 [PATCH] rds: rds-stress show all zeros after few minutes shamir rabinovitch
2016-03-31 13:13 ` Sergei Shtylyov
@ 2016-04-01 3:59 ` santosh.shilimkar
1 sibling, 0 replies; 6+ messages in thread
From: santosh.shilimkar @ 2016-04-01 3:59 UTC (permalink / raw)
To: shamir rabinovitch, rds-devel, netdev; +Cc: davem
Hi Shamir,
Nice to see this one soon on the list,
Just to make $subject more relevant. How about below?
RDS: fix congestion map corruption for PAGE_SIZE > 8k
On 3/30/16 5:50 PM, shamir rabinovitch wrote:
> Issue can be seen on platforms that use 8K and above page size
> while rds fragment size is 4K. On those platforms single page is
> shared between 2 or more rds fragments. Each fragment has it's own
> offeset and rds cong map code need to take this offset to account.
> Not taking this offset to account lead to reading the data fragment
> as congestion map fragment and hang of the rds transmit due to far
> cong map corruption.
>
> Reviewed-by: Wengang Wang <wen.gang.wang@oracle.com>
> Reviewed-by: Ajaykumar Hotchandani <ajaykumar.hotchandani@oracle.com>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>
> Tested-by: Anand Bibhuti <anand.bibhuti@oracle.com>
>
> Signed-off-by: shamir rabinovitch <shamir.rabinovitch@oracle.com>
> ---
> net/rds/ib_recv.c | 2 +-
> net/rds/iw_recv.c | 2 +-
> net/rds/page.c | 5 +++--
> 3 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/net/rds/ib_recv.c b/net/rds/ib_recv.c
> index 977fb86..abc8cc8 100644
> --- a/net/rds/ib_recv.c
> +++ b/net/rds/ib_recv.c
> @@ -796,7 +796,7 @@ static void rds_ib_cong_recv(struct rds_connection *conn,
>
> addr = kmap_atomic(sg_page(&frag->f_sg));
>
> - src = addr + frag_off;
> + src = addr + frag->f_sg.offset + frag_off;
> dst = (void *)map->m_page_addrs[map_page] + map_off;
> for (k = 0; k < to_copy; k += 8) {
> /* Record ports that became uncongested, ie
> diff --git a/net/rds/iw_recv.c b/net/rds/iw_recv.c
If you refresh the patch against 4.6-rc1, you won't need to
patch iw_recv.c :-)
> diff --git a/net/rds/page.c b/net/rds/page.c
> index 5a14e6d..715cbaa 100644
> --- a/net/rds/page.c
> +++ b/net/rds/page.c
> @@ -135,8 +135,9 @@ int rds_page_remainder_alloc(struct scatterlist *scat, unsigned long bytes,
> if (rem->r_offset != 0)
> rds_stats_inc(s_page_remainder_hit);
>
> - rem->r_offset += bytes;
> - if (rem->r_offset == PAGE_SIZE) {
> + /* some hw (e.g. sparc) require aligned memory */
> + rem->r_offset += ALIGN(bytes, 8);
> + if (rem->r_offset >= PAGE_SIZE) {
> __free_page(rem->r_page);
> rem->r_page = NULL;
> }
>
This hunk I missed out looks like. This doesn't belong to the
$subject patch. Could you please add this in separate patch. I
will need more than just "some hw (e.g. sparc) require aligned memory"
Once you fix these, please repost the updated version, and I will add
them to the 4.7 queue. Thanks !!
Regards,
Santosh
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-01 3:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-31 0:50 [PATCH] rds: rds-stress show all zeros after few minutes shamir rabinovitch
2016-03-31 13:13 ` Sergei Shtylyov
2016-03-31 14:20 ` Shamir Rabinovitch
2016-03-31 14:23 ` Sergei Shtylyov
2016-03-31 14:25 ` Shamir Rabinovitch
2016-04-01 3:59 ` santosh.shilimkar
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).