diff mbox series

[net-next] tipc: clarify meaning of 'inactive' field in struct tipc_subscription

Message ID 20220329173218.1737499-1-jmaloy@redhat.com (mailing list archive)
State Deferred
Delegated to: Netdev Maintainers
Headers show
Series [net-next] tipc: clarify meaning of 'inactive' field in struct tipc_subscription | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 1 maintainers not CCed: pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 81 lines checked
netdev/kdoc fail Errors and warnings before: 0 this patch: 1
netdev/source_inline success Was 0 now: 0

Commit Message

Jon Maloy March 29, 2022, 5:32 p.m. UTC 
From: Jon Maloy <jmaloy@redhat.com>

struct tipc_subscription has a boolean field 'inactive' which purpose
is not immediately obvious. When the subscription timer expires we are
still in interrupt context, and cannot easily just delete the
subscription. We therefore delay that action until the expiration
event has reached the work queue context where it is being sent to the
user. However, in the meantime other events may occur, which must be
suppressed to avoid any unexpected behavior.

We now clarify this with renaming the field and adding a comment.

Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
 net/tipc/subscr.c | 10 ++++++----
 net/tipc/subscr.h |  4 ++--
 net/tipc/topsrv.c |  7 ++++---
 3 files changed, 12 insertions(+), 9 deletions(-)

Comments

Jakub Kicinski March 30, 2022, 1:22 a.m. UTC | #1 
On Tue, 29 Mar 2022 13:32:18 -0400 jmaloy@redhat.com wrote:
> From: Jon Maloy <jmaloy@redhat.com>
> 
> struct tipc_subscription has a boolean field 'inactive' which purpose
> is not immediately obvious. When the subscription timer expires we are
> still in interrupt context, and cannot easily just delete the
> subscription. We therefore delay that action until the expiration
> event has reached the work queue context where it is being sent to the
> user. However, in the meantime other events may occur, which must be
> suppressed to avoid any unexpected behavior.
> 
> We now clarify this with renaming the field and adding a comment.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>

# Form letter - net-next is closed

We have already sent the networking pull request for 5.18
and therefore net-next is closed for new drivers, features,
code refactoring and optimizations. We are currently accepting
bug fixes only.

Please repost when net-next reopens after 5.18-rc1 is cut.

RFC patches sent for review only are obviously welcome at any time.
diff mbox series

Patch

diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
index 05d49ad81290..094a5bf5145c 100644
--- a/net/tipc/subscr.c
+++ b/net/tipc/subscr.c
@@ -3,7 +3,7 @@ 
  *
  * Copyright (c) 2000-2017, Ericsson AB
  * Copyright (c) 2005-2007, 2010-2013, Wind River Systems
- * Copyright (c) 2020-2021, Red Hat Inc
+ * Copyright (c) 2020-2022, Red Hat Inc
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -46,7 +46,7 @@  static void tipc_sub_send_event(struct tipc_subscription *sub,
 	struct tipc_subscr *s = &sub->evt.s;
 	struct tipc_event *evt = &sub->evt;
 
-	if (sub->inactive)
+	if (sub->expired)
 		return;
 	tipc_evt_write(evt, event, event);
 	if (p) {
@@ -109,7 +109,9 @@  static void tipc_sub_timeout(struct timer_list *t)
 
 	spin_lock(&sub->lock);
 	tipc_sub_send_event(sub, NULL, TIPC_SUBSCR_TIMEOUT);
-	sub->inactive = true;
+
+	/* Block for more events until sub can be deleted from work context */
+	sub->expired = true;
 	spin_unlock(&sub->lock);
 }
 
@@ -152,7 +154,7 @@  struct tipc_subscription *tipc_sub_subscribe(struct net *net,
 	INIT_LIST_HEAD(&sub->sub_list);
 	sub->net = net;
 	sub->conid = conid;
-	sub->inactive = false;
+	sub->expired = false;
 	memcpy(&sub->evt.s, s, sizeof(*s));
 	sub->s.seq.type = tipc_sub_read(s, seq.type);
 	sub->s.seq.lower = lower;
diff --git a/net/tipc/subscr.h b/net/tipc/subscr.h
index 60b877531b66..1af00c69cd6c 100644
--- a/net/tipc/subscr.h
+++ b/net/tipc/subscr.h
@@ -3,7 +3,7 @@ 
  *
  * Copyright (c) 2003-2017, Ericsson AB
  * Copyright (c) 2005-2007, 2012-2013, Wind River Systems
- * Copyright (c) 2020-2021, Red Hat Inc
+ * Copyright (c) 2020-2022, Red Hat Inc
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -69,7 +69,7 @@  struct tipc_subscription {
 	struct list_head service_list;
 	struct list_head sub_list;
 	int conid;
-	bool inactive;
+	bool expired;
 	spinlock_t lock;
 };
 
diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c
index 5522865deae9..2d0e044a2524 100644
--- a/net/tipc/topsrv.c
+++ b/net/tipc/topsrv.c
@@ -3,6 +3,7 @@ 
  *
  * Copyright (c) 2012-2013, Wind River Systems
  * Copyright (c) 2017-2018, Ericsson AB
+ * Copyright (c) 2020-2022, Redhat Inc
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -105,7 +106,7 @@  struct tipc_conn {
 
 /* An entry waiting to be sent */
 struct outqueue_entry {
-	bool inactive;
+	bool expired;
 	struct tipc_event evt;
 	struct list_head list;
 };
@@ -261,7 +262,7 @@  static void tipc_conn_send_to_sock(struct tipc_conn *con)
 		evt = &e->evt;
 		spin_unlock_bh(&con->outqueue_lock);
 
-		if (e->inactive)
+		if (e->expired)
 			tipc_conn_delete_sub(con, &evt->s);
 
 		memset(&msg, 0, sizeof(msg));
@@ -325,7 +326,7 @@  void tipc_topsrv_queue_evt(struct net *net, int conid,
 	e = kmalloc(sizeof(*e), GFP_ATOMIC);
 	if (!e)
 		goto err;
-	e->inactive = (event == TIPC_SUBSCR_TIMEOUT);
+	e->expired = (event == TIPC_SUBSCR_TIMEOUT);
 	memcpy(&e->evt, evt, sizeof(*evt));
 	spin_lock_bh(&con->outqueue_lock);
 	list_add_tail(&e->list, &con->outqueue);