From: Kuniyuki Iwashima <kuniyu@amazon.com>
To: "David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: Kuniyuki Iwashima <kuniyu@amazon.com>,
Kuniyuki Iwashima <kuni1840@gmail.com>, <netdev@vger.kernel.org>
Subject: [PATCH v1 net-next 5/6] af_unix: Schedule GC based on graph state during sendmsg().
Date: Fri, 3 May 2024 15:31:49 -0700 [thread overview]
Message-ID: <20240503223150.6035-6-kuniyu@amazon.com> (raw)
In-Reply-To: <20240503223150.6035-1-kuniyu@amazon.com>
The conventional test to trigger GC was based on the number of inflight
sockets.
Now we have more reliable data indicating if the loop exists in the graph.
When the graph state is
1. UNIX_GRAPH_NOT_CYCLIC, do not scheudle GC
2. UNIX_GRAPH_MAYBE_CYCLIC, schedule GC if unix_tot_inflight > 16384
3. UNIX_GRAPH_CYCLIC, schedule GC if unix_graph_circles > 1024
1024 might sound much smaller than 16384, but if the number of loops
is larger than 1024, there must be something wrong.
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
net/unix/garbage.c | 44 ++++++++++++++++++++++++++++----------------
1 file changed, 28 insertions(+), 16 deletions(-)
diff --git a/net/unix/garbage.c b/net/unix/garbage.c
index 85c0500764d4..48cea3cf4a42 100644
--- a/net/unix/garbage.c
+++ b/net/unix/garbage.c
@@ -128,7 +128,7 @@ static void unix_update_graph(struct unix_vertex *vertex)
if (!vertex)
return;
- unix_graph_state = UNIX_GRAPH_MAYBE_CYCLIC;
+ WRITE_ONCE(unix_graph_state, UNIX_GRAPH_MAYBE_CYCLIC);
}
static LIST_HEAD(unix_unvisited_vertices);
@@ -533,7 +533,8 @@ static void unix_walk_scc(struct sk_buff_head *hitlist)
list_replace_init(&unix_visited_vertices, &unix_unvisited_vertices);
swap(unix_vertex_unvisited_index, unix_vertex_grouped_index);
- unix_graph_state = unix_graph_circles ? UNIX_GRAPH_CYCLIC : UNIX_GRAPH_NOT_CYCLIC;
+ WRITE_ONCE(unix_graph_state,
+ unix_graph_circles ? UNIX_GRAPH_CYCLIC : UNIX_GRAPH_NOT_CYCLIC);
}
static void unix_walk_scc_fast(struct sk_buff_head *hitlist)
@@ -555,7 +556,7 @@ static void unix_walk_scc_fast(struct sk_buff_head *hitlist)
if (scc_dead) {
unix_collect_skb(&scc, hitlist);
- unix_graph_circles--;
+ WRITE_ONCE(unix_graph_circles, unix_graph_circles - 1);
}
list_del(&scc);
@@ -564,7 +565,7 @@ static void unix_walk_scc_fast(struct sk_buff_head *hitlist)
list_replace_init(&unix_visited_vertices, &unix_unvisited_vertices);
if (!unix_graph_circles)
- unix_graph_state = UNIX_GRAPH_NOT_CYCLIC;
+ WRITE_ONCE(unix_graph_state, UNIX_GRAPH_NOT_CYCLIC);
}
static bool gc_in_progress;
@@ -608,27 +609,38 @@ void unix_gc(void)
queue_work(system_unbound_wq, &unix_gc_work);
}
-#define UNIX_INFLIGHT_TRIGGER_GC 16000
+#define UNIX_INFLIGHT_SANE_CIRCLES (1 << 10)
+#define UNIX_INFLIGHT_SANE_SOCKETS (1 << 14)
#define UNIX_INFLIGHT_SANE_USER (SCM_MAX_FD * 8)
static void __unix_schedule_gc(struct scm_fp_list *fpl)
{
- /* If number of inflight sockets is insane,
- * force a garbage collect right now.
- *
- * Paired with the WRITE_ONCE() in unix_inflight(),
- * unix_notinflight(), and __unix_gc().
+ unsigned char graph_state = READ_ONCE(unix_graph_state);
+ bool wait = false;
+
+ if (graph_state == UNIX_GRAPH_NOT_CYCLIC)
+ return;
+
+ /* If the number of inflight sockets or cyclic references
+ * is insane, schedule garbage collector if not running.
*/
- if (READ_ONCE(unix_tot_inflight) > UNIX_INFLIGHT_TRIGGER_GC &&
- !READ_ONCE(gc_in_progress))
- unix_gc();
+ if (graph_state == UNIX_GRAPH_CYCLIC) {
+ if (READ_ONCE(unix_graph_circles) < UNIX_INFLIGHT_SANE_CIRCLES)
+ return;
+ } else {
+ if (READ_ONCE(unix_tot_inflight) < UNIX_INFLIGHT_SANE_SOCKETS)
+ return;
+ }
/* Penalise users who want to send AF_UNIX sockets
* but whose sockets have not been received yet.
*/
- if (READ_ONCE(fpl->user->unix_inflight) < UNIX_INFLIGHT_SANE_USER)
- return;
+ if (READ_ONCE(fpl->user->unix_inflight) > UNIX_INFLIGHT_SANE_USER)
+ wait = true;
+
+ if (!READ_ONCE(gc_in_progress))
+ unix_gc();
- if (READ_ONCE(gc_in_progress))
+ if (wait)
flush_work(&unix_gc_work);
}
--
2.30.2
next prev parent reply other threads:[~2024-05-03 22:34 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-03 22:31 [PATCH v1 net-next 0/6] af_unix: GC cleanup and optimisation Kuniyuki Iwashima
2024-05-03 22:31 ` [PATCH v1 net-next 1/6] af_unix: Add dead flag to struct scm_fp_list Kuniyuki Iwashima
2024-05-03 22:31 ` [PATCH v1 net-next 2/6] af_unix: Save the number of loops in inflight graph Kuniyuki Iwashima
2024-05-07 13:54 ` Paolo Abeni
2024-05-07 16:11 ` Kuniyuki Iwashima
2024-05-08 10:08 ` Paolo Abeni
2024-05-08 17:05 ` Kuniyuki Iwashima
2024-05-03 22:31 ` [PATCH v1 net-next 3/6] af_unix: Manage inflight graph state as unix_graph_state Kuniyuki Iwashima
2024-05-03 22:31 ` [PATCH v1 net-next 4/6] af_unix: Move wait_for_unix_gc() to unix_prepare_fpl() Kuniyuki Iwashima
2024-05-03 22:31 ` Kuniyuki Iwashima [this message]
2024-05-03 22:31 ` [PATCH v1 net-next 6/6] af_unix: Schedule GC only if loop exists during close() Kuniyuki Iwashima
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240503223150.6035-6-kuniyu@amazon.com \
--to=kuniyu@amazon.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=kuni1840@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).