Re: [PATCH v2 5/6] KEYS: Replace assoc_array with rhashtable

[Jarkko Sakkinen <jarkko@kernel.org>]

On Fri, Oct 23, 2026 at 03:09:25PM +0300, Jarkko Sakkinen wrote:
> @@ -516,8 +507,7 @@ static long keyring_read(const struct key *keyring,
>  		ctx.buffer = (key_serial_t *)buffer;
>  		ctx.buflen = buflen;
>  		ctx.count = 0;
> -		ret = assoc_array_iterate(&keyring->keys,
> -					  keyring_read_iterator, &ctx);
> +		ret = keyring_iterate(keyring, keyring_read_iterator, &ctx);
>  		if (ret < 0) {
>  			kleave(" = %ld [iterate]", ret);
>  			return ret;
> @@ -525,7 +515,7 @@ static long keyring_read(const struct key *keyring,
>  	}
>  
>  	/* Return the size of the buffer needed */
> -	ret = keyring->keys.nr_leaves_on_tree * sizeof(key_serial_t);
> +	ret = keyring_nr_links(keyring) * sizeof(key_serial_t);
>  	if (ret <= buflen)
>  		kleave("= %ld [ok]", ret);
>  	else

I changed this a bit because I wanted to bake cond_resched() between the
iterations. I'd be interested getting feedback about the change below.

I.e. in my working branch I have this laid out as follows:

-struct keyring_read_iterator_context {
-	size_t			buflen;
-	size_t			count;
-	key_serial_t		*buffer;
-};
-
-static int keyring_read_iterator(const void *object, void *data)
-{
-	struct keyring_read_iterator_context *ctx = data;
-	const struct key *key = keyring_ptr_to_key(object);
-
-	kenter("{%s,%d},,{%zu/%zu}",
-	       key->type->name, key->serial, ctx->count, ctx->buflen);
-
-	if (ctx->count >= ctx->buflen)
-		return 1;
-
-	*ctx->buffer++ = key->serial;
-	ctx->count += sizeof(key->serial);
-	return 0;
-}
-
 /*
  * Read a list of key IDs from the keyring's contents in binary form
  *
@@ -503,7 +473,10 @@ static int keyring_read_iterator(const void *object, void *data)
 static long keyring_read(const struct key *keyring,
 			 char *buffer, size_t buflen)
 {
-	struct keyring_read_iterator_context ctx;
+	struct keyring_index *links = keyring_index_locked(keyring);
+	struct keyring_link *link;
+	key_serial_t *serials = (key_serial_t *)buffer;
+	size_t count = 0;
 	long ret;

 	kenter("{%d},,%zu", key_serial(keyring), buflen);
@@ -512,20 +485,18 @@ static long keyring_read(const struct key *keyring,
 		return -EINVAL;

 	/* Copy as many key IDs as fit into the buffer */
-	if (buffer && buflen) {
-		ctx.buffer = (key_serial_t *)buffer;
-		ctx.buflen = buflen;
-		ctx.count = 0;
-		ret = assoc_array_iterate(&keyring->keys,
-					  keyring_read_iterator, &ctx);
-		if (ret < 0) {
-			kleave(" = %ld [iterate]", ret);
-			return ret;
+	if (buffer && buflen && links) {
+		list_for_each_entry(link, &links->list, list) {
+			if (count == buflen / sizeof(*serials))
+				break;
+
+			serials[count++] = keyring_link_key(link)->serial;
+			cond_resched();
 		}
 	}

 	/* Return the size of the buffer needed */
-	ret = keyring->keys.nr_leaves_on_tree * sizeof(key_serial_t);
+	ret = links ? links->nr * sizeof(key_serial_t) : 0;
 	if (ret <= buflen)
 		kleave("= %ld [ok]", ret);
 	else

What do you think about this? I'm worried about blind spot mainly
because this feels too obvious.

Br, Jarkko