Hello,
Milos Nikic, le dim. 29 mars 2026 21:48:10 -0700, a ecrit:
> This patch replaces that potentially giant allocation with an O(1)
> intrusive doubly-linked list, completely eliminating the malloc
> overhead and improving sync latency under heavy inode loads.
That should be helpful indeed.
> @@ -60,6 +59,28 @@ static struct hurd_ihash nodecache =
> HURD_IHASH_INITIALIZER_GKI (offsetof (struct node, slot), NULL, NULL,
> hash, compare);
> static pthread_rwlock_t nodecache_lock = PTHREAD_RWLOCK_INITIALIZER;
> +static pthread_mutex_t active_nodes_lock = PTHREAD_MUTEX_INITIALIZER;
Do we really want a separate lock?
AIUI the node cache and your introduced list are supposed to contain the
same set. Better use the same lock to make sure of this. That will make
the picture much simpler for maintenance: we have a set of nodes, with
two structures to traverse them: a hashtable and a list.
> +static struct node *active_nodes_head = NULL;
> +static struct node *active_nodes_tail = NULL;
> +
> +/* Unlinks a node from the active nodes doubly-linked list.
> + The caller MUST hold active_nodes_lock before calling this. */
> +static void
> +unlink_active_node (struct node *np)
> +{
> + if (np->cache_prev)
> + np->cache_prev->cache_next = np->cache_next;
> + if (np->cache_next)
> + np->cache_next->cache_prev = np->cache_prev;
> +
> + if (active_nodes_head == np)
> + active_nodes_head = np->cache_next;
> + if (active_nodes_tail == np)
> + active_nodes_tail = np->cache_prev;
> +
> + np->cache_prev = NULL;
> + np->cache_next = NULL;
> +}
>
> /* Fetch inode INUM, set *NPP to the node structure;
> gain one user reference and lock the node. */
> @@ -109,6 +130,15 @@ diskfs_cached_lookup_context (ino_t inum, struct node
> **npp,
> assert_perror_backtrace (err);
> diskfs_nref_light (np);
> pthread_rwlock_unlock (&nodecache_lock);
> + pthread_mutex_lock (&active_nodes_lock);
> + np->cache_next = active_nodes_head;
> + np->cache_prev = NULL;
> + if (active_nodes_head)
> + active_nodes_head->cache_prev = np;
> + else
> + active_nodes_tail = np;
> + active_nodes_head = np;
Better make this a link_active_node function.
Samuel
