Re: [PATCH v9 2/3] KEYS: Replace assoc_array with rhashtable

[Jarkko Sakkinen <jarkko@kernel.org>]

On Tue, Nov 10, 2026 at 07:33:22AM +0200, Jarkko Sakkinen wrote:
>  /*
>   * Preallocate memory so that a key can be linked into to a keyring.
>   */
>  int __key_link_begin(struct key *keyring,
>  		     const struct keyring_index_key *index_key,
> -		     struct assoc_array_edit **_edit)
> +		     struct keyring_edit *edit)
>  {
> -	struct assoc_array_edit *edit;
> +	struct keyring_link *link = NULL;
> +	struct keyring_link *replace;
> +	struct keyring_index *links;
>  	int ret;
>  
>  	kenter("%d,%s,%s,",
>  	       keyring->serial, index_key->type->name, index_key->description);
>  
> -	BUG_ON(index_key->desc_len == 0);
> -	BUG_ON(*_edit != NULL);
> +	if (WARN_ON(index_key->desc_len == 0) || WARN_ON(edit->new)) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
>  
> -	*_edit = NULL;
> +	edit->new = NULL;
> +	edit->old = NULL;
>  
> -	ret = -EKEYREVOKED;
> -	if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
> -		goto error;
> +	if (test_bit(KEY_FLAG_REVOKED, &keyring->flags)) {
> +		ret = -EKEYREVOKED;
> +		goto out;
> +	}
>  
> -	/* Create an edit script that will insert/replace the key in the
> -	 * keyring tree.
> -	 */
> -	edit = assoc_array_insert(&keyring->keys,
> -				  &keyring_assoc_array_ops,
> -				  index_key,
> -				  NULL);
> -	if (IS_ERR(edit)) {
> -		ret = PTR_ERR(edit);
> -		goto error;
> +	links = keyring_get_index(keyring);
> +	if (IS_ERR(links)) {
> +		ret = PTR_ERR(links);
> +		goto out;
>  	}
>  
> -	/* If we're not replacing a link in-place then we're going to need some
> +	link = kzalloc_obj(struct keyring_link);
> +	if (!link) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	link->hash = index_key->hash;
> +
> +	replace = keyring_find_link(links, index_key);
> +
> +	/*
> +	 * If we're not replacing a link in-place then we're going to need some
>  	 * extra quota.
>  	 */
> -	if (!edit->dead_leaf) {
> +	if (!replace) {
>  		ret = key_payload_reserve(keyring,
>  					  keyring->datalen + KEYQUOTA_LINK_BYTES);
>  		if (ret < 0)
> -			goto error_cancel;
> +			goto out;
> +
> +		ret = rhashtable_insert_fast(&links->ht, &link->node,
> +					     keyring_link_params);
> +		if (ret < 0) {
> +			key_payload_reserve(keyring,
> +					    keyring->datalen - KEYQUOTA_LINK_BYTES);
> +			goto out;
> +		}
>  	}
>  
> -	*_edit = edit;
> -	kleave(" = 0");
> -	return 0;
> +	edit->new = link;
> +	edit->old = replace;
> +	ret = 0;
>  
> -error_cancel:
> -	assoc_array_cancel_edit(edit);
> -error:
> +out:
> +	if (ret && link)
> +		kfree(link);
>  	kleave(" = %d", ret);
>  	return ret;
>  }

No additional Sashiko complains.

Despite a few unfortunate extra rounds I'm happy how this shaped in the
end. It has a sane flow now I think.

And it definitely made sense to drop cleanup.h shenanigans from
__key_link_begin().

Br, Jarkko