| Message ID | 20240223214003.17369-13-kuniyu@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | af_unix: Rework GC. | expand |
On Fri, 2024-02-23 at 13:40 -0800, Kuniyuki Iwashima wrote: > When iterating SCC, we call unix_vertex_dead() for each vertex > to check if the vertex is close()d and has no bridge to another > SCC. > > If both conditions are true for every vertex in SCC, we can > execute garbage collection for all skb in the SCC. > > The actual garbage collection is done in the following patch, > replacing the old implementation. > > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> > --- > net/unix/garbage.c | 37 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 36 insertions(+), 1 deletion(-) > > diff --git a/net/unix/garbage.c b/net/unix/garbage.c > index 0eb1610c96d7..060e81be3614 100644 > --- a/net/unix/garbage.c > +++ b/net/unix/garbage.c > @@ -288,6 +288,32 @@ void unix_destroy_fpl(struct scm_fp_list *fpl) > unix_free_vertices(fpl); > } > > +static bool unix_vertex_dead(struct unix_vertex *vertex) > +{ > + struct unix_edge *edge; > + struct unix_sock *u; > + long total_ref; > + > + list_for_each_entry(edge, &vertex->edges, vertex_entry) { > + struct unix_vertex *next_vertex = unix_edge_successor(edge); > + > + if (!next_vertex) > + return false; > + > + if (next_vertex->scc_index != vertex->scc_index) > + return false; > + } > + > + edge = list_first_entry(&vertex->edges, typeof(*edge), vertex_entry); > + u = edge->predecessor; > + total_ref = file_count(u->sk.sk_socket->file); > + > + if (total_ref != vertex->out_degree) > + return false; > + > + return true; > +} > + > static bool unix_scc_cyclic(struct list_head *scc) > { > struct unix_vertex *vertex; > @@ -350,6 +376,7 @@ static void __unix_walk_scc(struct unix_vertex *vertex) > > if (vertex->index == vertex->scc_index) { > struct list_head scc; > + bool dead = true; Very minor nit: the above variable 'sounds' alike referring to the current vertex, while instead it tracks a global status, what about 'all_dead' or 'gc_all'? Thanks, Paolo
From: Paolo Abeni <pabeni@redhat.com> Date: Tue, 27 Feb 2024 12:25:56 +0100 > On Fri, 2024-02-23 at 13:40 -0800, Kuniyuki Iwashima wrote: > > When iterating SCC, we call unix_vertex_dead() for each vertex > > to check if the vertex is close()d and has no bridge to another > > SCC. > > > > If both conditions are true for every vertex in SCC, we can > > execute garbage collection for all skb in the SCC. > > > > The actual garbage collection is done in the following patch, > > replacing the old implementation. > > > > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> > > --- > > net/unix/garbage.c | 37 ++++++++++++++++++++++++++++++++++++- > > 1 file changed, 36 insertions(+), 1 deletion(-) > > > > diff --git a/net/unix/garbage.c b/net/unix/garbage.c > > index 0eb1610c96d7..060e81be3614 100644 > > --- a/net/unix/garbage.c > > +++ b/net/unix/garbage.c > > @@ -288,6 +288,32 @@ void unix_destroy_fpl(struct scm_fp_list *fpl) > > unix_free_vertices(fpl); > > } > > > > +static bool unix_vertex_dead(struct unix_vertex *vertex) > > +{ > > + struct unix_edge *edge; > > + struct unix_sock *u; > > + long total_ref; > > + > > + list_for_each_entry(edge, &vertex->edges, vertex_entry) { > > + struct unix_vertex *next_vertex = unix_edge_successor(edge); > > + > > + if (!next_vertex) > > + return false; > > + > > + if (next_vertex->scc_index != vertex->scc_index) > > + return false; > > + } > > + > > + edge = list_first_entry(&vertex->edges, typeof(*edge), vertex_entry); > > + u = edge->predecessor; > > + total_ref = file_count(u->sk.sk_socket->file); > > + > > + if (total_ref != vertex->out_degree) > > + return false; > > + > > + return true; > > +} > > + > > static bool unix_scc_cyclic(struct list_head *scc) > > { > > struct unix_vertex *vertex; > > @@ -350,6 +376,7 @@ static void __unix_walk_scc(struct unix_vertex *vertex) > > > > if (vertex->index == vertex->scc_index) { > > struct list_head scc; > > + bool dead = true; > > Very minor nit: the above variable 'sounds' alike referring to the > current vertex, while instead it tracks a global status, what about > 'all_dead' or 'gc_all'? Exactly. I'll use scc_dead or gc_scc to be consistent with unix_vertex_dead(). Thanks!
diff --git a/net/unix/garbage.c b/net/unix/garbage.c index 0eb1610c96d7..060e81be3614 100644 --- a/net/unix/garbage.c +++ b/net/unix/garbage.c @@ -288,6 +288,32 @@ void unix_destroy_fpl(struct scm_fp_list *fpl) unix_free_vertices(fpl); } +static bool unix_vertex_dead(struct unix_vertex *vertex) +{ + struct unix_edge *edge; + struct unix_sock *u; + long total_ref; + + list_for_each_entry(edge, &vertex->edges, vertex_entry) { + struct unix_vertex *next_vertex = unix_edge_successor(edge); + + if (!next_vertex) + return false; + + if (next_vertex->scc_index != vertex->scc_index) + return false; + } + + edge = list_first_entry(&vertex->edges, typeof(*edge), vertex_entry); + u = edge->predecessor; + total_ref = file_count(u->sk.sk_socket->file); + + if (total_ref != vertex->out_degree) + return false; + + return true; +} + static bool unix_scc_cyclic(struct list_head *scc) { struct unix_vertex *vertex; @@ -350,6 +376,7 @@ static void __unix_walk_scc(struct unix_vertex *vertex) if (vertex->index == vertex->scc_index) { struct list_head scc; + bool dead = true; __list_cut_position(&scc, &vertex_stack, &vertex->scc_entry); @@ -357,6 +384,9 @@ static void __unix_walk_scc(struct unix_vertex *vertex) list_move_tail(&vertex->entry, &unix_visited_vertices); vertex->index = unix_vertex_grouped_index; + + if (dead) + dead = unix_vertex_dead(vertex); } if (!unix_graph_maybe_cyclic) @@ -392,13 +422,18 @@ static void unix_walk_scc_fast(void) while (!list_empty(&unix_unvisited_vertices)) { struct unix_vertex *vertex; struct list_head scc; + bool dead = true; vertex = list_first_entry(&unix_unvisited_vertices, typeof(*vertex), entry); list_add(&scc, &vertex->scc_entry); - list_for_each_entry_reverse(vertex, &scc, scc_entry) + list_for_each_entry_reverse(vertex, &scc, scc_entry) { list_move_tail(&vertex->entry, &unix_visited_vertices); + if (dead) + dead = unix_vertex_dead(vertex); + } + list_del(&scc); }
When iterating SCC, we call unix_vertex_dead() for each vertex to check if the vertex is close()d and has no bridge to another SCC. If both conditions are true for every vertex in SCC, we can execute garbage collection for all skb in the SCC. The actual garbage collection is done in the following patch, replacing the old implementation. Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> --- net/unix/garbage.c | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-)