* [PATCH 1/3] rhashtable: Remove GFP flag from rhashtable_walk_init
2016-08-18 8:48 [PATCH 0/3] rhashtable: Get rid of raw table walkers part 1 Herbert Xu
@ 2016-08-18 8:50 ` Herbert Xu
2016-08-19 16:25 ` Thomas Graf
2016-08-18 8:50 ` [PATCH 2/3] MAINTAINERS: Add extra rhashtable maintainer Herbert Xu
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Herbert Xu @ 2016-08-18 8:50 UTC (permalink / raw)
To: David S. Miller, Thomas Graf, netdev
The commit 8f6fd83c6c5ec66a4a70c728535ddcdfef4f3697 ("rhashtable:
accept GFP flags in rhashtable_walk_init") added a GFP flag argument
to rhashtable_walk_init because some users wish to use the walker
in an unsleepable context.
In fact we don't need to allocate memory in rhashtable_walk_init
at all. The walker is always paired with an iterator so we could
just stash ourselves there.
This patch does that by introducing a new enter function to replace
the existing init function. This way we don't have to churn all
the existing users again.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
include/linux/rhashtable.h | 14 ++++++++++---
lib/rhashtable.c | 46 +++++++++++++++++----------------------------
2 files changed, 29 insertions(+), 31 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 3eef080..8b72ee7 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -173,7 +173,7 @@ struct rhashtable_walker {
struct rhashtable_iter {
struct rhashtable *ht;
struct rhash_head *p;
- struct rhashtable_walker *walker;
+ struct rhashtable_walker walker;
unsigned int slot;
unsigned int skip;
};
@@ -346,8 +346,8 @@ struct bucket_table *rhashtable_insert_slow(struct rhashtable *ht,
struct bucket_table *old_tbl);
int rhashtable_insert_rehash(struct rhashtable *ht, struct bucket_table *tbl);
-int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter,
- gfp_t gfp);
+void rhashtable_walk_enter(struct rhashtable *ht,
+ struct rhashtable_iter *iter);
void rhashtable_walk_exit(struct rhashtable_iter *iter);
int rhashtable_walk_start(struct rhashtable_iter *iter) __acquires(RCU);
void *rhashtable_walk_next(struct rhashtable_iter *iter);
@@ -906,4 +906,12 @@ static inline int rhashtable_replace_fast(
return err;
}
+/* Obsolete function, do not use in new code. */
+static inline int rhashtable_walk_init(struct rhashtable *ht,
+ struct rhashtable_iter *iter, gfp_t gfp)
+{
+ rhashtable_walk_enter(ht, iter);
+ return 0;
+}
+
#endif /* _LINUX_RHASHTABLE_H */
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 5d845ff..5b0a2b3 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -484,10 +484,9 @@ exit:
EXPORT_SYMBOL_GPL(rhashtable_insert_slow);
/**
- * rhashtable_walk_init - Initialise an iterator
+ * rhashtable_walk_enter - Initialise an iterator
* @ht: Table to walk over
* @iter: Hash table Iterator
- * @gfp: GFP flags for allocations
*
* This function prepares a hash table walk.
*
@@ -502,30 +501,22 @@ EXPORT_SYMBOL_GPL(rhashtable_insert_slow);
* This function may sleep so you must not call it from interrupt
* context or with spin locks held.
*
- * You must call rhashtable_walk_exit if this function returns
- * successfully.
+ * You must call rhashtable_walk_exit after this function returns.
*/
-int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter,
- gfp_t gfp)
+void rhashtable_walk_enter(struct rhashtable *ht, struct rhashtable_iter *iter)
{
iter->ht = ht;
iter->p = NULL;
iter->slot = 0;
iter->skip = 0;
- iter->walker = kmalloc(sizeof(*iter->walker), gfp);
- if (!iter->walker)
- return -ENOMEM;
-
spin_lock(&ht->lock);
- iter->walker->tbl =
+ iter->walker.tbl =
rcu_dereference_protected(ht->tbl, lockdep_is_held(&ht->lock));
- list_add(&iter->walker->list, &iter->walker->tbl->walkers);
+ list_add(&iter->walker.list, &iter->walker.tbl->walkers);
spin_unlock(&ht->lock);
-
- return 0;
}
-EXPORT_SYMBOL_GPL(rhashtable_walk_init);
+EXPORT_SYMBOL_GPL(rhashtable_walk_enter);
/**
* rhashtable_walk_exit - Free an iterator
@@ -536,10 +527,9 @@ EXPORT_SYMBOL_GPL(rhashtable_walk_init);
void rhashtable_walk_exit(struct rhashtable_iter *iter)
{
spin_lock(&iter->ht->lock);
- if (iter->walker->tbl)
- list_del(&iter->walker->list);
+ if (iter->walker.tbl)
+ list_del(&iter->walker.list);
spin_unlock(&iter->ht->lock);
- kfree(iter->walker);
}
EXPORT_SYMBOL_GPL(rhashtable_walk_exit);
@@ -565,12 +555,12 @@ int rhashtable_walk_start(struct rhashtable_iter *iter)
rcu_read_lock();
spin_lock(&ht->lock);
- if (iter->walker->tbl)
- list_del(&iter->walker->list);
+ if (iter->walker.tbl)
+ list_del(&iter->walker.list);
spin_unlock(&ht->lock);
- if (!iter->walker->tbl) {
- iter->walker->tbl = rht_dereference_rcu(ht->tbl, ht);
+ if (!iter->walker.tbl) {
+ iter->walker.tbl = rht_dereference_rcu(ht->tbl, ht);
return -EAGAIN;
}
@@ -592,7 +582,7 @@ EXPORT_SYMBOL_GPL(rhashtable_walk_start);
*/
void *rhashtable_walk_next(struct rhashtable_iter *iter)
{
- struct bucket_table *tbl = iter->walker->tbl;
+ struct bucket_table *tbl = iter->walker.tbl;
struct rhashtable *ht = iter->ht;
struct rhash_head *p = iter->p;
@@ -625,8 +615,8 @@ next:
/* Ensure we see any new tables. */
smp_rmb();
- iter->walker->tbl = rht_dereference_rcu(tbl->future_tbl, ht);
- if (iter->walker->tbl) {
+ iter->walker.tbl = rht_dereference_rcu(tbl->future_tbl, ht);
+ if (iter->walker.tbl) {
iter->slot = 0;
iter->skip = 0;
return ERR_PTR(-EAGAIN);
@@ -646,7 +636,7 @@ void rhashtable_walk_stop(struct rhashtable_iter *iter)
__releases(RCU)
{
struct rhashtable *ht;
- struct bucket_table *tbl = iter->walker->tbl;
+ struct bucket_table *tbl = iter->walker.tbl;
if (!tbl)
goto out;
@@ -655,9 +645,9 @@ void rhashtable_walk_stop(struct rhashtable_iter *iter)
spin_lock(&ht->lock);
if (tbl->rehash < tbl->size)
- list_add(&iter->walker->list, &tbl->walkers);
+ list_add(&iter->walker.list, &tbl->walkers);
else
- iter->walker->tbl = NULL;
+ iter->walker.tbl = NULL;
spin_unlock(&ht->lock);
iter->p = NULL;
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] rhashtable: Remove GFP flag from rhashtable_walk_init
2016-08-18 8:50 ` [PATCH 1/3] rhashtable: Remove GFP flag from rhashtable_walk_init Herbert Xu
@ 2016-08-19 16:25 ` Thomas Graf
2016-08-19 16:38 ` Herbert Xu
0 siblings, 1 reply; 13+ messages in thread
From: Thomas Graf @ 2016-08-19 16:25 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev
On 08/18/16 at 04:50pm, Herbert Xu wrote:
> +/* Obsolete function, do not use in new code. */
> +static inline int rhashtable_walk_init(struct rhashtable *ht,
> + struct rhashtable_iter *iter, gfp_t gfp)
> +{
> + rhashtable_walk_enter(ht, iter);
> + return 0;
> +}
Converting the 5 users of rhashtable_walk_init() looks very straight
forward, any reason to keep this around?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] rhashtable: Remove GFP flag from rhashtable_walk_init
2016-08-19 16:25 ` Thomas Graf
@ 2016-08-19 16:38 ` Herbert Xu
0 siblings, 0 replies; 13+ messages in thread
From: Herbert Xu @ 2016-08-19 16:38 UTC (permalink / raw)
To: Thomas Graf; +Cc: David S. Miller, netdev
On Fri, Aug 19, 2016 at 06:25:04PM +0200, Thomas Graf wrote:
> On 08/18/16 at 04:50pm, Herbert Xu wrote:
> > +/* Obsolete function, do not use in new code. */
> > +static inline int rhashtable_walk_init(struct rhashtable *ht,
> > + struct rhashtable_iter *iter, gfp_t gfp)
> > +{
> > + rhashtable_walk_enter(ht, iter);
> > + return 0;
> > +}
>
> Converting the 5 users of rhashtable_walk_init() looks very straight
> forward, any reason to keep this around?
It is easy but it's needless churn, especially since we've just
converted all of them the other way.
Feel free to do a patch on top of this to get rid of them.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] MAINTAINERS: Add extra rhashtable maintainer
2016-08-18 8:48 [PATCH 0/3] rhashtable: Get rid of raw table walkers part 1 Herbert Xu
2016-08-18 8:50 ` [PATCH 1/3] rhashtable: Remove GFP flag from rhashtable_walk_init Herbert Xu
@ 2016-08-18 8:50 ` Herbert Xu
2016-08-19 16:25 ` Thomas Graf
2016-08-18 8:50 ` [PATCH 3/3] netlink: Use rhashtable walk interface in diag dump Herbert Xu
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Herbert Xu @ 2016-08-18 8:50 UTC (permalink / raw)
To: David S. Miller, Thomas Graf, netdev
As I'm working actively on rhashtable it helps if people CCed me
when they work on in.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 5d813a3..db09498 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9641,6 +9641,7 @@ F: net/rfkill/
RHASHTABLE
M: Thomas Graf <tgraf@suug.ch>
+M: Herbert Xu <herbert@gondor.apana.org.au>
L: netdev@vger.kernel.org
S: Maintained
F: lib/rhashtable.c
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] netlink: Use rhashtable walk interface in diag dump
2016-08-18 8:48 [PATCH 0/3] rhashtable: Get rid of raw table walkers part 1 Herbert Xu
2016-08-18 8:50 ` [PATCH 1/3] rhashtable: Remove GFP flag from rhashtable_walk_init Herbert Xu
2016-08-18 8:50 ` [PATCH 2/3] MAINTAINERS: Add extra rhashtable maintainer Herbert Xu
@ 2016-08-18 8:50 ` Herbert Xu
2016-08-19 8:09 ` Herbert Xu
2016-08-19 16:34 ` [PATCH 0/3] rhashtable: Get rid of raw table walkers part 1 Thomas Graf
2016-08-19 21:40 ` David Miller
4 siblings, 1 reply; 13+ messages in thread
From: Herbert Xu @ 2016-08-18 8:50 UTC (permalink / raw)
To: David S. Miller, Thomas Graf, netdev
This patch converts the diag dumping code to use the rhashtable
walk code instead of going through rhashtable by hand. The lock
nl_table_lock is now only taken while we process the multicast
list as it's not needed for the rhashtable walk.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/netlink/diag.c | 105 +++++++++++++++++++++++++++++++++++++----------------
1 file changed, 75 insertions(+), 30 deletions(-)
diff --git a/net/netlink/diag.c b/net/netlink/diag.c
index 8dd836a..d4e1eca 100644
--- a/net/netlink/diag.c
+++ b/net/netlink/diag.c
@@ -63,43 +63,77 @@ out_nlmsg_trim:
static int __netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
int protocol, int s_num)
{
+ struct rhashtable_iter *hti = (void *)cb->args[2];
struct netlink_table *tbl = &nl_table[protocol];
- struct rhashtable *ht = &tbl->hash;
- const struct bucket_table *htbl = rht_dereference_rcu(ht->tbl, ht);
struct net *net = sock_net(skb->sk);
struct netlink_diag_req *req;
struct netlink_sock *nlsk;
+ struct rhash_head *obj;
struct sock *sk;
- int ret = 0, num = 0, i;
+ int num = 2;
+ int ret = 0;
req = nlmsg_data(cb->nlh);
- for (i = 0; i < htbl->size; i++) {
- struct rhash_head *pos;
+ if (s_num > 1)
+ goto mc_list;
- rht_for_each_entry_rcu(nlsk, pos, htbl, i, node) {
- sk = (struct sock *)nlsk;
+ if (!hti) {
+ hti = kmalloc(sizeof(*hti), GFP_KERNEL);
+ if (!hti)
+ return -ENOMEM;
- if (!net_eq(sock_net(sk), net))
- continue;
- if (num < s_num) {
- num++;
+ cb->args[2] = (long)hti;
+ }
+
+ if (!s_num) {
+ rhashtable_walk_enter(&tbl->hash, hti);
+ num--;
+ }
+
+ ret = rhashtable_walk_start(hti);
+ if (ret == -EAGAIN)
+ ret = 0;
+ if (ret)
+ goto stop;
+
+ while ((obj = rhashtable_walk_next(hti))) {
+ if (IS_ERR(obj)) {
+ ret = PTR_ERR(obj);
+ if (ret == -EAGAIN) {
+ ret = 0;
continue;
}
+ break;
+ }
- if (sk_diag_fill(sk, skb, req,
- NETLINK_CB(cb->skb).portid,
- cb->nlh->nlmsg_seq,
- NLM_F_MULTI,
- sock_i_ino(sk)) < 0) {
- ret = 1;
- goto done;
- }
+ nlsk = container_of(obj, struct netlink_sock, node);
+ sk = (struct sock *)nlsk;
- num++;
+ if (!net_eq(sock_net(sk), net))
+ continue;
+
+ if (sk_diag_fill(sk, skb, req,
+ NETLINK_CB(cb->skb).portid,
+ cb->nlh->nlmsg_seq,
+ NLM_F_MULTI,
+ sock_i_ino(sk)) < 0) {
+ ret = 1;
+ break;
}
}
+stop:
+ rhashtable_walk_stop(hti);
+ if (ret)
+ goto done;
+
+ rhashtable_walk_exit(hti);
+ cb->args[2] = 0;
+ num++;
+
+mc_list:
+ read_lock(&nl_table_lock);
sk_for_each_bound(sk, &tbl->mc_list) {
if (sk_hashed(sk))
continue;
@@ -116,13 +150,14 @@ static int __netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
NLM_F_MULTI,
sock_i_ino(sk)) < 0) {
ret = 1;
- goto done;
+ break;
}
num++;
}
+ read_unlock(&nl_table_lock);
+
done:
cb->args[0] = num;
- cb->args[1] = protocol;
return ret;
}
@@ -131,20 +166,20 @@ static int netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
{
struct netlink_diag_req *req;
int s_num = cb->args[0];
+ int err = 0;
req = nlmsg_data(cb->nlh);
- rcu_read_lock();
- read_lock(&nl_table_lock);
-
if (req->sdiag_protocol == NDIAG_PROTO_ALL) {
int i;
for (i = cb->args[1]; i < MAX_LINKS; i++) {
- if (__netlink_diag_dump(skb, cb, i, s_num))
+ err = __netlink_diag_dump(skb, cb, i, s_num);
+ if (err)
break;
s_num = 0;
}
+ cb->args[1] = i;
} else {
if (req->sdiag_protocol >= MAX_LINKS) {
read_unlock(&nl_table_lock);
@@ -152,13 +187,22 @@ static int netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
return -ENOENT;
}
- __netlink_diag_dump(skb, cb, req->sdiag_protocol, s_num);
+ err = __netlink_diag_dump(skb, cb, req->sdiag_protocol, s_num);
}
- read_unlock(&nl_table_lock);
- rcu_read_unlock();
+ return err < 0 ? err : skb->len;
+}
+
+static int netlink_diag_dump_done(struct netlink_callback *cb)
+{
+ struct rhashtable_iter *hti = (void *)cb->args[2];
- return skb->len;
+ if (cb->args[0] == 1)
+ rhashtable_walk_exit(hti);
+
+ kfree(hti);
+
+ return 0;
}
static int netlink_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
@@ -172,6 +216,7 @@ static int netlink_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
if (h->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
.dump = netlink_diag_dump,
+ .done = netlink_diag_dump_done,
};
return netlink_dump_start(net->diag_nlsk, skb, h, &c);
} else
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] netlink: Use rhashtable walk interface in diag dump
2016-08-18 8:50 ` [PATCH 3/3] netlink: Use rhashtable walk interface in diag dump Herbert Xu
@ 2016-08-19 8:09 ` Herbert Xu
2016-08-19 8:21 ` [v2 PATCH " Herbert Xu
0 siblings, 1 reply; 13+ messages in thread
From: Herbert Xu @ 2016-08-19 8:09 UTC (permalink / raw)
To: David S. Miller, Thomas Graf, netdev
On Thu, Aug 18, 2016 at 04:50:58PM +0800, Herbert Xu wrote:
> This patch converts the diag dumping code to use the rhashtable
> walk code instead of going through rhashtable by hand. The lock
> nl_table_lock is now only taken while we process the multicast
> list as it's not needed for the rhashtable walk.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Please drop this patch. While the current dump is already buggy
my new version seems worse and returns a lot less entries. I'll
post a new version of this patch when I figure out why.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 13+ messages in thread
* [v2 PATCH 3/3] netlink: Use rhashtable walk interface in diag dump
2016-08-19 8:09 ` Herbert Xu
@ 2016-08-19 8:21 ` Herbert Xu
2016-08-19 16:32 ` Thomas Graf
0 siblings, 1 reply; 13+ messages in thread
From: Herbert Xu @ 2016-08-19 8:21 UTC (permalink / raw)
To: David S. Miller, Thomas Graf, netdev
This patch converts the diag dumping code to use the rhashtable
walk code instead of going through rhashtable by hand. The lock
nl_table_lock is now only taken while we process the multicast
list as it's not needed for the rhashtable walk.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/netlink/diag.c | 103 +++++++++++++++++++++++++++++++++++++----------------
1 file changed, 73 insertions(+), 30 deletions(-)
diff --git a/net/netlink/diag.c b/net/netlink/diag.c
index 8dd836a..3e3e253 100644
--- a/net/netlink/diag.c
+++ b/net/netlink/diag.c
@@ -63,43 +63,75 @@ out_nlmsg_trim:
static int __netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
int protocol, int s_num)
{
+ struct rhashtable_iter *hti = (void *)cb->args[2];
struct netlink_table *tbl = &nl_table[protocol];
- struct rhashtable *ht = &tbl->hash;
- const struct bucket_table *htbl = rht_dereference_rcu(ht->tbl, ht);
struct net *net = sock_net(skb->sk);
struct netlink_diag_req *req;
struct netlink_sock *nlsk;
struct sock *sk;
- int ret = 0, num = 0, i;
+ int num = 2;
+ int ret = 0;
req = nlmsg_data(cb->nlh);
- for (i = 0; i < htbl->size; i++) {
- struct rhash_head *pos;
+ if (s_num > 1)
+ goto mc_list;
- rht_for_each_entry_rcu(nlsk, pos, htbl, i, node) {
- sk = (struct sock *)nlsk;
+ num--;
- if (!net_eq(sock_net(sk), net))
- continue;
- if (num < s_num) {
- num++;
+ if (!hti) {
+ hti = kmalloc(sizeof(*hti), GFP_KERNEL);
+ if (!hti)
+ return -ENOMEM;
+
+ cb->args[2] = (long)hti;
+ }
+
+ if (!s_num)
+ rhashtable_walk_enter(&tbl->hash, hti);
+
+ ret = rhashtable_walk_start(hti);
+ if (ret == -EAGAIN)
+ ret = 0;
+ if (ret)
+ goto stop;
+
+ while ((nlsk = rhashtable_walk_next(hti))) {
+ if (IS_ERR(nlsk)) {
+ ret = PTR_ERR(nlsk);
+ if (ret == -EAGAIN) {
+ ret = 0;
continue;
}
+ break;
+ }
- if (sk_diag_fill(sk, skb, req,
- NETLINK_CB(cb->skb).portid,
- cb->nlh->nlmsg_seq,
- NLM_F_MULTI,
- sock_i_ino(sk)) < 0) {
- ret = 1;
- goto done;
- }
+ sk = (struct sock *)nlsk;
- num++;
+ if (!net_eq(sock_net(sk), net))
+ continue;
+
+ if (sk_diag_fill(sk, skb, req,
+ NETLINK_CB(cb->skb).portid,
+ cb->nlh->nlmsg_seq,
+ NLM_F_MULTI,
+ sock_i_ino(sk)) < 0) {
+ ret = 1;
+ break;
}
}
+stop:
+ rhashtable_walk_stop(hti);
+ if (ret)
+ goto done;
+
+ rhashtable_walk_exit(hti);
+ cb->args[2] = 0;
+ num++;
+
+mc_list:
+ read_lock(&nl_table_lock);
sk_for_each_bound(sk, &tbl->mc_list) {
if (sk_hashed(sk))
continue;
@@ -116,13 +148,14 @@ static int __netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb,
NLM_F_MULTI,
sock_i_ino(sk)) < 0) {
ret = 1;
- goto done;
+ break;
}
num++;
}
+ read_unlock(&nl_table_lock);
+
done:
cb->args[0] = num;
- cb->args[1] = protocol;
return ret;
}
@@ -131,20 +164,20 @@ static int netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
{
struct netlink_diag_req *req;
int s_num = cb->args[0];
+ int err = 0;
req = nlmsg_data(cb->nlh);
- rcu_read_lock();
- read_lock(&nl_table_lock);
-
if (req->sdiag_protocol == NDIAG_PROTO_ALL) {
int i;
for (i = cb->args[1]; i < MAX_LINKS; i++) {
- if (__netlink_diag_dump(skb, cb, i, s_num))
+ err = __netlink_diag_dump(skb, cb, i, s_num);
+ if (err)
break;
s_num = 0;
}
+ cb->args[1] = i;
} else {
if (req->sdiag_protocol >= MAX_LINKS) {
read_unlock(&nl_table_lock);
@@ -152,13 +185,22 @@ static int netlink_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
return -ENOENT;
}
- __netlink_diag_dump(skb, cb, req->sdiag_protocol, s_num);
+ err = __netlink_diag_dump(skb, cb, req->sdiag_protocol, s_num);
}
- read_unlock(&nl_table_lock);
- rcu_read_unlock();
+ return err < 0 ? err : skb->len;
+}
+
+static int netlink_diag_dump_done(struct netlink_callback *cb)
+{
+ struct rhashtable_iter *hti = (void *)cb->args[2];
+
+ if (cb->args[0] == 1)
+ rhashtable_walk_exit(hti);
- return skb->len;
+ kfree(hti);
+
+ return 0;
}
static int netlink_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
@@ -172,6 +214,7 @@ static int netlink_diag_handler_dump(struct sk_buff *skb, struct nlmsghdr *h)
if (h->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
.dump = netlink_diag_dump,
+ .done = netlink_diag_dump_done,
};
return netlink_dump_start(net->diag_nlsk, skb, h, &c);
} else
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [v2 PATCH 3/3] netlink: Use rhashtable walk interface in diag dump
2016-08-19 8:21 ` [v2 PATCH " Herbert Xu
@ 2016-08-19 16:32 ` Thomas Graf
2016-08-19 16:39 ` Herbert Xu
0 siblings, 1 reply; 13+ messages in thread
From: Thomas Graf @ 2016-08-19 16:32 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev
On 08/19/16 at 04:21pm, Herbert Xu wrote:
> This patch converts the diag dumping code to use the rhashtable
> walk code instead of going through rhashtable by hand. The lock
> nl_table_lock is now only taken while we process the multicast
> list as it's not needed for the rhashtable walk.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
LGTM. Curious, what did you use to test this? I've been struggling
to find a good test case.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [v2 PATCH 3/3] netlink: Use rhashtable walk interface in diag dump
2016-08-19 16:32 ` Thomas Graf
@ 2016-08-19 16:39 ` Herbert Xu
0 siblings, 0 replies; 13+ messages in thread
From: Herbert Xu @ 2016-08-19 16:39 UTC (permalink / raw)
To: Thomas Graf; +Cc: David S. Miller, netdev
On Fri, Aug 19, 2016 at 06:32:37PM +0200, Thomas Graf wrote:
> On 08/19/16 at 04:21pm, Herbert Xu wrote:
> > This patch converts the diag dumping code to use the rhashtable
> > walk code instead of going through rhashtable by hand. The lock
> > nl_table_lock is now only taken while we process the multicast
> > list as it's not needed for the rhashtable walk.
> >
> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> LGTM. Curious, what did you use to test this? I've been struggling
> to find a good test case.
ss -A netlink
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] rhashtable: Get rid of raw table walkers part 1
2016-08-18 8:48 [PATCH 0/3] rhashtable: Get rid of raw table walkers part 1 Herbert Xu
` (2 preceding siblings ...)
2016-08-18 8:50 ` [PATCH 3/3] netlink: Use rhashtable walk interface in diag dump Herbert Xu
@ 2016-08-19 16:34 ` Thomas Graf
2016-08-19 21:40 ` David Miller
4 siblings, 0 replies; 13+ messages in thread
From: Thomas Graf @ 2016-08-19 16:34 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev
On 08/18/16 at 04:48pm, Herbert Xu wrote:
> Dave/Tomas, please keep an eye out for any new patches that try
> to introduce raw table walkers and nack them.
Very nice work Herbert. I think we should move the rht_for_each_* macros
to a private header or lib/rhashtable.c to discourage usage altogether.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] rhashtable: Get rid of raw table walkers part 1
2016-08-18 8:48 [PATCH 0/3] rhashtable: Get rid of raw table walkers part 1 Herbert Xu
` (3 preceding siblings ...)
2016-08-19 16:34 ` [PATCH 0/3] rhashtable: Get rid of raw table walkers part 1 Thomas Graf
@ 2016-08-19 21:40 ` David Miller
4 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2016-08-19 21:40 UTC (permalink / raw)
To: herbert; +Cc: tgraf, netdev
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 18 Aug 2016 16:48:54 +0800
> This series starts the process of getting rid of all raw rhashtable
> walkers (e.g., using any of the rht_for_each helpers) from the
> kernel.
>
> We need to do this before I can fix the resize kmalloc failure issue
> by using multi-layered tables.
>
> We should do this anyway because almost all raw table walkers are
> already buggy in that they don't handle multiple rhashtables during
> a resize.
Series applied to net-next with v2 of patch #3.
> Dave/Tomas, please keep an eye out for any new patches that try
> to introduce raw table walkers and nack them.
Ok.
^ permalink raw reply [flat|nested] 13+ messages in thread