[RFC PATCH net-next 0/4] rhashtable: length helper for rhashtable and rhltable

[RFC PATCH net-next 0/4] rhashtable: length helper for rhashtable and rhltable

[Pedro Tammela]

Whenever someone wants to retrieve the total number of elements in a
rhashtable/rhltable it needs to open code the access to 'nelems'.
Therefore provide a helper for such operation and convert two accesses as
an example.

Pedro Tammela (4):
  rhashtable: add length helper for rhashtable and rhltable
  rhashtable: use new length helpers
  net/ipv4: use rhashtable length helper
  net/ipv6: use rhashtable length helper

 include/linux/rhashtable.h | 16 ++++++++++++++++
 lib/rhashtable.c           |  2 +-
 lib/test_rhashtable.c      |  4 ++--
 net/ipv4/proc.c            |  3 ++-
 net/ipv6/proc.c            |  3 ++-
 5 files changed, 23 insertions(+), 5 deletions(-)

-- 
2.39.2

[RFC PATCH net-next 1/4] rhashtable: add length helper for rhashtable and rhltable

[Pedro Tammela]

Instead of having users open code the rhashtable length like:
   atomic_read(&ht->nelems)

Provide a helper for both flavours of rhashtables.

Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 include/linux/rhashtable.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 5b5357c0bd8c..aac803491916 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -1283,4 +1283,20 @@ static inline void rhltable_destroy(struct rhltable *hlt)
 	return rhltable_free_and_destroy(hlt, NULL, NULL);
 }
 
+/**
+ * rhashtable_len - hash table length
+ * @ht: the hash table
+ *
+ * Returns the number of elements in the hash table
+ */
+static inline int rhashtable_len(struct rhashtable *ht)
+{
+	return atomic_read(&ht->nelems);
+}
+
+static inline int rhltable_len(struct rhltable *hlt)
+{
+	return rhashtable_len(&hlt->ht);
+}
+
 #endif /* _LINUX_RHASHTABLE_H */
-- 
2.39.2

Re: [RFC PATCH net-next 1/4] rhashtable: add length helper for rhashtable and rhltable

[Eric Dumazet]

On Fri, Jun 9, 2023 at 5:13 PM Pedro Tammela <pctammela@mojatatu.com> wrote:
>
> Instead of having users open code the rhashtable length like:
>    atomic_read(&ht->nelems)
>
> Provide a helper for both flavours of rhashtables.
>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
> ---
>  include/linux/rhashtable.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
> index 5b5357c0bd8c..aac803491916 100644
> --- a/include/linux/rhashtable.h
> +++ b/include/linux/rhashtable.h
> @@ -1283,4 +1283,20 @@ static inline void rhltable_destroy(struct rhltable *hlt)
>         return rhltable_free_and_destroy(hlt, NULL, NULL);
>  }
>
> +/**
> + * rhashtable_len - hash table length
> + * @ht: the hash table
> + *
> + * Returns the number of elements in the hash table
> + */
> +static inline int rhashtable_len(struct rhashtable *ht)
> +{
> +       return atomic_read(&ht->nelems);
> +}
> +
> +static inline int rhltable_len(struct rhltable *hlt)
> +{
> +       return rhashtable_len(&hlt->ht);
> +}
> +

If we want/need these, please add 'const' qualifiers to both

static inline int rhltable_len(const struct rhltable *hlt)
...

[RFC PATCH net-next 2/4] rhashtable: use new length helpers

[Pedro Tammela]

Use the new length helpers instead of open coding the length read

Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 lib/rhashtable.c      | 2 +-
 lib/test_rhashtable.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 6ae2ba8e06a2..1f8ca27af853 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -390,7 +390,7 @@ static int rhashtable_rehash_alloc(struct rhashtable *ht,
 static int rhashtable_shrink(struct rhashtable *ht)
 {
 	struct bucket_table *old_tbl = rht_dereference(ht->tbl, ht);
-	unsigned int nelems = atomic_read(&ht->nelems);
+	unsigned int nelems = rhashtable_len(ht);
 	unsigned int size = 0;
 
 	if (nelems)
diff --git a/lib/test_rhashtable.c b/lib/test_rhashtable.c
index c20f6cb4bf55..5853b83d9ad1 100644
--- a/lib/test_rhashtable.c
+++ b/lib/test_rhashtable.c
@@ -199,9 +199,9 @@ static void test_bucket_stats(struct rhashtable *ht, unsigned int entries)
 	rhashtable_walk_exit(&hti);
 
 	pr_info("  Traversal complete: counted=%u, nelems=%u, entries=%d, table-jumps=%u\n",
-		total, atomic_read(&ht->nelems), entries, chain_len);
+		total, rhashtable_len(ht), entries, chain_len);
 
-	if (total != atomic_read(&ht->nelems) || total != entries)
+	if (total != rhashtable_len(ht) || total != entries)
 		pr_warn("Test failed: Total count mismatch ^^^");
 }
 
-- 
2.39.2

[RFC PATCH net-next 3/4] net/ipv4: use rhashtable length helper

[Pedro Tammela]

Avoid open coding the rhashtable length read

Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 net/ipv4/proc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index eaf1d3113b62..cab1edc3c416 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -42,6 +42,7 @@
 #include <linux/export.h>
 #include <net/sock.h>
 #include <net/raw.h>
+#include <linux/rhashtable.h>
 
 #define TCPUDP_MIB_MAX max_t(u32, UDP_MIB_MAX, TCP_MIB_MAX)
 
@@ -69,7 +70,7 @@ static int sockstat_seq_show(struct seq_file *seq, void *v)
 	seq_printf(seq, "RAW: inuse %d\n",
 		   sock_prot_inuse_get(net, &raw_prot));
 	seq_printf(seq,  "FRAG: inuse %u memory %lu\n",
-		   atomic_read(&net->ipv4.fqdir->rhashtable.nelems),
+		   rhashtable_len(&net->ipv4.fqdir->rhashtable),
 		   frag_mem_limit(net->ipv4.fqdir));
 	return 0;
 }
-- 
2.39.2

[RFC PATCH net-next 4/4] net/ipv6: use rhashtable length helper

[Pedro Tammela]

Avoid open coding the rhashtab length read

Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 net/ipv6/proc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/proc.c b/net/ipv6/proc.c
index e20b3705c2d2..402e4f6f9e25 100644
--- a/net/ipv6/proc.c
+++ b/net/ipv6/proc.c
@@ -25,6 +25,7 @@
 #include <net/udp.h>
 #include <net/transp_v6.h>
 #include <net/ipv6.h>
+#include <linux/rhashtable.h>
 
 #define MAX4(a, b, c, d) \
 	max_t(u32, max_t(u32, a, b), max_t(u32, c, d))
@@ -44,7 +45,7 @@ static int sockstat6_seq_show(struct seq_file *seq, void *v)
 	seq_printf(seq, "RAW6: inuse %d\n",
 		       sock_prot_inuse_get(net, &rawv6_prot));
 	seq_printf(seq, "FRAG6: inuse %u memory %lu\n",
-		   atomic_read(&net->ipv6.fqdir->rhashtable.nelems),
+		   rhashtable_len(&net->ipv6.fqdir->rhashtable),
 		   frag_mem_limit(net->ipv6.fqdir));
 	return 0;
 }
-- 
2.39.2

Re: [RFC PATCH net-next 0/4] rhashtable: length helper for rhashtable and rhltable

[Jakub Kicinski]

On Fri,  9 Jun 2023 12:13:28 -0300 Pedro Tammela wrote:
> Whenever someone wants to retrieve the total number of elements in a
> rhashtable/rhltable it needs to open code the access to 'nelems'.
> Therefore provide a helper for such operation and convert two accesses as
> an example.

IMHO read of nelems is much more readable than len(). I mean the name
of the helper is not great. IDK what length of a hashtable is. Feels
like a Python-ism.

Re: [RFC PATCH net-next 0/4] rhashtable: length helper for rhashtable and rhltable

[Pedro Tammela]

On 09/06/2023 14:36, Jakub Kicinski wrote:
> On Fri,  9 Jun 2023 12:13:28 -0300 Pedro Tammela wrote:
>> Whenever someone wants to retrieve the total number of elements in a
>> rhashtable/rhltable it needs to open code the access to 'nelems'.
>> Therefore provide a helper for such operation and convert two accesses as
>> an example.
> 
> IMHO read of nelems is much more readable than len().

For the case of rhltable it requires:
	atomic_read(rhltable->rt.nelems);

Which feels like it defeats the purpose of the dedicated rhltable type 
in the first place.
But I must admit that there are no use cases in-tree AFAIK.

Another point is that having this sort of helper also conveys that it's 
OK for a consumer to query this value at any time.

> I mean the name of the helper is not great. > IDK what length of a hashtable is. Feels like a Python-ism.

Well 'length' has been the term used in a few languages (Python, Rust, 
Go, etc...) so it felt the most fitting. If you have any suggestions in 
mind, do tell; Some that crossed my mind were:
- count
- size
- elems
- num

Re: [RFC PATCH net-next 0/4] rhashtable: length helper for rhashtable and rhltable

[Jakub Kicinski]

On Fri, 9 Jun 2023 15:13:40 -0300 Pedro Tammela wrote:
> > I mean the name of the helper is not great. > IDK what length of a hashtable is. Feels like a Python-ism.  
> 
> Well 'length' has been the term used in a few languages (Python, Rust, 
> Go, etc...) so it felt the most fitting. If you have any suggestions in 
> mind, do tell; Some that crossed my mind were:
> - count
> - size
> - elems
> - num

count and elems sound best to me