Re: [PATCH] RDS: sync congestion map updating

[Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>]

Hi Leon,

在 2016年03月31日 00:19, Leon Romanovsky 写道:
> On Wed, Mar 30, 2016 at 05:08:22PM +0800, Wengang Wang wrote:
>> Problem is found that some among a lot of parallel RDS communications hang.
>> In my test ten or so among 33 communications hang. The send requests got
>> -ENOBUF error meaning the peer socket (port) is congested. But meanwhile,
>> peer socket (port) is not congested.
>>
>> The congestion map updating can happen in two paths: one is in rds_recvmsg path
>> and the other is when it receives packets from the hardware. There is no
>> synchronization when updating the congestion map. So a bit operation (clearing)
>> in the rds_recvmsg path can be skipped by another bit operation (setting) in
>> hardware packet receving path.
>>
>> Fix is to add a spin lock per congestion map to sync the update on it.
>> No performance drop found during the test for the fix.
> I assume that this change fixed your issue, however it looks suspicious
> that performance wasn't change.
Sure it I verified that patch fixes the issue.
For performance, I will reply to Santosh's email later, please check there.
>> Signed-off-by: Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>> ---
>>   net/rds/cong.c | 7 +++++++
>>   net/rds/rds.h  | 1 +
>>   2 files changed, 8 insertions(+)
> According to get_maintainer script, you send this patch to wrong lists
> and persons.
>
> ➜  linux git:(master) ./scripts/get_maintainer.pl -f net/rds/cong.c
> Santosh Shilimkar <santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> (supporter:RDS - RELIABLE DATAGRAM SOCKETS)
> "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> (maintainer:NETWORKING [GENERAL])
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org (open list:RDS - RELIABLE DATAGRAM SOCKETS)
> linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org (open list:RDS - RELIABLE DATAGRAM SOCKETS)

So linux-rdma is here :)

thanks,
wengang
> rds-devel-N0ozoZBvEnrZJqsBc5GL+g@public.gmane.org (moderated list:RDS - RELIABLE DATAGRAM SOCKETS)
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org (open list)
>
>> diff --git a/net/rds/cong.c b/net/rds/cong.c
>> index e6144b8..7afc1bf 100644
>> --- a/net/rds/cong.c
>> +++ b/net/rds/cong.c
>> @@ -144,6 +144,7 @@ static struct rds_cong_map *rds_cong_from_addr(__be32 addr)
>>   	if (!map)
>>   		return NULL;
>>   
>> +	spin_lock_init(&map->m_lock);
>>   	map->m_addr = addr;
>>   	init_waitqueue_head(&map->m_waitq);
>>   	INIT_LIST_HEAD(&map->m_conn_list);
>> @@ -292,6 +293,7 @@ void rds_cong_set_bit(struct rds_cong_map *map, __be16 port)
>>   {
>>   	unsigned long i;
>>   	unsigned long off;
>> +	unsigned long flags;
>>   
>>   	rdsdebug("setting congestion for %pI4:%u in map %p\n",
>>   	  &map->m_addr, ntohs(port), map);
>> @@ -299,13 +301,16 @@ void rds_cong_set_bit(struct rds_cong_map *map, __be16 port)
>>   	i = be16_to_cpu(port) / RDS_CONG_MAP_PAGE_BITS;
>>   	off = be16_to_cpu(port) % RDS_CONG_MAP_PAGE_BITS;
>>   
>> +	spin_lock_irqsave(&map->m_lock, flags);
>>   	__set_bit_le(off, (void *)map->m_page_addrs[i]);
>> +	spin_unlock_irqrestore(&map->m_lock, flags);
>>   }
>>   
>>   void rds_cong_clear_bit(struct rds_cong_map *map, __be16 port)
>>   {
>>   	unsigned long i;
>>   	unsigned long off;
>> +	unsigned long flags;
>>   
>>   	rdsdebug("clearing congestion for %pI4:%u in map %p\n",
>>   	  &map->m_addr, ntohs(port), map);
>> @@ -313,7 +318,9 @@ void rds_cong_clear_bit(struct rds_cong_map *map, __be16 port)
>>   	i = be16_to_cpu(port) / RDS_CONG_MAP_PAGE_BITS;
>>   	off = be16_to_cpu(port) % RDS_CONG_MAP_PAGE_BITS;
>>   
>> +	spin_lock_irqsave(&map->m_lock, flags);
>>   	__clear_bit_le(off, (void *)map->m_page_addrs[i]);
>> +	spin_unlock_irqrestore(&map->m_lock, flags);
>>   }
>>   
>>   static int rds_cong_test_bit(struct rds_cong_map *map, __be16 port)
>> diff --git a/net/rds/rds.h b/net/rds/rds.h
>> index 80256b0..f359cf8 100644
>> --- a/net/rds/rds.h
>> +++ b/net/rds/rds.h
>> @@ -59,6 +59,7 @@ struct rds_cong_map {
>>   	__be32			m_addr;
>>   	wait_queue_head_t	m_waitq;
>>   	struct list_head	m_conn_list;
>> +	spinlock_t		m_lock;
>>   	unsigned long		m_page_addrs[RDS_CONG_MAP_PAGES];
>>   };
>>   
>> -- 
>> 2.1.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html