Jon, please take a look at my comments in another thread. Hope we can
solve the zero-timeout crash as soon as possible.

Thanks,
Ying

On 01/07/2018 11:18 PM, Jon Maloy wrote:
> Thank you for your thorough review, Ying. I am actually wondering if it would 
> be possible to merge the RECLAIMED and REMITTED state too, but I need to 
> think more about that. Anyway, I'll leave the FSM at this stage for now.
> 
> On another  note; are you doing any work on the topology server? We need a 
> fix to at least the zero-timeout crash upstream soon, since we want to use 
> 4.15 code in one of our products in the near future.
> 
> BR
> ///jon
> 
> 
> 
>> -----Original Message-----
>> From: Jon Maloy
>> Sent: Thursday, January 04, 2018 15:26
>> To: Jon Maloy <[email protected]>; Jon Maloy
>> <[email protected]>
>> Cc: Mohan Krishna Ghanta Krishnamurthy
>> <[email protected]>; Tung Quang Nguyen
>> <[email protected]>; Hoang Huu Le
>> <[email protected]>; Canh Duc Luu
>> <[email protected]>; Ying Xue <[email protected]>;
>> [email protected]; [email protected]; tipc-
>> [email protected]
>> Subject: [net-next v2 9/9] tipc: improve poll() for group member socket
>>
>> The current criteria for returning POLLOUT from a group member socket is
>> too simplistic. It basically returns POLLOUT as soon as the group has 
>> external
>> destinations, something obviously leading to a lot of spinning during
>> destination congestion situations. At the same time, the internal congestion
>> handling is unnecessarily complex.
>>
>> We now change this as follows.
>>
>> - We introduce an 'open' flag in  struct tipc_group. This flag is used
>>   only to help poll() get the setting of POLLOUT right, and *not* for
>>   congeston handling as such. This means that a user can choose to
>>   ignore an  EAGAIN for a destination and go on sending messages to
>>   other destinations in the group if he wants to.
>>
>> - The flag is set to false every time we return EAGAIN on a send call.
>>
>> - The flag is set to true every time any member, i.e., not necessarily
>>   the member that caused EAGAIN, is removed from the small_win list.
>>
>> - We remove the group member 'usr_pending' flag. The size of the send
>>   window and presence in the 'small_win' list is sufficient criteria
>>   for recognizing congestion.
>>
>> This solution seems to be a reasonable compromise between 'anycast',
>> which is normally not waiting for POLLOUT for a specific destination, and the
>> other three send modes, which are.
>>
>> Signed-off-by: Jon Maloy <[email protected]>
>> ---
>>  net/tipc/group.c  | 64 +++++++++++++++++++++++++++++++-----------------
>> -------
>>  net/tipc/group.h  |  2 +-
>>  net/tipc/socket.c |  5 ++---
>>  3 files changed, 39 insertions(+), 32 deletions(-)
>>
>> diff --git a/net/tipc/group.c b/net/tipc/group.c index 5c3d527e..bbeb8a5
>> 100644
>> --- a/net/tipc/group.c
>> +++ b/net/tipc/group.c
>> @@ -74,7 +74,6 @@ struct tipc_member {
>>  u16 bc_rcv_nxt;
>>  u16 bc_syncpt;
>>  u16 bc_acked;
>> -bool usr_pending;
>>  };
>>
>>  struct tipc_group {
>> @@ -96,11 +95,27 @@ struct tipc_group {
>>  u16 bc_ackers;
>>  bool loopback;
>>  bool events;
>> +bool open;
>>  };
>>
>>  static void tipc_group_proto_xmit(struct tipc_group *grp, struct
>> tipc_member *m,
>>    int mtyp, struct sk_buff_head *xmitq);
>>
>> +bool tipc_group_is_open(struct tipc_group *grp) {
>> +return grp->open;
>> +}
>> +
>> +static void tipc_group_open(struct tipc_member *m, bool *wakeup) {
>> +*wakeup = false;
>> +if (list_empty(&m->small_win))
>> +return;
>> +list_del_init(&m->small_win);
>> +m->group->open = true;
>> +*wakeup = true;
>> +}
>> +
>>  static void tipc_group_decr_active(struct tipc_group *grp,
>>     struct tipc_member *m)
>>  {
>> @@ -406,20 +421,20 @@ bool tipc_group_cong(struct tipc_group *grp, u32
>> dnode, u32 dport,
>>  int adv, state;
>>
>>  m = tipc_group_find_dest(grp, dnode, dport);
>> -*mbr = m;
>> -if (!m)
>> +if (!tipc_group_is_receiver(m)) {
>> +*mbr = NULL;
>>  return false;
>> -if (m->usr_pending)
>> -return true;
>> +}
>> +*mbr = m;
>> +
>>  if (m->window >= len)
>>  return false;
>> -m->usr_pending = true;
>> +
>> +grp->open = false;
>>
>>  /* If not fully advertised, do it now to prevent mutual blocking */
>>  adv = m->advertised;
>>  state = m->state;
>> -if (state < MBR_JOINED)
>> -return true;
>>  if (state == MBR_JOINED && adv == ADV_IDLE)
>>  return true;
>>  if (state == MBR_ACTIVE && adv == ADV_ACTIVE) @@ -437,9
>> +452,10 @@ bool tipc_group_bc_cong(struct tipc_group *grp, int len)
>>  struct tipc_member *m = NULL;
>>
>>  /* If prev bcast was replicast, reject until all receivers have acked */
>> -if (grp->bc_ackers)
>> +if (grp->bc_ackers) {
>> +grp->open = false;
>>  return true;
>> -
>> +}
>>  if (list_empty(&grp->small_win))
>>  return false;
>>
>> @@ -753,9 +769,7 @@ void tipc_group_proto_rcv(struct tipc_group *grp,
>> bool *usr_wakeup,
>>
>>  /* Member can be taken into service */
>>  m->state = MBR_JOINED;
>> -*usr_wakeup = true;
>> -m->usr_pending = false;
>> -list_del_init(&m->small_win);
>> +tipc_group_open(m, usr_wakeup);
>>  tipc_group_update_member(m, 0);
>>  tipc_group_proto_xmit(grp, m, GRP_ADV_MSG, xmitq);
>>  tipc_group_create_event(grp, m, TIPC_PUBLISHED, @@ -
>> 766,8 +780,7 @@ void tipc_group_proto_rcv(struct tipc_group *grp, bool
>> *usr_wakeup,
>>  return;
>>  m->bc_syncpt = msg_grp_bc_syncpt(hdr);
>>  list_del_init(&m->list);
>> -list_del_init(&m->small_win);
>> -*usr_wakeup = true;
>> +tipc_group_open(m, usr_wakeup);
>>  tipc_group_decr_active(grp, m);
>>  m->state = MBR_LEAVING;
>>  tipc_group_create_event(grp, m, TIPC_WITHDRAWN, @@ -
>> 777,26 +790,25 @@ void tipc_group_proto_rcv(struct tipc_group *grp, bool
>> *usr_wakeup,
>>  if (!m)
>>  return;
>>  m->window += msg_adv_win(hdr);
>> -*usr_wakeup = m->usr_pending;
>> -m->usr_pending = false;
>> -list_del_init(&m->small_win);
>> +tipc_group_open(m, usr_wakeup);
>>  return;
>>  case GRP_ACK_MSG:
>>  if (!m)
>>  return;
>>  m->bc_acked = msg_grp_bc_acked(hdr);
>>  if (--grp->bc_ackers)
>> -break;
>> +return;
>> +list_del_init(&m->small_win);
>> +m->group->open = true;
>>  *usr_wakeup = true;
>> -m->usr_pending = false;
>> +tipc_group_update_member(m, 0);
>>  return;
>>  case GRP_RECLAIM_MSG:
>>  if (!m)
>>  return;
>> -*usr_wakeup = m->usr_pending;
>> -m->usr_pending = false;
>>  tipc_group_proto_xmit(grp, m, GRP_REMIT_MSG, xmitq);
>>  m->window = ADV_IDLE;
>> +tipc_group_open(m, usr_wakeup);
>>  return;
>>  case GRP_REMIT_MSG:
>>  if (!m || m->state != MBR_RECLAIMING) @@ -885,9 +897,7
>> @@ void tipc_group_member_evt(struct tipc_group *grp,
>>  /* Member can be taken into service */
>>  m->instance = instance;
>>  m->state = MBR_JOINED;
>> -*usr_wakeup = true;
>> -m->usr_pending = false;
>> -list_del_init(&m->small_win);
>> +tipc_group_open(m, usr_wakeup);
>>  tipc_group_update_member(m, 0);
>>  tipc_group_proto_xmit(grp, m, GRP_JOIN_MSG, xmitq);
>>  tipc_group_create_event(grp, m, TIPC_PUBLISHED, @@ -
>> 897,12 +907,10 @@ void tipc_group_member_evt(struct tipc_group *grp,
>>  if (!m)
>>  break;
>>
>> -*usr_wakeup = true;
>> -m->usr_pending = false;
>>  tipc_group_decr_active(grp, m);
>>  m->state = MBR_LEAVING;
>>  list_del_init(&m->list);
>> -list_del_init(&m->small_win);
>> +tipc_group_open(m, usr_wakeup);
>>
>>  /* Only send event if no LEAVE message can be expected */
>>  if (!tipc_node_is_up(net, node))
>> diff --git a/net/tipc/group.h b/net/tipc/group.h index 71663dc..8bc5bbd
>> 100644
>> --- a/net/tipc/group.h
>> +++ b/net/tipc/group.h
>> @@ -67,9 +67,9 @@ void tipc_group_update_bc_members(struct
>> tipc_group *grp, int len, bool ack);  bool tipc_group_cong(struct tipc_group
>> *grp, u32 dnode, u32 dport,
>>       int len, struct tipc_member **m);  bool
>> tipc_group_bc_cong(struct tipc_group *grp, int len);
>> +bool tipc_group_is_open(struct tipc_group *grp);
>>  void tipc_group_update_rcv_win(struct tipc_group *grp, int blks, u32 node,
>>         u32 port, struct sk_buff_head *xmitq);
>>  u16 tipc_group_bc_snd_nxt(struct tipc_group *grp);  void
>> tipc_group_update_member(struct tipc_member *m, int len); -int
>> tipc_group_size(struct tipc_group *grp);  #endif diff --git 
>> a/net/tipc/socket.c
>> b/net/tipc/socket.c index 7129fe5..e004ee4 100644
>> --- a/net/tipc/socket.c
>> +++ b/net/tipc/socket.c
>> @@ -736,9 +736,8 @@ static unsigned int tipc_poll(struct file *file, struct
>> socket *sock,
>>  revents |= POLLIN | POLLRDNORM;
>>  break;
>>  case TIPC_OPEN:
>> -if (!grp || tipc_group_size(grp))
>> -if (!tsk->cong_link_cnt)
>> -revents |= POLLOUT;
>> +if ((!grp || tipc_group_is_open(grp)) && !tsk-
>>> cong_link_cnt)
>> +revents |= POLLOUT;
>>  if (!tipc_sk_type_connectionless(sk))
>>  break;
>>  if (skb_queue_empty(&sk->sk_receive_queue))
>> --
>> 2.1.4
> 
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
tipc-discussion mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Reply via email to