Re: [PATCH 1/1][PKT_CLS] Avoid multiple tree locks

2007-03-27 Thread David Miller
From: Patrick McHardy [EMAIL PROTECTED] Date: Thu, 22 Mar 2007 12:36:17 +0100 jamal wrote: The mutex is certainly a cleaner approach; and a lot of the RCU protection would go away. I like it. Not as much as I initially thought, but at least we would have consistent locking for the dump

Re: [PATCH 1/1][PKT_CLS] Avoid multiple tree locks

2007-03-23 Thread jamal
On Thu, 2007-22-03 at 12:36 +0100, Patrick McHardy wrote: jamal wrote: On Wed, 2007-21-03 at 15:04 +0100, Patrick McHardy wrote: We can remove qdisc_tree_lock since with this patch all changes and all tree walking happen under the RTNL. We still need to keep dev-queue_lock for the data

Re: [PATCH 1/1][PKT_CLS] Avoid multiple tree locks

2007-03-22 Thread Patrick McHardy
jamal wrote: On Wed, 2007-21-03 at 15:04 +0100, Patrick McHardy wrote: These (compile tested) patches demonstrate the idea. The first one lets netlink_kernel_create users specify a mutex that should be held during dump callbacks, the second one uses this for rtnetlink and changes

[PATCH 1/1][PKT_CLS] Avoid multiple tree locks

2007-03-21 Thread jamal
Seems to have been around a while. IMO, mterial for 2.6.21 but not stable. I have only compile-tested but it looks right(tm). I could have moved the lock down, but this looked safer. cheers, jamal [PKT_CLS] Avoid multiple tree locks This fixes: When dumping filters the tree is locked first in the

Re: [PATCH 1/1][PKT_CLS] Avoid multiple tree locks

2007-03-21 Thread Patrick McHardy
jamal wrote: Seems to have been around a while. IMO, mterial for 2.6.21 but not stable. I have only compile-tested but it looks right(tm). Its harmless since its a read lock, which can be nested. I actually don't see any need for qdisc_tree_lock at all, all changes and all walking is done

Re: [PATCH 1/1][PKT_CLS] Avoid multiple tree locks

2007-03-21 Thread Patrick McHardy
Patrick McHardy wrote: jamal wrote: Seems to have been around a while. IMO, mterial for 2.6.21 but not stable. I have only compile-tested but it looks right(tm). Its harmless since its a read lock, which can be nested. I actually don't see any need for qdisc_tree_lock at all, all

Re: [PATCH 1/1][PKT_CLS] Avoid multiple tree locks

2007-03-21 Thread jamal
On Wed, 2007-21-03 at 11:10 +0100, Patrick McHardy wrote: Its harmless since its a read lock, which can be nested. I actually don't see any need for qdisc_tree_lock at all, all changes and all walking is done under the RTNL, which is why I've removed it in my (upcoming) patches. I suggest to

Re: [PATCH 1/1][PKT_CLS] Avoid multiple tree locks

2007-03-21 Thread Patrick McHardy
Patrick McHardy wrote: Its harmless since its a read lock, which can be nested. I actually don't see any need for qdisc_tree_lock at all, all changes and all walking is done under the RTNL, which is why I've removed it in my (upcoming) patches. I suggest to leave it as is for now so I don't need

Re: [PATCH 1/1][PKT_CLS] Avoid multiple tree locks

2007-03-21 Thread Patrick McHardy
Patrick McHardy wrote: Alexey just explained to me why we do need qdisc_tree_lock in private mail. While dumping only the first skb is filled under the RTNL, while filling further skbs we don't hold the RTNL anymore. So I will probably have to drop that patch. What we could do is replace

Re: [PATCH 1/1][PKT_CLS] Avoid multiple tree locks

2007-03-21 Thread Patrick McHardy
Patrick McHardy wrote: Patrick McHardy wrote: Alexey just explained to me why we do need qdisc_tree_lock in private mail. While dumping only the first skb is filled under the RTNL, while filling further skbs we don't hold the RTNL anymore. So I will probably have to drop that patch. What we

Re: [PATCH 1/1][PKT_CLS] Avoid multiple tree locks

2007-03-21 Thread jamal
On Wed, 2007-21-03 at 15:04 +0100, Patrick McHardy wrote: Patrick McHardy wrote: What we could do is replace the netlink cb_lock spinlock by a user-supplied mutex (supplied to netlink_kernel_create, rtnl_mutex in this case). That would put the entire dump under the rtnl and allow us to