netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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 v3 net-next 00/14] af_unix: Rework GC.
Date: Fri, 23 Feb 2024 13:39:49 -0800	[thread overview]
Message-ID: <20240223214003.17369-1-kuniyu@amazon.com> (raw)

When we pass a file descriptor to an AF_UNIX socket via SCM_RIGTHS,
the underlying struct file of the inflight fd gets its refcount bumped.
If the fd is of an AF_UNIX socket, we need to track it in case it forms
cyclic references.

Let's say we send a fd of AF_UNIX socket A to B and vice versa and
close() both sockets.

When created, each socket's struct file initially has one reference.
After the fd exchange, both refcounts are bumped up to 2.  Then, close()
decreases both to 1.  From this point on, no one can touch the file/socket.

However, the struct file has one refcount and thus never calls the
release() function of the AF_UNIX socket.

That's why we need to track all inflight AF_UNIX sockets and run garbage
collection.

This series replaces the current GC implementation that locks each inflight
socket's receive queue and requires trickiness in other places.

The new GC does not lock each socket's queue to minimise its effect and
tries to be lightweight if there is no cyclic reference or no update in
the shape of the inflight fd graph.

The new implementation is based on Tarjan's Strongly Connected Components
algorithm, and we will consider each inflight AF_UNIX socket as a vertex
and its file descriptor as an edge in a directed graph.

For the details, please see each patch.

  patch 1  -  3 : Add struct to express inflight socket graphs
  patch       4 : Optimse inflight fd counting
  patch       5 : Group SCC possibly forming a cycle
  patch 6  -  7 : Support embryo socket
  patch 8  - 10 : Make GC lightweight
  patch 11 - 12 : Detect dead cycle references
  patch      13 : Replace GC algorithm
  patch      14 : selftest

After this series is applied, we can remove the two ugly tricks for race,
scm_fp_dup() in unix_attach_fds() and spin_lock dance in unix_peek_fds()
as done in patch 14/15 of v1.


Changes:
  v3:
    * Patch 1
      * Allocate struct unix_vertex dynamically only for inflight socket
    * Patch 2
      * Rename unix_edge.entry to unix_edge.vertex_entry
      * Change edge->successor/predecessor to struct unix_sock
    * Patch 7
      * Moved after SCC detection patch
      * Fix up embryo successor during GC instead of overwriting edge
        in unix_add_edge()
        * To not allcoate unix_vertex to listener for embryo socket
        * Kept the name unix_update_edges() unchanged as it affect
          successor tracking during GC
    * Patch 12
      * Drop self_degree and check all edges
        * To not allcoate unix_vertex to listener for embryo socket

  v2: https://lore.kernel.org/netdev/20240216210556.65913-1-kuniyu@amazon.com/
    * Drop 2 patches as follow-up that removes trickiness in
      unix_attach_fds() and unix_peek_fds().

    * Patch 2
      * Fix build error when CONFIG_UNIX=n
    * Patch 3
      * Remove unnecessary INIT_LIST_HEAD()
    * Patch 7
      * Fix build warning for using goto label at the end of the loop
    * Patch 13
      * Call kfree_skb() for oob skb
    * Patch 14
      * Add test case for MSG_OOB

  v1: https://lore.kernel.org/netdev/20240203030058.60750-1-kuniyu@amazon.com/


Kuniyuki Iwashima (14):
  af_unix: Allocate struct unix_vertex for each inflight AF_UNIX fd.
  af_unix: Allocate struct unix_edge for each inflight AF_UNIX fd.
  af_unix: Link struct unix_edge when queuing skb.
  af_unix: Bulk update unix_tot_inflight/unix_inflight when queuing skb.
  af_unix: Detect Strongly Connected Components.
  af_unix: Save listener for embryo socket.
  af_unix: Fix up unix_edge.successor for embryo socket.
  af_unix: Save O(n) setup of Tarjan's algo.
  af_unix: Skip GC if no cycle exists.
  af_unix: Avoid Tarjan's algorithm if unnecessary.
  af_unix: Assign a unique index to SCC.
  af_unix: Detect dead SCC.
  af_unix: Replace garbage collection algorithm.
  selftest: af_unix: Test GC for SCM_RIGHTS.

 include/net/af_unix.h                         |  31 +-
 include/net/scm.h                             |   9 +
 net/core/scm.c                                |  11 +
 net/unix/af_unix.c                            |  27 +-
 net/unix/garbage.c                            | 526 +++++++++++-------
 tools/testing/selftests/net/.gitignore        |   1 +
 tools/testing/selftests/net/af_unix/Makefile  |   2 +-
 .../selftests/net/af_unix/scm_rights.c        | 286 ++++++++++
 8 files changed, 685 insertions(+), 208 deletions(-)
 create mode 100644 tools/testing/selftests/net/af_unix/scm_rights.c

-- 
2.30.2


             reply	other threads:[~2024-02-23 21:40 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-23 21:39 Kuniyuki Iwashima [this message]
2024-02-23 21:39 ` [PATCH v3 net-next 01/14] af_unix: Allocate struct unix_vertex for each inflight AF_UNIX fd Kuniyuki Iwashima
2024-02-23 21:39 ` [PATCH v3 net-next 02/14] af_unix: Allocate struct unix_edge " Kuniyuki Iwashima
2024-02-23 21:39 ` [PATCH v3 net-next 03/14] af_unix: Link struct unix_edge when queuing skb Kuniyuki Iwashima
2024-02-23 21:39 ` [PATCH v3 net-next 04/14] af_unix: Bulk update unix_tot_inflight/unix_inflight " Kuniyuki Iwashima
2024-02-27 10:47   ` Paolo Abeni
2024-02-28  2:34     ` Kuniyuki Iwashima
2024-02-28  7:46       ` Paolo Abeni
2024-02-23 21:39 ` [PATCH v3 net-next 05/14] af_unix: Detect Strongly Connected Components Kuniyuki Iwashima
2024-02-25  0:34   ` Jakub Kicinski
2024-02-26 19:07     ` Kuniyuki Iwashima
2024-02-27 11:02   ` Paolo Abeni
2024-02-28  2:49     ` Kuniyuki Iwashima
2024-02-23 21:39 ` [PATCH v3 net-next 06/14] af_unix: Save listener for embryo socket Kuniyuki Iwashima
2024-02-23 21:39 ` [PATCH v3 net-next 07/14] af_unix: Fix up unix_edge.successor " Kuniyuki Iwashima
2024-02-23 21:39 ` [PATCH v3 net-next 08/14] af_unix: Save O(n) setup of Tarjan's algo Kuniyuki Iwashima
2024-02-23 21:39 ` [PATCH v3 net-next 09/14] af_unix: Skip GC if no cycle exists Kuniyuki Iwashima
2024-02-23 21:39 ` [PATCH v3 net-next 10/14] af_unix: Avoid Tarjan's algorithm if unnecessary Kuniyuki Iwashima
2024-02-23 21:40 ` [PATCH v3 net-next 11/14] af_unix: Assign a unique index to SCC Kuniyuki Iwashima
2024-02-27 11:19   ` Paolo Abeni
2024-02-28  3:05     ` Kuniyuki Iwashima
2024-02-28  7:49       ` Paolo Abeni
2024-02-28 16:25         ` Kuniyuki Iwashima
2024-02-28 17:51           ` Paolo Abeni
2024-02-23 21:40 ` [PATCH v3 net-next 12/14] af_unix: Detect dead SCC Kuniyuki Iwashima
2024-02-27 11:25   ` Paolo Abeni
2024-02-28  3:14     ` Kuniyuki Iwashima
2024-02-23 21:40 ` [PATCH v3 net-next 13/14] af_unix: Replace garbage collection algorithm Kuniyuki Iwashima
2024-02-27 11:36   ` Paolo Abeni
2024-02-28  3:32     ` Kuniyuki Iwashima
2024-02-28  8:08       ` Paolo Abeni
2024-02-28 16:29         ` Kuniyuki Iwashima
2024-02-23 21:40 ` [PATCH v3 net-next 14/14] selftest: af_unix: Test GC for SCM_RIGHTS 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=20240223214003.17369-1-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).