Re: [PATCH] RDS: sync congestion map updating

[Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org>]

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.

> 
> 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)
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