The minsize parameter of create_hashtable() doesn't have any real use
case for Xenstore, so drop it.
For better talloc_report_full() diagnostic output add a name parameter
to create_hashtable().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
tools/xenstore/hashtable.c | 20 ++++++--------------
tools/xenstore/hashtable.h | 4 ++--
tools/xenstore/xenstored_core.c | 2 +-
tools/xenstore/xenstored_domain.c | 4 ++--
4 files changed, 11 insertions(+), 19 deletions(-)
diff --git a/tools/xenstore/hashtable.c b/tools/xenstore/hashtable.c
index c1b11743bb..ab1e687d0b 100644
--- a/tools/xenstore/hashtable.c
+++ b/tools/xenstore/hashtable.c
@@ -55,36 +55,28 @@ static unsigned int loadlimit(unsigned int pindex)
return ((uint64_t)primes[pindex] * MAX_LOAD_PERCENT) / 100;
}
-struct hashtable *create_hashtable(const void *ctx, unsigned int minsize,
+struct hashtable *create_hashtable(const void *ctx, const char *name,
unsigned int (*hashf) (const void *),
int (*eqf) (const void *, const void *),
unsigned int flags)
{
struct hashtable *h;
- unsigned int pindex, size = primes[0];
-
- /* Check requested hashtable isn't too large */
- if (minsize > (1u << 30)) return NULL;
-
- /* Enforce size as prime */
- for (pindex=0; pindex < PRIME_TABLE_LEN; pindex++) {
- if (primes[pindex] > minsize) { size = primes[pindex]; break; }
- }
h = talloc_zero(ctx, struct hashtable);
if (NULL == h)
goto err0;
- h->table = talloc_zero_array(h, struct entry *, size);
+ talloc_set_name_const(h, name);
+ h->table = talloc_zero_array(h, struct entry *, primes[0]);
if (NULL == h->table)
goto err1;
- h->tablelength = size;
+ h->tablelength = primes[0];
h->flags = flags;
- h->primeindex = pindex;
+ h->primeindex = 0;
h->entrycount = 0;
h->hashfn = hashf;
h->eqfn = eqf;
- h->loadlimit = loadlimit(pindex);
+ h->loadlimit = loadlimit(0);
return h;
err1:
diff --git a/tools/xenstore/hashtable.h b/tools/xenstore/hashtable.h
index 04310783b6..0e1a6f61c2 100644
--- a/tools/xenstore/hashtable.h
+++ b/tools/xenstore/hashtable.h
@@ -10,7 +10,7 @@ struct hashtable;
* @name create_hashtable
* @param ctx talloc context to use for allocations
- * @param minsize minimum initial size of hashtable
+ * @param name talloc name of the hashtable
* @param hashfunction function for hashing keys
* @param key_eq_fn function for determining key equality
* @param flags flags HASHTABLE_*
@@ -23,7 +23,7 @@ struct hashtable;
#define HASHTABLE_FREE_KEY (1U << 1)
struct hashtable *
-create_hashtable(const void *ctx, unsigned int minsize,
+create_hashtable(const void *ctx, const char *name,
unsigned int (*hashfunction) (const void *),
int (*key_eq_fn) (const void *, const void *),
unsigned int flags
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 7214b3df03..6ce7be3161 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2511,7 +2511,7 @@ void check_store(void)
struct check_store_data data;
/* Don't free values (they are all void *1) */
- data.reachable = create_hashtable(NULL, 16, hash_from_key_fn,
+ data.reachable = create_hashtable(NULL, "checkstore", hash_from_key_fn,
keys_equal_fn, HASHTABLE_FREE_KEY);
if (!data.reachable) {
log("check_store: ENOMEM");
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index df64c87efc..6d40aefc63 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -1017,7 +1017,7 @@ void domain_init(int evtfd)
int rc;
/* Start with a random rather low domain count for the hashtable. */
- domhash = create_hashtable(NULL, 8, domhash_fn, domeq_fn, 0);
+ domhash = create_hashtable(NULL, "domains", domhash_fn, domeq_fn, 0);
if (!domhash)
barf_perror("Failed to allocate domain hashtable");
@@ -1804,7 +1804,7 @@ struct hashtable *domain_check_acc_init(void)
{
struct hashtable *domains;
- domains = create_hashtable(NULL, 8, domhash_fn, domeq_fn,
+ domains = create_hashtable(NULL, "domain_check", domhash_fn, domeq_fn,
HASHTABLE_FREE_VALUE);
if (!domains)
return NULL;
--
2.35.3
Hi Juergen,
On 30/03/2023 09:50, Juergen Gross wrote:
> The minsize parameter of create_hashtable() doesn't have any real use
> case for Xenstore, so drop it.
>
> For better talloc_report_full() diagnostic output add a name parameter
> to create_hashtable().
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> tools/xenstore/hashtable.c | 20 ++++++--------------
> tools/xenstore/hashtable.h | 4 ++--
> tools/xenstore/xenstored_core.c | 2 +-
> tools/xenstore/xenstored_domain.c | 4 ++--
> 4 files changed, 11 insertions(+), 19 deletions(-)
>
> diff --git a/tools/xenstore/hashtable.c b/tools/xenstore/hashtable.c
> index c1b11743bb..ab1e687d0b 100644
> --- a/tools/xenstore/hashtable.c
> +++ b/tools/xenstore/hashtable.c
> @@ -55,36 +55,28 @@ static unsigned int loadlimit(unsigned int pindex)
> return ((uint64_t)primes[pindex] * MAX_LOAD_PERCENT) / 100;
> }
>
> -struct hashtable *create_hashtable(const void *ctx, unsigned int minsize,
> +struct hashtable *create_hashtable(const void *ctx, const char *name,
> unsigned int (*hashf) (const void *),
> int (*eqf) (const void *, const void *),
> unsigned int flags)
> {
> struct hashtable *h;
> - unsigned int pindex, size = primes[0];
> -
> - /* Check requested hashtable isn't too large */
> - if (minsize > (1u << 30)) return NULL;
> -
> - /* Enforce size as prime */
> - for (pindex=0; pindex < PRIME_TABLE_LEN; pindex++) {
> - if (primes[pindex] > minsize) { size = primes[pindex]; break; }
> - }
>
> h = talloc_zero(ctx, struct hashtable);
> if (NULL == h)
> goto err0;
> - h->table = talloc_zero_array(h, struct entry *, size);
> + talloc_set_name_const(h, name);
> + h->table = talloc_zero_array(h, struct entry *, primes[0]);
> if (NULL == h->table)
> goto err1;
>
> - h->tablelength = size;
> + h->tablelength = primes[0];
I find the connection between this line, ...
> h->flags = flags;
> - h->primeindex = pindex;
> + h->primeindex = 0;
... this one and ...
> h->entrycount = 0;
> h->hashfn = hashf;
> h->eqfn = eqf;
> - h->loadlimit = loadlimit(pindex);
> + h->loadlimit = loadlimit(0);
... now more difficult to find. How about setting h->primeindex first
and then using it in place of 0?
> return h;
>
> err1:
> diff --git a/tools/xenstore/hashtable.h b/tools/xenstore/hashtable.h
> index 04310783b6..0e1a6f61c2 100644
> --- a/tools/xenstore/hashtable.h
> +++ b/tools/xenstore/hashtable.h
> @@ -10,7 +10,7 @@ struct hashtable;
>
> * @name create_hashtable
> * @param ctx talloc context to use for allocations
> - * @param minsize minimum initial size of hashtable
> + * @param name talloc name of the hashtable
> * @param hashfunction function for hashing keys
> * @param key_eq_fn function for determining key equality
> * @param flags flags HASHTABLE_*
> @@ -23,7 +23,7 @@ struct hashtable;
> #define HASHTABLE_FREE_KEY (1U << 1)
>
> struct hashtable *
> -create_hashtable(const void *ctx, unsigned int minsize,
> +create_hashtable(const void *ctx, const char *name,
> unsigned int (*hashfunction) (const void *),
> int (*key_eq_fn) (const void *, const void *),
> unsigned int flags
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 7214b3df03..6ce7be3161 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -2511,7 +2511,7 @@ void check_store(void)
> struct check_store_data data;
>
> /* Don't free values (they are all void *1) */
> - data.reachable = create_hashtable(NULL, 16, hash_from_key_fn,
> + data.reachable = create_hashtable(NULL, "checkstore", hash_from_key_fn,
> keys_equal_fn, HASHTABLE_FREE_KEY);
> if (!data.reachable) {
> log("check_store: ENOMEM");
> diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
> index df64c87efc..6d40aefc63 100644
> --- a/tools/xenstore/xenstored_domain.c
> +++ b/tools/xenstore/xenstored_domain.c
> @@ -1017,7 +1017,7 @@ void domain_init(int evtfd)
> int rc;
>
> /* Start with a random rather low domain count for the hashtable. */
> - domhash = create_hashtable(NULL, 8, domhash_fn, domeq_fn, 0);
> + domhash = create_hashtable(NULL, "domains", domhash_fn, domeq_fn, 0);
> if (!domhash)
> barf_perror("Failed to allocate domain hashtable");
>
> @@ -1804,7 +1804,7 @@ struct hashtable *domain_check_acc_init(void)
> {
> struct hashtable *domains;
>
> - domains = create_hashtable(NULL, 8, domhash_fn, domeq_fn,
> + domains = create_hashtable(NULL, "domain_check", domhash_fn, domeq_fn,
> HASHTABLE_FREE_VALUE);
> if (!domains)
> return NULL;
Cheers,
--
Julien Grall
On 03.05.23 14:59, Julien Grall wrote:
> Hi Juergen,
>
> On 30/03/2023 09:50, Juergen Gross wrote:
>> The minsize parameter of create_hashtable() doesn't have any real use
>> case for Xenstore, so drop it.
>>
>> For better talloc_report_full() diagnostic output add a name parameter
>> to create_hashtable().
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> tools/xenstore/hashtable.c | 20 ++++++--------------
>> tools/xenstore/hashtable.h | 4 ++--
>> tools/xenstore/xenstored_core.c | 2 +-
>> tools/xenstore/xenstored_domain.c | 4 ++--
>> 4 files changed, 11 insertions(+), 19 deletions(-)
>>
>> diff --git a/tools/xenstore/hashtable.c b/tools/xenstore/hashtable.c
>> index c1b11743bb..ab1e687d0b 100644
>> --- a/tools/xenstore/hashtable.c
>> +++ b/tools/xenstore/hashtable.c
>> @@ -55,36 +55,28 @@ static unsigned int loadlimit(unsigned int pindex)
>> return ((uint64_t)primes[pindex] * MAX_LOAD_PERCENT) / 100;
>> }
>> -struct hashtable *create_hashtable(const void *ctx, unsigned int minsize,
>> +struct hashtable *create_hashtable(const void *ctx, const char *name,
>> unsigned int (*hashf) (const void *),
>> int (*eqf) (const void *, const void *),
>> unsigned int flags)
>> {
>> struct hashtable *h;
>> - unsigned int pindex, size = primes[0];
>> -
>> - /* Check requested hashtable isn't too large */
>> - if (minsize > (1u << 30)) return NULL;
>> -
>> - /* Enforce size as prime */
>> - for (pindex=0; pindex < PRIME_TABLE_LEN; pindex++) {
>> - if (primes[pindex] > minsize) { size = primes[pindex]; break; }
>> - }
>> h = talloc_zero(ctx, struct hashtable);
>> if (NULL == h)
>> goto err0;
>> - h->table = talloc_zero_array(h, struct entry *, size);
>> + talloc_set_name_const(h, name);
>> + h->table = talloc_zero_array(h, struct entry *, primes[0]);
>> if (NULL == h->table)
>> goto err1;
>> - h->tablelength = size;
>> + h->tablelength = primes[0];
>
> I find the connection between this line, ...
>
>> h->flags = flags;
>> - h->primeindex = pindex;
>> + h->primeindex = 0;
>
> ... this one and ...
>
>> h->entrycount = 0;
>> h->hashfn = hashf;
>> h->eqfn = eqf;
>> - h->loadlimit = loadlimit(pindex);
>> + h->loadlimit = loadlimit(0);
>
> ... now more difficult to find. How about setting h->primeindex first and then
> using it in place of 0?
Fine with me.
Juergen
© 2016 - 2026 Red Hat, Inc.