 drivers/net/cxgb3/cxgb3_main.c |    3 +--
 include/linux/rtnetlink.h      |    5 ++---
 net/core/rtnetlink.c           |    7 ++++---
 3 files changed, 7 insertions(+), 8 deletions(-)

drivers/net/cxgb3/cxgb3_main.c renamed from drivers/net/cxgb3/cxgb3_main.c~net-use-mutex_is_locked-for-assert_rtnl +1 -2

diff -puN drivers/net/cxgb3/cxgb3_main.c~net-use-mutex_is_locked-for-assert_rtnl drivers/net/cxgb3/cxgb3_main.c
@@ -2191,7 +2191,7 @@ static void check_t3b2_mac(struct adapte
 {
 	int i;
 
-	if (!rtnl_trylock())	/* synchronize with ifdown */
+	if (rtnl_is_locked())	/* synchronize with ifdown */
 		return;
 
 	for_each_port(adapter, i) {

@@ -2219,7 +2219,6 @@ static void check_t3b2_mac(struct adapte
 			p->mac.stats.num_resets++;
 		}
 	}
-	rtnl_unlock();
 }

include/linux/rtnetlink.h renamed from include/linux/rtnetlink.h~net-use-mutex_is_locked-for-assert_rtnl +2 -3

diff -puN include/linux/rtnetlink.h~net-use-mutex_is_locked-for-assert_rtnl include/linux/rtnetlink.h
@@ -751,14 +751,13 @@ extern void rtmsg_ifinfo(int type, struc
 /* RTNL is used as a global lock for all changes to network configuration  */
 extern void rtnl_lock(void);
 extern void rtnl_unlock(void);
-extern int rtnl_trylock(void);
+extern int rtnl_is_locked(void);
 
 extern void rtnetlink_init(void);
 extern void __rtnl_unlock(void);
 
 #define ASSERT_RTNL() do { \
-	if (unlikely(rtnl_trylock())) { \
-		rtnl_unlock(); \
+	if (unlikely(!rtnl_is_locked())) { \
 		printk(KERN_ERR "RTNL: assertion failed at %s (%d)\n", \
 		       __FILE__,  __LINE__); \
 		dump_stack(); \

net/core/rtnetlink.c renamed from net/core/rtnetlink.c~net-use-mutex_is_locked-for-assert_rtnl +4 -3

diff -puN net/core/rtnetlink.c~net-use-mutex_is_locked-for-assert_rtnl net/core/rtnetlink.c
@@ -77,9 +77,10 @@ void rtnl_unlock(void)
 	netdev_run_todo();
 }
 
-int rtnl_trylock(void)
+/* Return non-zero if rtnl_mutex is presently held */
+int rtnl_is_locked(void)
 {
-	return mutex_trylock(&rtnl_mutex);
+	return mutex_is_locked(&rtnl_mutex);
 }
 
 int rtattr_parse(struct rtattr *tb[], int maxattr, struct rtattr *rta, int len)

@@ -1424,7 +1425,7 @@ EXPORT_SYMBOL(rtattr_parse);
 EXPORT_SYMBOL(__rtattr_parse_nested_compat);
 EXPORT_SYMBOL(rtnetlink_put_metrics);
 EXPORT_SYMBOL(rtnl_lock);
-EXPORT_SYMBOL(rtnl_trylock);
+EXPORT_SYMBOL(rtnl_is_locked);
 EXPORT_SYMBOL(rtnl_unlock);
 EXPORT_SYMBOL(rtnl_unicast);
 EXPORT_SYMBOL(rtnl_notify);

Herbert Xu

2007-12-14 08:10 UTC #1 lore raw

Quoted 14 lines · [email protected] wrote:

diff -puN drivers/net/cxgb3/cxgb3_main.c~net-use-mutex_is_locked-for-assert_rtnl drivers/net/cxgb3/cxgb3_main.c
drivers/net/cxgb3/cxgb3_main.c renamed from drivers/net/cxgb3/cxgb3_main.c~net-use-mutex_is_locked-for-assert_rtnl +1 -1
@@ -2191,7 +2191,7 @@ static void check_t3b2_mac(struct adapte
 {
       int i;
 
-       if (!rtnl_trylock())    /* synchronize with ifdown */
+       if (rtnl_is_locked())   /* synchronize with ifdown */
               return;
 
       for_each_port(adapter, i) {

drivers/net/cxgb3/cxgb3_main.c static void check_t3b2_mac(struct adapte · line 2222

Quoted 4 lines

drivers/net/cxgb3/cxgb3_main.c renamed from drivers/net/cxgb3/cxgb3_main.c~net-use-mutex_is_locked-for-assert_rtnl +0 -1

@@ -2219,7 +2219,6 @@ static void check_t3b2_mac(struct adapte
                       p->mac.stats.num_resets++;
               }
       }
-       rtnl_unlock();

This doesn't look right. It seems that they really want trylock here so we should just fix it by removing the bang.

Also, does ASSERT_RTNL still warn when someone calls it from an atomic context? We definitely don't want to lose that check.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Andrew Morton

author

2007-12-14 08:23 UTC #2 lore raw

Quoted 13 lines · On Fri, 14 Dec 2007 16:10:44 +0800 Herbert Xu <[email protected]>…

Quoted 12 lines · [email protected] wrote:
diff -puN drivers/net/cxgb3/cxgb3_main.c~net-use-mutex_is_locked-for-assert_rtnl drivers/net/cxgb3/cxgb3_main.c
drivers/net/cxgb3/cxgb3_main.c renamed from drivers/net/cxgb3/cxgb3_main.c~net-use-mutex_is_locked-for-assert_rtnl +1 -1
@@ -2191,7 +2191,7 @@ static void check_t3b2_mac(struct adapte
 {
       int i;
 
-       if (!rtnl_trylock())    /* synchronize with ifdown */
+       if (rtnl_is_locked())   /* synchronize with ifdown */
               return;
 

Quoted 9 lines

drivers/net/cxgb3/cxgb3_main.c static void check_t3b2_mac(struct adapte · line 2222
Quoted 6 lines
drivers/net/cxgb3/cxgb3_main.c renamed from drivers/net/cxgb3/cxgb3_main.c~net-use-mutex_is_locked-for-assert_rtnl +0 -1
       for_each_port(adapter, i) {
@@ -2219,7 +2219,6 @@ static void check_t3b2_mac(struct adapte
                       p->mac.stats.num_resets++;
               }
       }
-       rtnl_unlock();
This doesn't look right. It seems that they really want trylock here so we should just fix it by removing the bang.

doh.

Quoted 2 lines

Also, does ASSERT_RTNL still warn when someone calls it from an atomic context? We definitely don't want to lose that check.

I don't see how it could warn about that. Nor should it - one might want to check that rtnl_lock is held inside preempt_disable() or spin_lock or whatever.

It might make sense to warn if ASSERT_RTNL is called in in_interrupt() contexts though.

Herbert Xu

2007-12-14 08:30 UTC #3 lore raw

Quoted 7 lines · Andrew Morton

I don't see how it could warn about that. Nor should it - one might want to check that rtnl_lock is held inside preempt_disable() or spin_lock or whatever.
It might make sense to warn if ASSERT_RTNL is called in in_interrupt() contexts though.

Well the paths where ASSERT_RTNL is used should never be in an atomic context. In the past it has been quite useful in pointing out bogus locking practices.

There is currently one path where it's known to warn because of this and it (promiscuous mode) is on my todo list.

Oh and it only warns when you have mutex debugging enabled.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

David Miller

maintainer

2007-12-14 19:15 UTC #4 lore raw

From: Herbert Xu <[email protected]>
Date: Fri, 14 Dec 2007 16:30:37 +0800

Quoted 9 lines · Andrew Morton

Quoted 7 lines
I don't see how it could warn about that. Nor should it - one might want to check that rtnl_lock is held inside preempt_disable() or spin_lock or whatever.
It might make sense to warn if ASSERT_RTNL is called in in_interrupt() contexts though.

Quoted 8 lines

Well the paths where ASSERT_RTNL is used should never be in an atomic context. In the past it has been quite useful in pointing out bogus locking practices.
There is currently one path where it's known to warn because of this and it (promiscuous mode) is on my todo list.
Oh and it only warns when you have mutex debugging enabled.

Right, this change is just totally bogus.

I'm all for using existing facilities to replace hand-crafted copies, but this case is removing useful debugging functionality so it's wrong.

Andrew Morton

author

2007-12-14 23:12 UTC #5 lore raw

Quoted 19 lines · On Fri, 14 Dec 2007 11:15:14 -0800 (PST) David Miller <[email protected]>…

From: Herbert Xu <[email protected]>
Date: Fri, 14 Dec 2007 16:30:37 +0800
Quoted 16 lines · Andrew Morton
Quoted 7 lines
I don't see how it could warn about that. Nor should it - one might want to check that rtnl_lock is held inside preempt_disable() or spin_lock or whatever.
It might make sense to warn if ASSERT_RTNL is called in in_interrupt() contexts though.
Well the paths where ASSERT_RTNL is used should never be in an atomic context. In the past it has been quite useful in pointing out bogus locking practices.
There is currently one path where it's known to warn because of this and it (promiscuous mode) is on my todo list.

Quoted 7 lines

Quoted 1 line
Oh and it only warns when you have mutex debugging enabled.
Right, this change is just totally bogus.
I'm all for using existing facilities to replace hand-crafted copies, but this case is removing useful debugging functionality so it's wrong.

I don't believe that ASSERT_RTNL() presently warns when called from atomic contexts. If it does then I missed it.

Herbert Xu

2007-12-15 04:18 UTC #6 lore raw

Quoted 3 lines · Andrew Morton

I don't believe that ASSERT_RTNL() presently warns when called from atomic contexts. If it does then I missed it.

It does when mutex debugging is enabled.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Andrew Morton

author

2007-12-15 05:45 UTC #7 lore raw

Quoted 7 lines · On Sat, 15 Dec 2007 12:18:27 +0800 Herbert Xu <[email protected]>…

Quoted 3 lines · Andrew Morton
I don't believe that ASSERT_RTNL() presently warns when called from atomic contexts. If it does then I missed it.
It does when mutex debugging is enabled.

That sounds like a bug in mutex_trylock() to me.

Where in the tangled forest of the mutex implementation is the code which does this?

Herbert Xu

2007-12-15 06:10 UTC #8 lore raw

Quoted 2 lines · Andrew Morton

That sounds like a bug in mutex_trylock() to me.

I was relying on

	http://kerneltrap.org/mailarchive/linux-netdev/2007/9/28/325129

which seems to be a bogus claim now that I actually look at the source code. So in that case I'm OK with your patch as long as it warns about hard IRQ usage.

Thanks,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Andrew Morton

author

2007-12-15 10:48 UTC #9 lore raw

Quoted 11 lines · On Sat, 15 Dec 2007 14:10:21 +0800 Herbert Xu <[email protected]>…

Quoted 2 lines · Andrew Morton
That sounds like a bug in mutex_trylock() to me.
I was relying on
	http://kerneltrap.org/mailarchive/linux-netdev/2007/9/28/325129
which seems to be a bogus claim now that I actually look at the source code. So in that case I'm OK with your patch as long as it warns about hard IRQ usage.

When Eric said

Quoted 5 lines

Way way deep in mutex debugging on the slowpath there is a unreadable and incomprehensible WARN_ON in muxtex_trylock that will trigger if you have 10 tons of debugging turned on, and you are in, interrupt context, and you manage to hit the slow path. I think that is a pretty unlikely scenario.

I think he's still right. That's if the warning which he managed to find even still exists.

I think the change which Eric proposed is a good one: it converts ASSERT_RTNL() from an atomic rmw which dirties a cacheline which will sometimes be owned by a different CPU into a plain old read. It's going to make ASSERT_RTNL() heaps cheaper.

Now as a separate issue we (ie: you) need to work out what _other_ things you want ASSERT_RTNL to check apart from "rtnl must be held".

If you want to check that no locks are held (which I think is a bit weird, but whatever) then add might_sleep().

If you want to check that we're not in interrupt context or whatever, then add the checks and be happy. might_sleep() will of course check for in_interrupt(), in_irq(), etc so if you go with a might_sleep() then nothing else needs to be added.

While doing this I'd also suggest that the thing should be uninlined - it'll probably generate less text and it'll give considerably more flexibility for adding new debug fetures. Ones which might be controlled at compile time or runtime. ie:

void __assert_rtnl(const char *file, int line); #define ASSERT_RTNL() __assert_rtnl(__FILE__, __LINE__)

Herbert Xu

2007-12-15 13:10 UTC #10 lore raw

Quoted 11 lines · Andrew Morton

When Eric said
Quoted 5 lines
Way way deep in mutex debugging on the slowpath there is a unreadable and incomprehensible WARN_ON in muxtex_trylock that will trigger if you have 10 tons of debugging turned on, and you are in, interrupt context, and you manage to hit the slow path. I think that is a pretty unlikely scenario.
I think he's still right. That's if the warning which he managed to find even still exists.

Well at the very start he said:

: When mutex debugging is on taking a mutex complains if we are not : allowed to sleep. At that point we have called netif_tx_lock_bh : so we are clearly not allowed to sleep. Arguably this is not a : problem for mutex_trylock.

This is what threw me off as it implied that we were warning about ASSERT_RTNL calls in any atomic context, including those with just soft IRQs disabled, which is an important distinction for the networking subsystem.

Had he said that it was just IRQ handlers rather than soft IRQs then we wouldn't be having this discussion now :)

Quoted 4 lines

I think the change which Eric proposed is a good one: it converts ASSERT_RTNL() from an atomic rmw which dirties a cacheline which will sometimes be owned by a different CPU into a plain old read. It's going to make ASSERT_RTNL() heaps cheaper.

I agree. Although paths using ASSERT_RTNL shouldn't be performance critical since the RTNL should only be held for control operations as opposed to data transport.

Quoted 2 lines

Now as a separate issue we (ie: you) need to work out what _other_ things you want ASSERT_RTNL to check apart from "rtnl must be held".

Since we have now established that ASSERT_RTNL never actually warned about usage on paths with BH off, I think Eric's original patch is fine as it is and I owe him an apology.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

David Miller

maintainer

2007-12-16 05:44 UTC #11 lore raw

From: Herbert Xu <[email protected]>
Date: Sat, 15 Dec 2007 21:10:16 +0800

Quoted 8 lines · Andrew Morton

Quoted 3 lines
Now as a separate issue we (ie: you) need to work out what _other_ things you want ASSERT_RTNL to check apart from "rtnl must be held".
Since we have now established that ASSERT_RTNL never actually warned about usage on paths with BH off, I think Eric's original patch is fine as it is and I owe him an apology.

Ok, same here...

Such situations (ASSERT_RTNL() in atomic context) have always been bugs though, and that continues to be true and I think the check should be added somehow.

Herbert Xu

2007-12-16 07:13 UTC #12 lore raw

Quoted 4 lines · David Miller

Such situations (ASSERT_RTNL() in atomic context) have always been bugs though, and that continues to be true and I think the check should be added somehow.

OK once I've fixed the set_multicast path I'll do an audit of the existing ASSERT_RTNL users and then add the might_sleep check.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Jarek Poplawski

2007-12-16 18:04 UTC #13 lore raw

In reply to Andrew Morton

Andrew Morton wrote, On 12/15/2007 11:48 AM:

Quoted 14 lines · On Sat, 15 Dec 2007 14:10:21 +0800 Herbert Xu <[email protected]>…

Quoted 10 lines · Andrew Morton
Quoted 1 line
That sounds like a bug in mutex_trylock() to me.
I was relying on
	http://kerneltrap.org/mailarchive/linux-netdev/2007/9/28/325129
which seems to be a bogus claim now that I actually look at the source code. So in that case I'm OK with your patch as long as it warns about hard IRQ usage.
When Eric said

Quoted 8 lines

Quoted 5 lines
Way way deep in mutex debugging on the slowpath there is a unreadable and incomprehensible WARN_ON in muxtex_trylock that will trigger if you have 10 tons of debugging turned on, and you are in, interrupt context, and you manage to hit the slow path. I think that is a pretty unlikely scenario.
I think he's still right. That's if the warning which he managed to find even still exists.

It seemed to exist a few days ago:
http://kerneltrap.org/mailarchive/linux-netdev/2007/12/4/473123

Btw., I don't know which of the patches: Eric's or yours will be chosen, but, IMHO, there is no reason to remove rtnl_trylock(), which can be still useful, just like mutex_trylock() is.

Regards,
Jarek P.

Herbert Xu

2007-12-17 01:27 UTC #14 lore raw

Quoted 7 lines · Jarek Poplawski

It seemed to exist a few days ago:
http://kerneltrap.org/mailarchive/linux-netdev/2007/12/4/473123
Btw., I don't know which of the patches: Eric's or yours will be chosen, but, IMHO, there is no reason to remove rtnl_trylock(), which can be still useful, just like mutex_trylock() is.

Doh! Andrew was too convincing :) I misread the grep result on in_interrupt. Of course that function returns true if we're either in an IRQ handler or have BH off.

I retract what I've said in this thread and continue to oppose this change without a might_sleep.

Thanks,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Jarek Poplawski

2007-12-17 07:20 UTC #15 lore raw

Quoted 9 lines · Herbert Xu

Quoted 7 lines · Jarek Poplawski
It seemed to exist a few days ago:
http://kerneltrap.org/mailarchive/linux-netdev/2007/12/4/473123
Btw., I don't know which of the patches: Eric's or yours will be chosen, but, IMHO, there is no reason to remove rtnl_trylock(), which can be still useful, just like mutex_trylock() is.

Quoted 7 lines

Doh! Andrew was too convincing :) I misread the grep result on in_interrupt. Of course that function returns true if we're either in an IRQ handler or have BH off.
I retract what I've said in this thread and continue to oppose this change without a might_sleep.

...And I think some change is needed here. Btw., I proposed to change this long time ago too. There were no response - only Ben Greear mentioned about some applications, which could rely on the trylock way. I didn't understand what he was talking about at all - and it didn't change until I've read this and Eric's patch thread!

So, I was surprised, probably just like Eric, ASSERT_RTNL is 2 in 1, with this atomic somewhere deep in mind. IMHO this should be better commented at least. But it's still dubious to me: using trylock this way makes impossible to verify (eg. by lockdep) recursion cases, when lock is taken with trylock in a loop.

So, I think using might_sleep() explicitly would be much more readable or, otherwise, Patrick's proposal with adding ASSERT_RTNL_ATOMIC would implicitly signal the real meaning of the other one.

Btw. #2: David Miller gave this example of ASSERT_RTNL use:

	ASSERT_RTNL();
	page = alloc_page(GFP_KERNEL);

But isn't there a debugging duplication: it seems alloc_page() is used in so many places and this check for GFP is/should_be there already?

Regards,
Jarek P.

Herbert Xu

2007-12-17 07:31 UTC #16 lore raw

Quoted 8 lines · Jarek Poplawski

Btw. #2: David Miller gave this example of ASSERT_RTNL use:
	ASSERT_RTNL();
	page = alloc_page(GFP_KERNEL);
But isn't there a debugging duplication: it seems alloc_page() is used in so many places and this check for GFP is/should_be there already?

On some paths this may be buried a conditional. Also if you replace it with a mutex_is_locked without the may_sleep it won't catch the case that started all this, namely the promiscuous path.

Cheers,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Jarek Poplawski

2007-12-17 07:52 UTC #17 lore raw

Quoted 7 lines · Herbert Xu

Quoted 6 lines · Jarek Poplawski
Btw. #2: David Miller gave this example of ASSERT_RTNL use:
	ASSERT_RTNL();
	page = alloc_page(GFP_KERNEL);

Quoted 7 lines

Quoted 2 lines
But isn't there a debugging duplication: it seems alloc_page() is used in so many places and this check for GFP is/should_be there already?
On some paths this may be buried a conditional. Also if you replace it with a mutex_is_locked without the may_sleep it won't catch the case that started all this, namely the promiscuous path.

Right! There is only a question how much is such cases vs. checked already, because then a might_sleep is really more readable when added explicitly.

Jarek P.

Jarek Poplawski

2007-12-17 07:38 UTC #18 lore raw

In reply to Jarek Poplawski

Quoted 1 line · Jarek Poplawski

On Mon, Dec 17, 2007 at 09:26:32AM +0800, Herbert Xu wrote:

...

Quoted 2 lines

Quoted 2 lines
I retract what I've said in this thread and continue to oppose this change without a might_sleep.

...

Quoted 4 lines

So, I think using might_sleep() explicitly would be much more readable or, otherwise, Patrick's proposal with adding ASSERT_RTNL_ATOMIC would implicitly signal the real meaning of the other one.

OOPS! I've looped again! Of course, ASSERT_RTNL with might_sleep() would be explicit enough by itself (if we don't believe atomicity is debugged enough). So, this atomic version could be usable for other reasons.

Jarek P.

David Miller

maintainer

2007-12-16 05:37 UTC #19 lore raw

In reply to Andrew Morton

From: Andrew Morton <[email protected]>
Date: Fri, 14 Dec 2007 21:44:18 -0800

Quoted 1 line

That sounds like a bug in mutex_trylock() to me.

I disagree, I have yet to see a legitimate case where doing a trylock on a mutex lock didn't turn out to be a bug when performed in an atomic context.

This is particularly true in the networking, the bonding driver has provided some excellent examples over the years :-)

Johannes Berg

2007-12-14 12:37 UTC #20 lore raw

In reply to Andrew Morton

Quoted 4 lines

I don't see how it could warn about that. Nor should it - one might want to check that rtnl_lock is held inside preempt_disable() or spin_lock or whatever.

I agree with this. IIRC I removed some ASSERT_RTNL()s in the wireless code (or maybe it was only during testing patches) where we had a function that required only the rtnl to be held but in certain contexts was called from within an RCU section.

johannes

Herbert Xu

2007-12-14 12:47 UTC #21 lore raw

Quoted 5 lines · Johannes Berg

I agree with this. IIRC I removed some ASSERT_RTNL()s in the wireless code (or maybe it was only during testing patches) where we had a function that required only the rtnl to be held but in certain contexts was called from within an RCU section.

Please point me to the actual code so I can see if this is legit or not.

Thanks,

Signature · 4 lines

Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Johannes Berg

2007-12-14 12:55 UTC #22 lore raw

Quoted 8 lines

Quoted 5 lines
I agree with this. IIRC I removed some ASSERT_RTNL()s in the wireless code (or maybe it was only during testing patches) where we had a function that required only the rtnl to be held but in certain contexts was called from within an RCU section.
Please point me to the actual code so I can see if this is legit or not.

I don't think I have that case any more since now my interface list is either protected by RCU or the rtnl.

johannes

David Miller

maintainer NAK

2007-12-14 19:31 UTC #23 lore raw

In reply to [email protected]

From: [email protected]
Date: Thu, 13 Dec 2007 16:02:36 -0800

Quoted 10 lines

From: Andrew Morton <[email protected]>
ASSERT_RTNL() uses mutex_trylock(), but it's better to use mutex_is_locked().
Make that change, and remove rtnl_trylock() altogether.
(not tested yet!)
Cc "David S. Miller" <[email protected]>
Signed-off-by Andrew Morton <[email protected]>

NACK, as explained please remove this until the replacement doesn't remove valid checks which are done currently.