* [PATCH 10/16] drivers/net/wireless/wl12xx: Use available error codes
From: Julia Lawall @ 2010-08-16 16:27 UTC (permalink / raw)
To: Luciano Coelho, John W. Linville, linux-wireless, netdev,
linux-kernel, kernel-ja
From: Julia Lawall <julia@diku.dk>
Error codes are stored in ret, but the return value is always 0. Return
ret instead.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r@
local idexpression x;
constant C;
@@
if (...) { ...
x = -C
... when != x
(
return <+...x...+>;
|
return NULL;
|
return;
|
* return ...;
)
}
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
This changes the semantics and has not been tested.
drivers/net/wireless/wl12xx/wl1271_acx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/wl12xx/wl1271_acx.c b/drivers/net/wireless/wl12xx/wl1271_acx.c
index bb245f0..f03ad08 100644
--- a/drivers/net/wireless/wl12xx/wl1271_acx.c
+++ b/drivers/net/wireless/wl12xx/wl1271_acx.c
@@ -269,7 +269,7 @@ int wl1271_acx_pd_threshold(struct wl1271 *wl)
out:
kfree(pd);
- return 0;
+ return ret;
}
int wl1271_acx_slot(struct wl1271 *wl, enum acx_slot_type slot_time)
^ permalink raw reply related
* [PATCH 13/16] net/ax25: Use available error codes
From: Julia Lawall @ 2010-08-16 16:28 UTC (permalink / raw)
To: Joerg Reuter, Ralf Baechle, D, avid S. Miller, linux-hams,
linux-kernel, netdev
From: Julia Lawall <julia@diku.dk>
Error codes are stored in err, but the return value is always 0. Return
err instead.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r@
local idexpression x;
constant C;
@@
if (...) { ...
x = -C
... when != x
(
return <+...x...+>;
|
return NULL;
|
return;
|
* return ...;
)
}
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
This changes the semantics of the function and has not been tested.
net/ax25/af_ax25.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index cfdfd7e..26eaebf 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -1103,7 +1103,7 @@ done:
out:
release_sock(sk);
- return 0;
+ return err;
}
/*
^ permalink raw reply related
* [PATCH 16/16] drivers/net/qlcnic: Use available error codes
From: Julia Lawall @ 2010-08-16 16:29 UTC (permalink / raw)
To: Amit Kumar Salecha, Anirban Chakraborty, linux-driver, netdev,
linux-kernel, kern
From: Julia Lawall <julia@diku.dk>
The error code is stored in the variable err, but it is the variable ret
that is returned instead. So store the error code in ret. Err is then
useless.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@r@
local idexpression x;
constant C;
@@
if (...) { ...
x = -C
... when != x
(
return <+...x...+>;
|
return NULL;
|
return;
|
* return ...;
)
}
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
This changes the semantics and has not been tested.
drivers/net/qlcnic/qlcnic_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index b9615bd..142ab1d 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -474,7 +474,7 @@ static int
qlcnic_init_pci_info(struct qlcnic_adapter *adapter)
{
struct qlcnic_pci_info pci_info[QLCNIC_MAX_PCI_FUNC];
- int i, ret = 0, err;
+ int i, ret = 0;
u8 pfn;
if (!adapter->npars)
@@ -487,7 +487,7 @@ qlcnic_init_pci_info(struct qlcnic_adapter *adapter)
adapter->eswitch = kzalloc(sizeof(struct qlcnic_eswitch) *
QLCNIC_NIU_MAX_XG_PORTS, GFP_KERNEL);
if (!adapter->eswitch) {
- err = -ENOMEM;
+ ret = -ENOMEM;
goto err_eswitch;
}
^ permalink raw reply related
* [iproute2] iproute2: Fix filtering related to flushing IP addresses.
From: Ben Greear @ 2010-08-16 17:00 UTC (permalink / raw)
To: netdev; +Cc: Ben Greear
The old 'ip addr flush' logic had several flaws:
* It reversed logic for primary v/s secondary flags
(though, it sort of worked right anyway)
* The code tried to remove secondaries and then primaries,
but in practice, it always removed one primary per loop,
which not at all efficient.
* The filter logic in the core would run only the first
filter in most cases.
* If you used '-s -s', the ifa_flags member would be
modified, which could make future filters fail
to function fine.
This patch attempts to fix all of these issues.
Tested-by: Brian Haley <brian.haley@hp.com>
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 3a411b1... 19b3d6e... M ip/ipaddress.c
:100644 100644 cfeb894... ee4f045... M lib/libnetlink.c
ip/ipaddress.c | 34 +++++++++++++++++++++++-----------
lib/libnetlink.c | 23 ++++++++++++++++-------
2 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 3a411b1..19b3d6e 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -453,6 +453,8 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
struct ifaddrmsg *ifa = NLMSG_DATA(n);
int len = n->nlmsg_len;
int deprecated = 0;
+ /* Use local copy of ifa_flags to not interfere with filtering code */
+ unsigned int ifa_flags;
struct rtattr * rta_tb[IFA_MAX+1];
char abuf[256];
SPRINT_BUF(b1);
@@ -572,40 +574,41 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
abuf, sizeof(abuf)));
}
fprintf(fp, "scope %s ", rtnl_rtscope_n2a(ifa->ifa_scope, b1, sizeof(b1)));
+ ifa_flags = ifa->ifa_flags;
if (ifa->ifa_flags&IFA_F_SECONDARY) {
- ifa->ifa_flags &= ~IFA_F_SECONDARY;
+ ifa_flags &= ~IFA_F_SECONDARY;
if (ifa->ifa_family == AF_INET6)
fprintf(fp, "temporary ");
else
fprintf(fp, "secondary ");
}
if (ifa->ifa_flags&IFA_F_TENTATIVE) {
- ifa->ifa_flags &= ~IFA_F_TENTATIVE;
+ ifa_flags &= ~IFA_F_TENTATIVE;
fprintf(fp, "tentative ");
}
if (ifa->ifa_flags&IFA_F_DEPRECATED) {
- ifa->ifa_flags &= ~IFA_F_DEPRECATED;
+ ifa_flags &= ~IFA_F_DEPRECATED;
deprecated = 1;
fprintf(fp, "deprecated ");
}
if (ifa->ifa_flags&IFA_F_HOMEADDRESS) {
- ifa->ifa_flags &= ~IFA_F_HOMEADDRESS;
+ ifa_flags &= ~IFA_F_HOMEADDRESS;
fprintf(fp, "home ");
}
if (ifa->ifa_flags&IFA_F_NODAD) {
- ifa->ifa_flags &= ~IFA_F_NODAD;
+ ifa_flags &= ~IFA_F_NODAD;
fprintf(fp, "nodad ");
}
if (!(ifa->ifa_flags&IFA_F_PERMANENT)) {
fprintf(fp, "dynamic ");
} else
- ifa->ifa_flags &= ~IFA_F_PERMANENT;
+ ifa_flags &= ~IFA_F_PERMANENT;
if (ifa->ifa_flags&IFA_F_DADFAILED) {
- ifa->ifa_flags &= ~IFA_F_DADFAILED;
+ ifa_flags &= ~IFA_F_DADFAILED;
fprintf(fp, "dadfailed ");
}
- if (ifa->ifa_flags)
- fprintf(fp, "flags %02x ", ifa->ifa_flags);
+ if (ifa_flags)
+ fprintf(fp, "flags %02x ", ifa_flags);
if (rta_tb[IFA_LABEL])
fprintf(fp, "%s", (char*)RTA_DATA(rta_tb[IFA_LABEL]));
if (rta_tb[IFA_CACHEINFO]) {
@@ -638,7 +641,7 @@ int print_addrinfo_primary(const struct sockaddr_nl *who, struct nlmsghdr *n,
{
struct ifaddrmsg *ifa = NLMSG_DATA(n);
- if (!ifa->ifa_flags & IFA_F_SECONDARY)
+ if (ifa->ifa_flags & IFA_F_SECONDARY)
return 0;
return print_addrinfo(who, n, arg);
@@ -649,7 +652,7 @@ int print_addrinfo_secondary(const struct sockaddr_nl *who, struct nlmsghdr *n,
{
struct ifaddrmsg *ifa = NLMSG_DATA(n);
- if (ifa->ifa_flags & IFA_F_SECONDARY)
+ if (!(ifa->ifa_flags & IFA_F_SECONDARY))
return 0;
return print_addrinfo(who, n, arg);
@@ -849,6 +852,7 @@ static int ipaddr_list_or_flush(int argc, char **argv, int flush)
exit(1);
}
if (filter.flushed == 0) {
+flush_done:
if (show_stats) {
if (round == 0)
printf("Nothing to flush.\n");
@@ -866,6 +870,14 @@ static int ipaddr_list_or_flush(int argc, char **argv, int flush)
printf("\n*** Round %d, deleting %d addresses ***\n", round, filter.flushed);
fflush(stdout);
}
+
+ /* If we are flushing, and specifying primary, then we
+ * want to flush only a single round. Otherwise, we'll
+ * start flushing secondaries that were promoted to
+ * primaries.
+ */
+ if (!(filter.flags & IFA_F_SECONDARY) && (filter.flagmask & IFA_F_SECONDARY))
+ goto flush_done;
}
fprintf(stderr, "*** Flush remains incomplete after %d rounds. ***\n", MAX_ROUNDS); fflush(stderr);
return 1;
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index cfeb894..ee4f045 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -189,6 +189,8 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
while (1) {
int status;
const struct rtnl_dump_filter_arg *a;
+ int found_done = 0;
+ int msglen = 0;
iov.iov_len = sizeof(buf);
status = recvmsg(rth->fd, &msg, 0);
@@ -208,8 +210,9 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
for (a = arg; a->filter; a++) {
struct nlmsghdr *h = (struct nlmsghdr*)buf;
+ msglen = status;
- while (NLMSG_OK(h, status)) {
+ while (NLMSG_OK(h, msglen)) {
int err;
if (nladdr.nl_pid != 0 ||
@@ -224,8 +227,10 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
goto skip_it;
}
- if (h->nlmsg_type == NLMSG_DONE)
- return 0;
+ if (h->nlmsg_type == NLMSG_DONE) {
+ found_done = 1;
+ break; /* process next filter */
+ }
if (h->nlmsg_type == NLMSG_ERROR) {
struct nlmsgerr *err = (struct nlmsgerr*)NLMSG_DATA(h);
if (h->nlmsg_len < NLMSG_LENGTH(sizeof(struct nlmsgerr))) {
@@ -242,15 +247,19 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
return err;
skip_it:
- h = NLMSG_NEXT(h, status);
+ h = NLMSG_NEXT(h, msglen);
}
- } while (0);
+ }
+
+ if (found_done)
+ return 0;
+
if (msg.msg_flags & MSG_TRUNC) {
fprintf(stderr, "Message truncated\n");
continue;
}
- if (status) {
- fprintf(stderr, "!!!Remnant of size %d\n", status);
+ if (msglen) {
+ fprintf(stderr, "!!!Remnant of size %d\n", msglen);
exit(1);
}
}
--
1.6.2.5
^ permalink raw reply related
* Re: [iproute2] iproute2: Fix 'addr flush secondary' logic.
From: Ben Greear @ 2010-08-16 17:01 UTC (permalink / raw)
To: Brian Haley; +Cc: netdev
In-Reply-To: <4C696014.4080508@hp.com>
On 08/16/2010 08:58 AM, Brian Haley wrote:
> On 08/13/2010 03:49 PM, Ben Greear wrote:
>> Attached is an updated patch to fix some of the errors you
>> mentioned. It also fixes a bug when '-s -s' is used: The old
>> code modified the ifa flags, which made it not handle the check
>> for primary v/s secondary correctly in the next filter.
>
> Thanks for the update, nitpicking comments below.
>
> BTW, promote_secondaries was the reason for some of my testing
> issues as I don't usually run with that enabled.
Thanks for all the help and suggestions. I just sent an
'official' patch with your final suggestions included.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* [LOCKDEP BUG][2.6.36-rc1] xt_info_wrlock?
From: Steven Rostedt @ 2010-08-16 17:07 UTC (permalink / raw)
To: netdev; +Cc: LKML, David S. Miller, Eric Dumazet, Patrick McHardy
Hi, I hit this when booting 2.6.36-rc1:
=================================
[ INFO: inconsistent lock state ]
2.6.36-rc1 #2937
---------------------------------
inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
ifup-eth/3288 [HC0[0]:SC1[2]:HE1:SE0] takes:
(&(&lock->lock)->rlock){+.?...}, at: [<ffffffffa0166eef>] ip6t_do_table+0x8a/0x3f1 [ip6_tables]
{SOFTIRQ-ON-W} state was registered at:
[<ffffffff8107b08e>] __lock_acquire+0x756/0x93c
[<ffffffff8107b374>] lock_acquire+0x100/0x12d
[<ffffffff813f4ec3>] _raw_spin_lock+0x40/0x73
[<ffffffffa01664b1>] get_counters+0xb2/0x168 [ip6_tables]
[<ffffffffa01665a3>] alloc_counters+0x3c/0x47 [ip6_tables]
[<ffffffffa0167a7b>] do_ip6t_get_ctl+0x10c/0x363 [ip6_tables]
[<ffffffff813863a2>] nf_sockopt+0x5a/0x86
[<ffffffff813863e6>] nf_getsockopt+0x18/0x1a
[<ffffffffa034c1ff>] ipv6_getsockopt+0x84/0xba [ipv6]
[<ffffffffa0353289>] rawv6_getsockopt+0x42/0x4b [ipv6]
[<ffffffff81355571>] sock_common_getsockopt+0x14/0x16
[<ffffffff813525bb>] sys_getsockopt+0x7a/0x9b
[<ffffffff8100ad32>] system_call_fastpath+0x16/0x1b
irq event stamp: 40
hardirqs last enabled at (40): [<ffffffff813f5ad5>] _raw_spin_unlock_irqrestore+0x47/0x79
hardirqs last disabled at (39): [<ffffffff813f5036>] _raw_spin_lock_irqsave+0x2b/0x92
softirqs last enabled at (0): [<ffffffff8104975a>] copy_process+0x40e/0x11ce
softirqs last disabled at (9): [<ffffffff8100bc9c>] call_softirq+0x1c/0x30
other info that might help us debug this:
3 locks held by ifup-eth/3288:
#0: (&idev->mc_ifc_timer){+.-...}, at: [<ffffffff8105841c>] run_timer_softirq+0x1f5/0x3e6
#1: (rcu_read_lock){.+.+..}, at: [<ffffffffa03578ca>] mld_sendpack+0x0/0x3ab [ipv6]
#2: (rcu_read_lock){.+.+..}, at: [<ffffffff81384f83>] nf_hook_slow+0x0/0x119
stack backtrace:
Pid: 3288, comm: ifup-eth Not tainted 2.6.36-rc1 #2937
Call Trace:
<IRQ> [<ffffffff81077ae6>] print_usage_bug+0x1a4/0x1b5
[<ffffffff810164fa>] ? save_stack_trace+0x2f/0x4c
[<ffffffff8106cd0c>] ? local_clock+0x40/0x59
[<ffffffff810786b6>] ? check_usage_forwards+0x0/0xcf
[<ffffffff81077de1>] mark_lock+0x2ea/0x51f
[<ffffffff8107b014>] __lock_acquire+0x6dc/0x93c
[<ffffffff8106cd0c>] ? local_clock+0x40/0x59
[<ffffffffa0166eef>] ? ip6t_do_table+0x8a/0x3f1 [ip6_tables]
[<ffffffff8107b374>] lock_acquire+0x100/0x12d
[<ffffffffa0166eef>] ? ip6t_do_table+0x8a/0x3f1 [ip6_tables]
[<ffffffff81011149>] ? sched_clock+0x9/0xd
[<ffffffff813f4ec3>] _raw_spin_lock+0x40/0x73
[<ffffffffa0166eef>] ? ip6t_do_table+0x8a/0x3f1 [ip6_tables]
[<ffffffffa0166eef>] ip6t_do_table+0x8a/0x3f1 [ip6_tables]
[<ffffffff810771db>] ? trace_hardirqs_off_caller+0x1f/0x9e
[<ffffffff81384f83>] ? nf_hook_slow+0x0/0x119
[<ffffffffa010601c>] ip6table_filter_hook+0x1c/0x20 [ip6table_filter]
[<ffffffff81384f40>] nf_iterate+0x46/0x89
[<ffffffffa035627b>] ? dst_output+0x0/0x5c [ipv6]
[<ffffffff8138501b>] nf_hook_slow+0x98/0x119
[<ffffffffa035627b>] ? dst_output+0x0/0x5c [ipv6]
[<ffffffffa0349557>] ? icmp6_dst_alloc+0x0/0x1b2 [ipv6]
[<ffffffffa0357b01>] mld_sendpack+0x237/0x3ab [ipv6]
[<ffffffff81051475>] ? local_bh_enable_ip+0xc7/0xeb
[<ffffffffa0358390>] mld_ifc_timer_expire+0x254/0x28d [ipv6]
[<ffffffff810584ed>] run_timer_softirq+0x2c6/0x3e6
[<ffffffff8105841c>] ? run_timer_softirq+0x1f5/0x3e6
[<ffffffffa035813c>] ? mld_ifc_timer_expire+0x0/0x28d [ipv6]
[<ffffffff8105169e>] ? __do_softirq+0x79/0x247
[<ffffffff81051763>] __do_softirq+0x13e/0x247
[<ffffffff8100bc9c>] call_softirq+0x1c/0x30
[<ffffffff8100d32f>] do_softirq+0x4b/0xa3
[<ffffffff8105120a>] irq_exit+0x4a/0x95
[<ffffffff813fc185>] smp_apic_timer_interrupt+0x8c/0x9a
[<ffffffff8100b753>] apic_timer_interrupt+0x13/0x20
<EOI> [<ffffffff8102c72a>] ? native_flush_tlb_global+0x2b/0x32
[<ffffffff81031fec>] kernel_map_pages+0x12c/0x142
[<ffffffff810cc89a>] free_pages_prepare+0x14c/0x15d
[<ffffffff810cc9c8>] free_hot_cold_page+0x2d/0x165
[<ffffffff810ccb2b>] __free_pages+0x2b/0x34
[<ffffffff810ccb7d>] free_pages+0x49/0x4e
[<ffffffff81033871>] pgd_free+0x71/0x79
[<ffffffff81048b1b>] __mmdrop+0x27/0x54
[<ffffffff81042c62>] finish_task_switch+0xb4/0xe4
[<ffffffff81042bae>] ? finish_task_switch+0x0/0xe4
[<ffffffff810096e7>] ? __switch_to+0x1a9/0x297
[<ffffffff81042df3>] schedule_tail+0x30/0xa7
[<ffffffff8100ac33>] ret_from_fork+0x13/0x80
I noticed in net/ipv6/netfilter/ip6_tables.c in get_counters() as with
other "get_counters()" functions do not block bottom halves anymore as
to this commit:
commit 24b36f0193467fa727b85b4c004016a8dae999b9
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon Aug 2 16:49:01 2010 +0200
netfilter: {ip,ip6,arp}_tables: dont block bottom half more than necessary
We currently disable BH for the whole duration of get_counters()
Now we take xt_info_wrlock(cpu) lock out of BH disabling. And that lock
even has the following comment:
/*
* The "writer" side needs to get exclusive access to the lock,
* regardless of readers. This must be called with bottom half
* processing (and thus also preemption) disabled.
*/
static inline void xt_info_wrlock(unsigned int cpu)
As lockdep has proven, this is not satisfied.
-- Steve
^ permalink raw reply
* Re: [LOCKDEP BUG][2.6.36-rc1] xt_info_wrlock?
From: Eric Dumazet @ 2010-08-16 17:31 UTC (permalink / raw)
To: Steven Rostedt; +Cc: netdev, LKML, David S. Miller, Patrick McHardy
In-Reply-To: <1281978469.3268.55.camel@gandalf.stny.rr.com>
Le lundi 16 août 2010 à 13:07 -0400, Steven Rostedt a écrit :
> Hi, I hit this when booting 2.6.36-rc1:
>
> =================================
> [ INFO: inconsistent lock state ]
> 2.6.36-rc1 #2937
> ---------------------------------
> inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
> ifup-eth/3288 [HC0[0]:SC1[2]:HE1:SE0] takes:
> (&(&lock->lock)->rlock){+.?...}, at: [<ffffffffa0166eef>] ip6t_do_table+0x8a/0x3f1 [ip6_tables]
> {SOFTIRQ-ON-W} state was registered at:
> [<ffffffff8107b08e>] __lock_acquire+0x756/0x93c
> [<ffffffff8107b374>] lock_acquire+0x100/0x12d
> [<ffffffff813f4ec3>] _raw_spin_lock+0x40/0x73
> [<ffffffffa01664b1>] get_counters+0xb2/0x168 [ip6_tables]
> [<ffffffffa01665a3>] alloc_counters+0x3c/0x47 [ip6_tables]
> [<ffffffffa0167a7b>] do_ip6t_get_ctl+0x10c/0x363 [ip6_tables]
> [<ffffffff813863a2>] nf_sockopt+0x5a/0x86
> [<ffffffff813863e6>] nf_getsockopt+0x18/0x1a
> [<ffffffffa034c1ff>] ipv6_getsockopt+0x84/0xba [ipv6]
> [<ffffffffa0353289>] rawv6_getsockopt+0x42/0x4b [ipv6]
> [<ffffffff81355571>] sock_common_getsockopt+0x14/0x16
> [<ffffffff813525bb>] sys_getsockopt+0x7a/0x9b
> [<ffffffff8100ad32>] system_call_fastpath+0x16/0x1b
> irq event stamp: 40
> hardirqs last enabled at (40): [<ffffffff813f5ad5>] _raw_spin_unlock_irqrestore+0x47/0x79
> hardirqs last disabled at (39): [<ffffffff813f5036>] _raw_spin_lock_irqsave+0x2b/0x92
> softirqs last enabled at (0): [<ffffffff8104975a>] copy_process+0x40e/0x11ce
> softirqs last disabled at (9): [<ffffffff8100bc9c>] call_softirq+0x1c/0x30
>
> other info that might help us debug this:
> 3 locks held by ifup-eth/3288:
> #0: (&idev->mc_ifc_timer){+.-...}, at: [<ffffffff8105841c>] run_timer_softirq+0x1f5/0x3e6
> #1: (rcu_read_lock){.+.+..}, at: [<ffffffffa03578ca>] mld_sendpack+0x0/0x3ab [ipv6]
> #2: (rcu_read_lock){.+.+..}, at: [<ffffffff81384f83>] nf_hook_slow+0x0/0x119
>
> stack backtrace:
> Pid: 3288, comm: ifup-eth Not tainted 2.6.36-rc1 #2937
> Call Trace:
> <IRQ> [<ffffffff81077ae6>] print_usage_bug+0x1a4/0x1b5
> [<ffffffff810164fa>] ? save_stack_trace+0x2f/0x4c
> [<ffffffff8106cd0c>] ? local_clock+0x40/0x59
> [<ffffffff810786b6>] ? check_usage_forwards+0x0/0xcf
> [<ffffffff81077de1>] mark_lock+0x2ea/0x51f
> [<ffffffff8107b014>] __lock_acquire+0x6dc/0x93c
> [<ffffffff8106cd0c>] ? local_clock+0x40/0x59
> [<ffffffffa0166eef>] ? ip6t_do_table+0x8a/0x3f1 [ip6_tables]
> [<ffffffff8107b374>] lock_acquire+0x100/0x12d
> [<ffffffffa0166eef>] ? ip6t_do_table+0x8a/0x3f1 [ip6_tables]
> [<ffffffff81011149>] ? sched_clock+0x9/0xd
> [<ffffffff813f4ec3>] _raw_spin_lock+0x40/0x73
> [<ffffffffa0166eef>] ? ip6t_do_table+0x8a/0x3f1 [ip6_tables]
> [<ffffffffa0166eef>] ip6t_do_table+0x8a/0x3f1 [ip6_tables]
> [<ffffffff810771db>] ? trace_hardirqs_off_caller+0x1f/0x9e
> [<ffffffff81384f83>] ? nf_hook_slow+0x0/0x119
> [<ffffffffa010601c>] ip6table_filter_hook+0x1c/0x20 [ip6table_filter]
> [<ffffffff81384f40>] nf_iterate+0x46/0x89
> [<ffffffffa035627b>] ? dst_output+0x0/0x5c [ipv6]
> [<ffffffff8138501b>] nf_hook_slow+0x98/0x119
> [<ffffffffa035627b>] ? dst_output+0x0/0x5c [ipv6]
> [<ffffffffa0349557>] ? icmp6_dst_alloc+0x0/0x1b2 [ipv6]
> [<ffffffffa0357b01>] mld_sendpack+0x237/0x3ab [ipv6]
> [<ffffffff81051475>] ? local_bh_enable_ip+0xc7/0xeb
> [<ffffffffa0358390>] mld_ifc_timer_expire+0x254/0x28d [ipv6]
> [<ffffffff810584ed>] run_timer_softirq+0x2c6/0x3e6
> [<ffffffff8105841c>] ? run_timer_softirq+0x1f5/0x3e6
> [<ffffffffa035813c>] ? mld_ifc_timer_expire+0x0/0x28d [ipv6]
> [<ffffffff8105169e>] ? __do_softirq+0x79/0x247
> [<ffffffff81051763>] __do_softirq+0x13e/0x247
> [<ffffffff8100bc9c>] call_softirq+0x1c/0x30
> [<ffffffff8100d32f>] do_softirq+0x4b/0xa3
> [<ffffffff8105120a>] irq_exit+0x4a/0x95
> [<ffffffff813fc185>] smp_apic_timer_interrupt+0x8c/0x9a
> [<ffffffff8100b753>] apic_timer_interrupt+0x13/0x20
> <EOI> [<ffffffff8102c72a>] ? native_flush_tlb_global+0x2b/0x32
> [<ffffffff81031fec>] kernel_map_pages+0x12c/0x142
> [<ffffffff810cc89a>] free_pages_prepare+0x14c/0x15d
> [<ffffffff810cc9c8>] free_hot_cold_page+0x2d/0x165
> [<ffffffff810ccb2b>] __free_pages+0x2b/0x34
> [<ffffffff810ccb7d>] free_pages+0x49/0x4e
> [<ffffffff81033871>] pgd_free+0x71/0x79
> [<ffffffff81048b1b>] __mmdrop+0x27/0x54
> [<ffffffff81042c62>] finish_task_switch+0xb4/0xe4
> [<ffffffff81042bae>] ? finish_task_switch+0x0/0xe4
> [<ffffffff810096e7>] ? __switch_to+0x1a9/0x297
> [<ffffffff81042df3>] schedule_tail+0x30/0xa7
> [<ffffffff8100ac33>] ret_from_fork+0x13/0x80
>
> I noticed in net/ipv6/netfilter/ip6_tables.c in get_counters() as with
> other "get_counters()" functions do not block bottom halves anymore as
> to this commit:
>
> commit 24b36f0193467fa727b85b4c004016a8dae999b9
> Author: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon Aug 2 16:49:01 2010 +0200
>
> netfilter: {ip,ip6,arp}_tables: dont block bottom half more than necessary
>
> We currently disable BH for the whole duration of get_counters()
>
> Now we take xt_info_wrlock(cpu) lock out of BH disabling. And that lock
> even has the following comment:
>
> /*
> * The "writer" side needs to get exclusive access to the lock,
> * regardless of readers. This must be called with bottom half
> * processing (and thus also preemption) disabled.
> */
> static inline void xt_info_wrlock(unsigned int cpu)
>
>
> As lockdep has proven, this is not satisfied.
>
> -- Steve
>
>
This is a false positive, and a patch was sent yesterday
http://patchwork.ozlabs.org/patch/61750/
Thanks
^ permalink raw reply
* Re: [PATCH 3/16] drivers/net/wireless/wl12xx: Use available error codes
From: Kalle Valo @ 2010-08-16 17:47 UTC (permalink / raw)
To: Julia Lawall
Cc: John W. Linville, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <Pine.LNX.4.64.1008161825010.19313-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org>
On 08/16/2010 07:25 PM, Julia Lawall wrote:
> From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
>
> In each case, error codes are stored in ret, but the return value is always
> 0. Return ret instead.
Thanks, looks correct to me.
Acked-by: Kalle Valo <kvalo-BkwN83ws05HQT0dZR+AlfA@public.gmane.org>
Kalle
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [LOCKDEP BUG][2.6.36-rc1] xt_info_wrlock?
From: Steven Rostedt @ 2010-08-16 17:55 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, LKML, David S. Miller, Patrick McHardy, Ingo Molnar,
Peter Zijlstra
In-Reply-To: <1281979893.2524.54.camel@edumazet-laptop>
On Mon, 2010-08-16 at 19:31 +0200, Eric Dumazet wrote:
> Le lundi 16 août 2010 à 13:07 -0400, Steven Rostedt a écrit :
> > Hi, I hit this when booting 2.6.36-rc1:
> >
> > =================================
> > [ INFO: inconsistent lock state ]
> > 2.6.36-rc1 #2937
> > ---------------------------------
> > inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
> > ifup-eth/3288 [HC0[0]:SC1[2]:HE1:SE0] takes:
> > (&(&lock->lock)->rlock){+.?...}, at: [<ffffffffa0166eef>] ip6t_do_table+0x8a/0x3f1 [ip6_tables]
> > {SOFTIRQ-ON-W} state was registered at:
> > [<ffffffff8107b08e>] __lock_acquire+0x756/0x93c
> > [<ffffffff8107b374>] lock_acquire+0x100/0x12d
> > [<ffffffff813f4ec3>] _raw_spin_lock+0x40/0x73
> > [<ffffffffa01664b1>] get_counters+0xb2/0x168 [ip6_tables]
> > [<ffffffffa01665a3>] alloc_counters+0x3c/0x47 [ip6_tables]
> > [<ffffffffa0167a7b>] do_ip6t_get_ctl+0x10c/0x363 [ip6_tables]
> > [<ffffffff813863a2>] nf_sockopt+0x5a/0x86
> > [<ffffffff813863e6>] nf_getsockopt+0x18/0x1a
> > [<ffffffffa034c1ff>] ipv6_getsockopt+0x84/0xba [ipv6]
> > [<ffffffffa0353289>] rawv6_getsockopt+0x42/0x4b [ipv6]
> > [<ffffffff81355571>] sock_common_getsockopt+0x14/0x16
> > [<ffffffff813525bb>] sys_getsockopt+0x7a/0x9b
> > [<ffffffff8100ad32>] system_call_fastpath+0x16/0x1b
> > irq event stamp: 40
> > hardirqs last enabled at (40): [<ffffffff813f5ad5>] _raw_spin_unlock_irqrestore+0x47/0x79
> > hardirqs last disabled at (39): [<ffffffff813f5036>] _raw_spin_lock_irqsave+0x2b/0x92
> > softirqs last enabled at (0): [<ffffffff8104975a>] copy_process+0x40e/0x11ce
> > softirqs last disabled at (9): [<ffffffff8100bc9c>] call_softirq+0x1c/0x30
> >
> > other info that might help us debug this:
> > 3 locks held by ifup-eth/3288:
> > #0: (&idev->mc_ifc_timer){+.-...}, at: [<ffffffff8105841c>] run_timer_softirq+0x1f5/0x3e6
> > #1: (rcu_read_lock){.+.+..}, at: [<ffffffffa03578ca>] mld_sendpack+0x0/0x3ab [ipv6]
> > #2: (rcu_read_lock){.+.+..}, at: [<ffffffff81384f83>] nf_hook_slow+0x0/0x119
> >
> > stack backtrace:
> > Pid: 3288, comm: ifup-eth Not tainted 2.6.36-rc1 #2937
> > Call Trace:
> > <IRQ> [<ffffffff81077ae6>] print_usage_bug+0x1a4/0x1b5
> > [<ffffffff810164fa>] ? save_stack_trace+0x2f/0x4c
> > [<ffffffff8106cd0c>] ? local_clock+0x40/0x59
> > [<ffffffff810786b6>] ? check_usage_forwards+0x0/0xcf
> > [<ffffffff81077de1>] mark_lock+0x2ea/0x51f
> > [<ffffffff8107b014>] __lock_acquire+0x6dc/0x93c
> > [<ffffffff8106cd0c>] ? local_clock+0x40/0x59
> > [<ffffffffa0166eef>] ? ip6t_do_table+0x8a/0x3f1 [ip6_tables]
> > [<ffffffff8107b374>] lock_acquire+0x100/0x12d
> > [<ffffffffa0166eef>] ? ip6t_do_table+0x8a/0x3f1 [ip6_tables]
> > [<ffffffff81011149>] ? sched_clock+0x9/0xd
> > [<ffffffff813f4ec3>] _raw_spin_lock+0x40/0x73
> > [<ffffffffa0166eef>] ? ip6t_do_table+0x8a/0x3f1 [ip6_tables]
> > [<ffffffffa0166eef>] ip6t_do_table+0x8a/0x3f1 [ip6_tables]
> > [<ffffffff810771db>] ? trace_hardirqs_off_caller+0x1f/0x9e
> > [<ffffffff81384f83>] ? nf_hook_slow+0x0/0x119
> > [<ffffffffa010601c>] ip6table_filter_hook+0x1c/0x20 [ip6table_filter]
> > [<ffffffff81384f40>] nf_iterate+0x46/0x89
> > [<ffffffffa035627b>] ? dst_output+0x0/0x5c [ipv6]
> > [<ffffffff8138501b>] nf_hook_slow+0x98/0x119
> > [<ffffffffa035627b>] ? dst_output+0x0/0x5c [ipv6]
> > [<ffffffffa0349557>] ? icmp6_dst_alloc+0x0/0x1b2 [ipv6]
> > [<ffffffffa0357b01>] mld_sendpack+0x237/0x3ab [ipv6]
> > [<ffffffff81051475>] ? local_bh_enable_ip+0xc7/0xeb
> > [<ffffffffa0358390>] mld_ifc_timer_expire+0x254/0x28d [ipv6]
> > [<ffffffff810584ed>] run_timer_softirq+0x2c6/0x3e6
> > [<ffffffff8105841c>] ? run_timer_softirq+0x1f5/0x3e6
> > [<ffffffffa035813c>] ? mld_ifc_timer_expire+0x0/0x28d [ipv6]
> > [<ffffffff8105169e>] ? __do_softirq+0x79/0x247
> > [<ffffffff81051763>] __do_softirq+0x13e/0x247
> > [<ffffffff8100bc9c>] call_softirq+0x1c/0x30
> > [<ffffffff8100d32f>] do_softirq+0x4b/0xa3
> > [<ffffffff8105120a>] irq_exit+0x4a/0x95
> > [<ffffffff813fc185>] smp_apic_timer_interrupt+0x8c/0x9a
> > [<ffffffff8100b753>] apic_timer_interrupt+0x13/0x20
> > <EOI> [<ffffffff8102c72a>] ? native_flush_tlb_global+0x2b/0x32
> > [<ffffffff81031fec>] kernel_map_pages+0x12c/0x142
> > [<ffffffff810cc89a>] free_pages_prepare+0x14c/0x15d
> > [<ffffffff810cc9c8>] free_hot_cold_page+0x2d/0x165
> > [<ffffffff810ccb2b>] __free_pages+0x2b/0x34
> > [<ffffffff810ccb7d>] free_pages+0x49/0x4e
> > [<ffffffff81033871>] pgd_free+0x71/0x79
> > [<ffffffff81048b1b>] __mmdrop+0x27/0x54
> > [<ffffffff81042c62>] finish_task_switch+0xb4/0xe4
> > [<ffffffff81042bae>] ? finish_task_switch+0x0/0xe4
> > [<ffffffff810096e7>] ? __switch_to+0x1a9/0x297
> > [<ffffffff81042df3>] schedule_tail+0x30/0xa7
> > [<ffffffff8100ac33>] ret_from_fork+0x13/0x80
> >
> > I noticed in net/ipv6/netfilter/ip6_tables.c in get_counters() as with
> > other "get_counters()" functions do not block bottom halves anymore as
> > to this commit:
> >
> > commit 24b36f0193467fa727b85b4c004016a8dae999b9
> > Author: Eric Dumazet <eric.dumazet@gmail.com>
> > Date: Mon Aug 2 16:49:01 2010 +0200
> >
> > netfilter: {ip,ip6,arp}_tables: dont block bottom half more than necessary
> >
> > We currently disable BH for the whole duration of get_counters()
> >
> > Now we take xt_info_wrlock(cpu) lock out of BH disabling. And that lock
> > even has the following comment:
> >
> > /*
> > * The "writer" side needs to get exclusive access to the lock,
> > * regardless of readers. This must be called with bottom half
> > * processing (and thus also preemption) disabled.
> > */
> > static inline void xt_info_wrlock(unsigned int cpu)
> >
> >
> > As lockdep has proven, this is not satisfied.
> >
> > -- Steve
> >
> >
>
>
> This is a false positive, and a patch was sent yesterday
>
> http://patchwork.ozlabs.org/patch/61750/
>
I still do not see how this is a false positive. Per-cpu locks do not
solve the issue.
Please tell me what prevents an interrupt going off after we grab the
xt_info_wrlock(cpu) in get_counters().
IOW, what prevents this:
get_counters() {
xt_info_wrlock(cpu);
<interrupt> --> softirq
xt_info_rblock_bh();
/* which grabs the writer lock */
DEADLOCK!!
-- Steve
^ permalink raw reply
* Re: [LOCKDEP BUG][2.6.36-rc1] xt_info_wrlock?
From: Steven Rostedt @ 2010-08-16 18:16 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, LKML, David S. Miller, Patrick McHardy, Ingo Molnar,
Peter Zijlstra
In-Reply-To: <1281981301.3268.110.camel@gandalf.stny.rr.com>
On Mon, 2010-08-16 at 13:55 -0400, Steven Rostedt wrote:
Re-looking at the code, I think I figured it out. But it should really
be documented better.
>
> Please tell me what prevents an interrupt going off after we grab the
> xt_info_wrlock(cpu) in get_counters().
>
> IOW, what prevents this:
>
> get_counters() {
I left out here:
for_each_possible_cpu(cpu) {
if (cpu == curcpu)
continue;
which means that we are grabbing the lock for other CPUs, and that a
softirq would not be a problem.
> xt_info_wrlock(cpu);
>
> <interrupt> --> softirq
>
> xt_info_rblock_bh();
> /* which grabs the writer lock */
> DEADLOCK!!
>
Your patchwork patch has:
@@ -729,8 +729,10 @@ static void get_counters(const struct
xt_table_info *t,
local_bh_enable();
/* Processing counters from other cpus, we can let bottom half enabled,
* (preemption is disabled)
+ * We must turn off lockdep to avoid a false positive.
*/
+ lockdep_off();
for_each_possible_cpu(cpu) {
We need a better comment than that. Could that be changed to something like:
/*
* lockdep tests if we grab a lock and can be preempted by
* a softirq and that softirq grabs the same lock causing a
* deadlock.
* This is a special case because this is a per-cpu lock,
* and we are only grabbing the lock for other CPUs. A softirq
* will only takes its local CPU lock thus, if we are preempted
* by a softirq, then it will grab the current CPU lock which
* we do not take here.
*
* Simply disable lockdep here until it can handle this situation.
*/
Thanks,
-- Steve
^ permalink raw reply
* Re: [PATCH 16/16] drivers/net/qlcnic: Use available error codes
From: Dan Carpenter @ 2010-08-16 18:35 UTC (permalink / raw)
To: Julia Lawall
Cc: Amit Kumar Salecha, Anirban Chakraborty, linux-driver, netdev,
linux-kernel, kernel-janitors
In-Reply-To: <Pine.LNX.4.64.1008161828520.19313@ask.diku.dk>
On Mon, Aug 16, 2010 at 06:29:13PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> The error code is stored in the variable err, but it is the variable ret
> that is returned instead. So store the error code in ret. Err is then
> useless.
Could you redo this for the latest git? (Sorry, I recently made the
problem slightly worse by adding another "err = -ENOMEM;")...
regards,
dan carpenter
^ permalink raw reply
* Re: [LOCKDEP BUG][2.6.36-rc1] xt_info_wrlock?
From: Peter Zijlstra @ 2010-08-16 18:36 UTC (permalink / raw)
To: Steven Rostedt
Cc: Eric Dumazet, netdev, LKML, David S. Miller, Patrick McHardy,
Ingo Molnar
In-Reply-To: <1281982566.3268.137.camel@gandalf.stny.rr.com>
On Mon, 2010-08-16 at 14:16 -0400, Steven Rostedt wrote:
> @@ -729,8 +729,10 @@ static void get_counters(const struct
> xt_table_info *t,
> local_bh_enable();
> /* Processing counters from other cpus, we can let bottom half
> enabled,
> * (preemption is disabled)
> + * We must turn off lockdep to avoid a false positive.
> */
>
> + lockdep_off();
> for_each_possible_cpu(cpu) {
>
nack!
^ permalink raw reply
* Re: [PATCH 2/2] platform: Facilitate the creation of pseudo-platform buses
From: Patrick Pannuto @ 2010-08-16 18:47 UTC (permalink / raw)
To: Grant Likely
Cc: linux-kernel, linux-arm-msm, magnus.damm, gregkh, Kevin Hilman,
Paul Mundt, Magnus Damm, Rafael J. Wysocki, Eric Miao,
Dmitry Torokhov, netdev
In-Reply-To: <AANLkTi=GduvsmkXpxsVoN7k3gPDjjRgqe0E_Z2YeEVS5@mail.gmail.com>
On 08/13/2010 03:05 PM, Grant Likely wrote:
> On Tue, Aug 10, 2010 at 5:49 PM, Patrick Pannuto
> <ppannuto@codeaurora.org> wrote:
>> (lkml.org seems to have lost August 3rd...)
>> RFC: http://lkml.indiana.edu/hypermail//linux/kernel/1008.0/01342.html
>> v1: http://lkml.indiana.edu/hypermail//linux/kernel/1008.0/01942.html
>>
>> Based on the idea and code originally proposed by Kevin Hilman:
>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html
>
> Hi Patrick,
>
> Before acking this as something that should be merged, I'd like to see
> some real device drivers converted to use this interface so I can
> better judge whether or not it is a good idea. More comments below.
>
Ok, I can do that.
>> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
>> index b69ccb4..933e0c1 100644
>> --- a/drivers/base/platform.c
>> +++ b/drivers/base/platform.c
>> @@ -238,8 +238,12 @@ int platform_device_add(struct platform_device *pdev)
>> if (!pdev)
>> return -EINVAL;
>>
>> - if (!pdev->dev.parent)
>> - pdev->dev.parent = &platform_bus;
>> + if (!pdev->dev.bus) {
>> + pdev->dev.bus = &platform_bus_type;
>> +
>> + if (!pdev->dev.parent)
>> + pdev->dev.parent = &platform_bus;
>> + }
>> pdev->dev.bus = &platform_bus_type;
>>
>
> For safety, I think you'd want to have a separate add function for
> each new platform bus type, and change the name of this function to
> explicitly pass the bus type:
>
> int __platform_device_add(struct platform_device *pdev)
> {
> if (!pdev->dev.bus)
> return -EINVAL;
> [... existing code ...]
> }
>
> int platform_device_add(struct platform_device *pdev)
> {
> if (!pdev->dev.parent)
> pdev->dev.parent = &platform_bus;
> pdev->dev.bus = &platform_bus_type;
> __platform_device_add(pdev);
> }
>
> And then for each bus type (in this example, I'll call it
> foo_bus_type, and assume foo_device wraps platform_device):
>
> int foo_device_add(struct foo_device *foo)
> {
> foo->pdev.dev.bus = &foo_bus_type;
> __platform_device_add(&foo->pdev);
> }
>
> That will allow the new bus_type code to keep foo_bus_type as an
> unexported static and will provide a bit more type safety. For
> example, it avoids accidentally registering a plain (unwrapped)
> platform device on the foo_bus_type.
>
> [...snip...]
Yes, this makes sense. Originally, I was trying to avoid this due
to a misguided notion of backwards compatibility - namely that
devices could register themselves conditionally via
.bus = HAVE_FOO_BUS,
and always call the same platform_[device|driver]_register, but
thinking about this more, such a compile (or run)-time decision can
just as easily be made in the foo_[device|driver]_register. The
impact on legacy code is the same either way, but your suggested
interface is much better.
I will modify all of these.
>
>> @@ -1017,6 +1022,87 @@ struct bus_type platform_bus_type = {
>> };
>> EXPORT_SYMBOL_GPL(platform_bus_type);
>>
>> +/** pseudo_platform_bus_register - register an "almost platform bus"
>
> Kerneldoc style error. Should be:
>
> +/**
> + * pseudo_platform_bus_register - register an "almost platform bus"
>
Fixed.
>> + * @bus: partially complete bus type to register
>> + *
>> + * This init will fill in any ommitted fields in @bus, however, it
>> + * also allocates memory and replaces the pm field in @bus, since
>> + * pm is const, but some of its pointers may need this one-time
>> + * initialziation overwrite.
>> + *
>> + * @bus's registered this way should be released with
>> + * pseudo_platform_bus_unregister
>> + */
>> +int pseudo_platform_bus_register(struct bus_type *bus)
>> +{
>> + int error;
>> + struct dev_pm_ops* heap_pm;
>
> Nit: heap_pm is an odd name. Typically Linux local vars are named
> according to what they are, not where the memory is allocated from.
> How about simply 'ops'
>
Ah yes. This was deliberate, and to draw attention to it :)
>> + heap_pm = kmalloc(sizeof (*heap_pm), GFP_KERNEL);
>> +
>> + if (!heap_pm)
>> + return -ENOMEM;
>> +
>> + if (!bus->dev_attrs)
>> + bus->dev_attrs = platform_bus_type.dev_attrs;
>> + if (!bus->match)
>> + bus->match = platform_bus_type.match;
>> + if (!bus->uevent)
>> + bus->uevent = platform_bus_type.uevent;
>> + if (!bus->pm)
>> + memcpy(heap_pm, &platform_bus_type.pm, sizeof(*heap_pm));
>> + else {
>> + heap_pm->prepare = (bus->pm->prepare) ?
>> + bus->pm->prepare : platform_pm_prepare;
>> + heap_pm->complete = (bus->pm->complete) ?
>> + bus->pm->complete : platform_pm_complete;
>> + heap_pm->suspend = (bus->pm->suspend) ?
>> + bus->pm->suspend : platform_pm_suspend;
>> + heap_pm->resume = (bus->pm->resume) ?
>> + bus->pm->resume : platform_pm_resume;
>> + heap_pm->freeze = (bus->pm->freeze) ?
>> + bus->pm->freeze : platform_pm_freeze;
>> + heap_pm->thaw = (bus->pm->thaw) ?
>> + bus->pm->thaw : platform_pm_thaw;
>> + heap_pm->poweroff = (bus->pm->poweroff) ?
>> + bus->pm->poweroff : platform_pm_poweroff;
>> + heap_pm->restore = (bus->pm->restore) ?
>> + bus->pm->restore : platform_pm_restore;
>> + heap_pm->suspend_noirq = (bus->pm->suspend_noirq) ?
>> + bus->pm->suspend_noirq : platform_pm_suspend_noirq;
>> + heap_pm->resume_noirq = (bus->pm->resume_noirq) ?
>> + bus->pm->resume_noirq : platform_pm_resume_noirq;
>> + heap_pm->freeze_noirq = (bus->pm->freeze_noirq) ?
>> + bus->pm->freeze_noirq : platform_pm_freeze_noirq;
>> + heap_pm->thaw_noirq = (bus->pm->thaw_noirq) ?
>> + bus->pm->thaw_noirq : platform_pm_thaw_noirq;
>> + heap_pm->poweroff_noirq = (bus->pm->poweroff_noirq) ?
>> + bus->pm->poweroff_noirq : platform_pm_poweroff_noirq;
>> + heap_pm->restore_noirq = (bus->pm->restore_noirq) ?
>> + bus->pm->restore_noirq : platform_pm_restore_noirq;
>> + heap_pm->runtime_suspend = (bus->pm->runtime_suspend) ?
>> + bus->pm->runtime_suspend : platform_pm_runtime_suspend;
>> + heap_pm->runtime_resume = (bus->pm->runtime_resume) ?
>> + bus->pm->runtime_resume : platform_pm_runtime_resume;
>> + heap_pm->runtime_idle = (bus->pm->runtime_idle) ?
>> + bus->pm->runtime_idle : platform_pm_runtime_idle;
>> + }
>> + bus->pm = heap_pm;
>> +
>> + error = bus_register(bus);
>> + if (error)
>> + kfree(bus->pm);
>> +
>> + return error;
>> +}
>> +EXPORT_SYMBOL_GPL(pseudo_platform_bus_register);
>
> Ick, this is a nasty list of conditional statements. :-) Instead it
> could have an unconditional initialize function which sets it up just
> like the platform bus without registering. Then allow the
> foo_bus_type initialization code override the ones it cares about, and
> then register directly.
>
No, this won't work. (Unless, every pseudo_bus_type author did this same
kludge to work around const - better to do once IMHO)
struct bus_type {
...
const struct dev_pm_ops *pm;
...
};
That const is there to *prevent* device/driver writers from overriding
pm ops on accident, and to that end, it has value. What we would really
like here is 'const after registered' semantics, where the bus author
can fill in half the structure, and the platform code can fill in the
rest. However, allowing subclasses to modify private data elements isn't
possible in C :)
I believe the 'const' here provides valuable safety. My original attempt
at doing this removed the const, and I overwrote one of the pointers in
platform_dev_pm_ops on my first try at implementing it!
This was the best solution I could come up with while preserving the const
nature of the pm_ops and introducing minimal complexity / potential to
screw up for pseudo-bus-type authors.
>> +
>> +void pseudo_platform_bus_unregister(struct bus_type *bus)
>> +{
>> + bus_unregister(bus);
>> + kfree(bus->pm);
>> +}
>> +EXPORT_SYMBOL_GPL(pseudo_platform_bus_unregister);
>> +
>> int __init platform_bus_init(void)
>> {
>> int error;
>> diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
>> index 5417944..5ec827c 100644
>> --- a/include/linux/platform_device.h
>> +++ b/include/linux/platform_device.h
>> @@ -79,6 +79,9 @@ extern int platform_driver_probe(struct platform_driver *driver,
>> #define platform_get_drvdata(_dev) dev_get_drvdata(&(_dev)->dev)
>> #define platform_set_drvdata(_dev,data) dev_set_drvdata(&(_dev)->dev, (data))
>>
>> +extern int pseudo_platform_bus_register(struct bus_type *);
>> +extern void pseudo_platform_bus_unregister(struct bus_type *);
>> +
>> extern struct platform_device *platform_create_bundle(struct platform_driver *driver,
>> int (*probe)(struct platform_device *),
>> struct resource *res, unsigned int n_res,
>> --
>> 1.7.2
>>
>>
>
>
>
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* Re: [LOCKDEP BUG][2.6.36-rc1] xt_info_wrlock?
From: Eric Dumazet @ 2010-08-16 18:48 UTC (permalink / raw)
To: Steven Rostedt
Cc: netdev, LKML, David S. Miller, Patrick McHardy, Ingo Molnar,
Peter Zijlstra
In-Reply-To: <1281982566.3268.137.camel@gandalf.stny.rr.com>
Le lundi 16 août 2010 à 14:16 -0400, Steven Rostedt a écrit :
> We need a better comment than that. Could that be changed to something like:
>
> /*
> * lockdep tests if we grab a lock and can be preempted by
> * a softirq and that softirq grabs the same lock causing a
> * deadlock.
> * This is a special case because this is a per-cpu lock,
> * and we are only grabbing the lock for other CPUs. A softirq
> * will only takes its local CPU lock thus, if we are preempted
> * by a softirq, then it will grab the current CPU lock which
> * we do not take here.
> *
> * Simply disable lockdep here until it can handle this situation.
> */
>
You mean duplicating this long comment in three files, or only once in
Changelog ?
My choice was to document the lockdep_off() use (very seldom used in
kernel) in the Changelog. Hopefully, people messing with this code know
about git ;)
I agree I didnt document how netfilter locks work in this Changelog.
And in original commit (24b36f019) I forgot to state that get_counters()
is guarded by a mutex, so that no more than one cpu runs in
get_counters().
What about following ?
[PATCH] netfilter: {ip,ip6,arp}_tables: avoid lockdep false positive
After commit 24b36f019 (netfilter: {ip,ip6,arp}_tables: dont block
bottom half more than necessary), lockdep can raise a warning
because we attempt to lock a spinlock with BH enabled, while
the same lock is usually locked by another cpu in a softirq context.
In this use case, the lockdep splat is a false positive, because
the BH disabling only matters for one cpu and its associated.
1) We use one spinlock per cpu.
2) A softirq will only lock the lock associated to current cpu.
3) get_counters() disables sofirqs while fetching data of current cpu.
(to avoid a deadlock if a softirq comes and try to lock same lock)
4) other locks are locked without blocking softirq
(as a softirq will lock another lock)
5) get_counter() calls are serialized by a mutex.
This to avoid a deadlock if two cpus were doing :
CPU1 CPU2
lock lock#1 lock lock#2
copy data#1 copy data#2
unlock lock#1 unlock lock#2
lock#2 lock#1
softirq lock#1 softirq, attempt to lock lock#2
<deadlock> <deadlock>
Use lockdep_off()/lockdep_on() around the problematic section to
avoid the splat.
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Diagnosed-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Patrick McHardy <kaber@trash.net>
---
net/ipv4/netfilter/arp_tables.c | 3 +++
net/ipv4/netfilter/ip_tables.c | 3 +++
net/ipv6/netfilter/ip6_tables.c | 3 +++
3 files changed, 9 insertions(+)
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index 6bccba3..b4f7ebf 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -729,8 +729,10 @@ static void get_counters(const struct xt_table_info *t,
local_bh_enable();
/* Processing counters from other cpus, we can let bottom half enabled,
* (preemption is disabled)
+ * We must turn off lockdep to avoid a false positive.
*/
+ lockdep_off();
for_each_possible_cpu(cpu) {
if (cpu == curcpu)
continue;
@@ -743,6 +745,7 @@ static void get_counters(const struct xt_table_info *t,
}
xt_info_wrunlock(cpu);
}
+ lockdep_on();
put_cpu();
}
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index c439721..dc5b2fd 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -903,8 +903,10 @@ get_counters(const struct xt_table_info *t,
local_bh_enable();
/* Processing counters from other cpus, we can let bottom half enabled,
* (preemption is disabled)
+ * We must turn off lockdep to avoid a false positive.
*/
+ lockdep_off();
for_each_possible_cpu(cpu) {
if (cpu == curcpu)
continue;
@@ -917,6 +919,7 @@ get_counters(const struct xt_table_info *t,
}
xt_info_wrunlock(cpu);
}
+ lockdep_on();
put_cpu();
}
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 5359ef4..fb55443 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -916,8 +916,10 @@ get_counters(const struct xt_table_info *t,
local_bh_enable();
/* Processing counters from other cpus, we can let bottom half enabled,
* (preemption is disabled)
+ * We must turn off lockdep to avoid a false positive.
*/
+ lockdep_off();
for_each_possible_cpu(cpu) {
if (cpu == curcpu)
continue;
@@ -930,6 +932,7 @@ get_counters(const struct xt_table_info *t,
}
xt_info_wrunlock(cpu);
}
+ lockdep_on();
put_cpu();
}
^ permalink raw reply related
* Re: [LOCKDEP BUG][2.6.36-rc1] xt_info_wrlock?
From: Eric Dumazet @ 2010-08-16 18:48 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Steven Rostedt, netdev, LKML, David S. Miller, Patrick McHardy,
Ingo Molnar
In-Reply-To: <1281983814.1926.1763.camel@laptop>
Le lundi 16 août 2010 à 20:36 +0200, Peter Zijlstra a écrit :
> On Mon, 2010-08-16 at 14:16 -0400, Steven Rostedt wrote:
> > @@ -729,8 +729,10 @@ static void get_counters(const struct
> > xt_table_info *t,
> > local_bh_enable();
> > /* Processing counters from other cpus, we can let bottom half
> > enabled,
> > * (preemption is disabled)
> > + * We must turn off lockdep to avoid a false positive.
> > */
> >
> > + lockdep_off();
> > for_each_possible_cpu(cpu) {
> >
> nack!
Interesting.
Care to elaborate ?
^ permalink raw reply
* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: Richard Cochran @ 2010-08-16 19:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: netdev, linux-kernel, linuxppc-dev, devicetree-discuss,
linux-arm-kernel, Krzysztof Halasa, Rodolfo Giometti
In-Reply-To: <201008161626.24083.arnd@arndb.de>
On Mon, Aug 16, 2010 at 04:26:23PM +0200, Arnd Bergmann wrote:
> Have you considered integrating the subsystem into the Posix clock/timer
> framework?
Yes, but see below.
> I can't really tell from reading the source if this is possible or
> not, but my feeling is that if it can be done, that would be a much
> nicer interface. We already have clock_gettime/clock_settime/
> timer_settime/... system calls, and while you'd need to add another
> clockid and some syscalls, my feeling is that it will be more
> usable in the end.
You are not the first person to ask about this. See this link for
longer explanation of why I did not go that way:
http://marc.info/?l=linux-netdev&m=127669810232201&w=2
You *could* offer the PTP clock as a Linux clock source/event device,
and I agree that it would be nicer. However, the problem is, what do
you do with the PHY based clocks? Just one 16 bit read from a PHY
clock can take 40 usec, and you need four such read operations just to
get the current time value.
Also, I really did not want to add or change any syscalls. I could not
see a practical way to extend the existing syscalls to accommodate PTP
clocks.
Richard
^ permalink raw reply
* Re: [PATCH 16/16] drivers/net/qlcnic: Use available error codes
From: Julia Lawall @ 2010-08-16 19:05 UTC (permalink / raw)
To: Dan Carpenter
Cc: Amit Kumar Salecha, Anirban Chakraborty, linux-driver, netdev,
linux-kernel, kernel-janitors
In-Reply-To: <20100816183540.GE645@bicker>
On Mon, 16 Aug 2010, Dan Carpenter wrote:
> On Mon, Aug 16, 2010 at 06:29:13PM +0200, Julia Lawall wrote:
> > From: Julia Lawall <julia@diku.dk>
> >
> > The error code is stored in the variable err, but it is the variable ret
> > that is returned instead. So store the error code in ret. Err is then
> > useless.
>
> Could you redo this for the latest git? (Sorry, I recently made the
> problem slightly worse by adding another "err = -ENOMEM;")...
Sure. I'll send it shortly.
julia
^ permalink raw reply
* Re: [LOCKDEP BUG][2.6.36-rc1] xt_info_wrlock?
From: Peter Zijlstra @ 2010-08-16 19:16 UTC (permalink / raw)
To: Eric Dumazet
Cc: Steven Rostedt, netdev, LKML, David S. Miller, Patrick McHardy,
Ingo Molnar
In-Reply-To: <1281984528.2487.25.camel@edumazet-laptop>
On Mon, 2010-08-16 at 20:48 +0200, Eric Dumazet wrote:
> Le lundi 16 août 2010 à 20:36 +0200, Peter Zijlstra a écrit :
> > On Mon, 2010-08-16 at 14:16 -0400, Steven Rostedt wrote:
> > > @@ -729,8 +729,10 @@ static void get_counters(const struct
> > > xt_table_info *t,
> > > local_bh_enable();
> > > /* Processing counters from other cpus, we can let bottom half
> > > enabled,
> > > * (preemption is disabled)
> > > + * We must turn off lockdep to avoid a false positive.
> > > */
> > >
> > > + lockdep_off();
> > > for_each_possible_cpu(cpu) {
> > >
> > nack!
>
>
> Interesting.
>
> Care to elaborate ?
Adding lockdep_off() is just plain wrong, if you cannot describe the
locking there's a fair chance its wrong anyway.
As it stands there's only a single lockdep_off(), and that lives in NTFS
it looks like it could be annotated differently, but then, who cares
about NTFS anyway ;-)
^ permalink raw reply
* Re: [PATCH 8/16] net/ax25: Use available error codes
From: Ralf Baechle @ 2010-08-16 19:23 UTC (permalink / raw)
To: Julia Lawall
Cc: David S. Miller, linux-hams, linux-kernel, netdev,
kernel-janitors
In-Reply-To: <Pine.LNX.4.64.1008161826390.19313@ask.diku.dk>
On Mon, Aug 16, 2010 at 06:26:57PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> Error codes are stored in err, but the return value is always 0. Return
> err instead.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @r@
> local idexpression x;
> constant C;
> @@
>
> if (...) { ...
> x = -C
> ... when != x
> (
> return <+...x...+>;
> |
> return NULL;
> |
> return;
> |
> * return ...;
> )
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
> This changes the semantics and has not been tested.
It restores the semantics to what they should be and used to be. This
probably happened during the introduction of proper refcounting and
locking into the AX.25 stack. Looks fine to me.
Acked-by: Ralf Baechle <ralf@linux-mips.org>
Ralf
^ permalink raw reply
* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: john stultz @ 2010-08-16 19:24 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, linux-kernel, linuxppc-dev, devicetree-discuss,
linux-arm-kernel, Krzysztof Halasa, Rodolfo Giometti
In-Reply-To: <363bd749a38d0b785d8431e591bf54c38db4c2d7.1281956490.git.richard.cochran@omicron.at>
On Mon, Aug 16, 2010 at 4:17 AM, Richard Cochran
<richardcochran@gmail.com> wrote:
> This patch adds an infrastructure for hardware clocks that implement
> IEEE 1588, the Precision Time Protocol (PTP). A class driver offers a
> registration method to particular hardware clock drivers. Each clock is
> exposed to user space as a character device with ioctls that allow tuning
> of the PTP clock.
>
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
Hey Richard!
Its very cool to see this work on lkml! I'm excited to see more
work done on ptp. We had a short private thread discussion earlier (I
got busy and never replied to your last message, my apologies!), but I
wanted to bring up the concerns I have here as well.
A few comments below....
> +** PTP user space API
> +
> + The class driver creates a character device for each registered PTP
> + clock. User space programs may control the clock using standardized
> + ioctls. A program may query, enable, configure, and disable the
> + ancillary clock features. User space can receive time stamped
> + events via blocking read() and poll(). One shot and periodic
> + signals may be configured via an ioctl API with semantics similar
> + to the POSIX timer_settime() system call.
As I mentioned earlier, I'm not a huge fan of the char device
interface for abstracted PTP clocks.
If it was just the direct hardware access, similar to RTC, which user
apps then use as a timesource, I'd not have much of a problem. But as
I mentioned in an earlier private mail, the abstraction level concerns
me.
1) The driver-like model exposes a char dev for each clock, which
allows for poorly-written userland applications to hit portability
issues (ie: /dev/hpet vs /dev/rtc). Granted this isn't a huge flaw,
but good APIs should be hard to get wrong.
2) As Arnd already mentioned, the chardev interface seems to duplicate
the clock_gettime/settime() and adjtimex() interfaces.
3) I'm not sure I see the benefit of being able to have multiple
frequency corrected time domains. In other words, what benefit would
you get from adjusting a PTP clock's frequency instead of just
adjusting the system's time freq? Having the PTP time as a reference
to correct the system time seems reasonable, but I'm not sure I see
why userland would want to adjust the PTP clock's freq.
thanks
-john
^ permalink raw reply
* Re: [PATCH 13/16] net/ax25: Use available error codes
From: Ralf Baechle @ 2010-08-16 19:31 UTC (permalink / raw)
To: Julia Lawall
Cc: Joerg Reuter, David S. Miller, linux-hams, linux-kernel, netdev,
kernel-janitors
In-Reply-To: <Pine.LNX.4.64.1008161828030.19313@ask.diku.dk>
On Mon, Aug 16, 2010 at 06:28:19PM +0200, Julia Lawall wrote:
> Error codes are stored in err, but the return value is always 0. Return
> err instead.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @r@
> local idexpression x;
> constant C;
> @@
>
> if (...) { ...
> x = -C
> ... when != x
> (
> return <+...x...+>;
> |
> return NULL;
> |
> return;
> |
> * return ...;
> )
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
> This changes the semantics of the function and has not been tested.
It restores the semantics to what they should be and used to be. This
probably happened during the introduction of proper refcounting and
locking into the AX.25 stack. Looks fine to me.
Acked-by: Ralf Baechle <ralf@linux-mips.org>
Ralf
^ permalink raw reply
* Re: [LOCKDEP BUG][2.6.36-rc1] xt_info_wrlock?
From: Eric Dumazet @ 2010-08-16 19:35 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Steven Rostedt, netdev, LKML, David S. Miller, Patrick McHardy,
Ingo Molnar
In-Reply-To: <1281986177.1926.1858.camel@laptop>
Le lundi 16 août 2010 à 21:16 +0200, Peter Zijlstra a écrit :
> Adding lockdep_off() is just plain wrong, if you cannot describe the
> locking there's a fair chance its wrong anyway.
>
I see.
I described the fine locking after Steven comment, adding a long
Changelog.
http://patchwork.ozlabs.org/patch/61827/
If someone thinks this locking is buggy, please speak now ;)
^ permalink raw reply
* Re: [GIT] Networking
From: David Miller @ 2010-08-16 19:36 UTC (permalink / raw)
To: eric.dumazet; +Cc: torvalds, akpm, netdev, linux-kernel, kaber
In-Reply-To: <1281883637.2942.42.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 15 Aug 2010 16:47:17 +0200
> Le dimanche 15 août 2010 à 12:55 +0200, Eric Dumazet a écrit :
>
>> We have one lock per cpu, and only one cpu can possibly lock its
>> associated lock under softirq. So the usual lockdep check, warning a
>> lock is taken with BH enabled, while same lock was taken inside softirq
>> handler is triggering a false positive here.
>>
>> I believe no existing lockdep annotation can instruct lockdep this use
>> is OK, I guess we have following choice :
>>
>> 1) Mask BH again, using xt_info_wrlock_lockdep(cpu) instead of
>> xt_info_wrlock(cpu).
>>
>> xt_info_wrlock_lockdep() being a variant, that disables BH in case
>> CONFIG_PROVE_LOCKING=y
>>
>> 2) temporally switch off lockdep in get_counters(), using a
>> lockdep_off()/lockdep_on() pair, and a comment why this is necessary.
>>
>
> In any case, here is patch implementing the later
I'm hesistent to say that we should put this kind of patch in.
It will shut up lockdep for this specific case, but it also means
that if we do any other kinds of locking in this sequence we will
not validate it.
The valuable of this is open for debate I guess.
But locking is hard so I would say that disabling lockdep to kill a
warning it generates should be an absolute last resort.
I also don't think making the locking mechanics conditional upon
LOCKDEP is sane either, exactly because it means lockdep is testing
something other than what actually gets used in practice. :-)
^ permalink raw reply
* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: john stultz @ 2010-08-16 19:38 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, linux-kernel, linuxppc-dev, devicetree-discuss,
linux-arm-kernel, Krzysztof Halasa, Rodolfo Giometti
In-Reply-To: <AANLkTik_2MKMhOuDGOmu8Kzyq-ipLe+Bxrb3FaD+Tv4U@mail.gmail.com>
On Mon, Aug 16, 2010 at 12:24 PM, john stultz <johnstul@us.ibm.com> wrote:
> On Mon, Aug 16, 2010 at 4:17 AM, Richard Cochran
> A few comments below....
>
>> +** PTP user space API
>> +
>> + The class driver creates a character device for each registered PTP
>> + clock. User space programs may control the clock using standardized
>> + ioctls. A program may query, enable, configure, and disable the
>> + ancillary clock features. User space can receive time stamped
>> + events via blocking read() and poll(). One shot and periodic
>> + signals may be configured via an ioctl API with semantics similar
>> + to the POSIX timer_settime() system call.
>
> As I mentioned earlier, I'm not a huge fan of the char device
> interface for abstracted PTP clocks.
> If it was just the direct hardware access, similar to RTC, which user
> apps then use as a timesource, I'd not have much of a problem. But as
> I mentioned in an earlier private mail, the abstraction level concerns
> me.
[snip]
> 2) As Arnd already mentioned, the chardev interface seems to duplicate
> the clock_gettime/settime() and adjtimex() interfaces.
And maybe just to clarify, as I saw your response to Arnd, I'm not
suggesting using PTP clocks as clocksources for the internal
timekeeping core. Instead I'm trying to understand why PTP clocks need
the equivalent of the existing posix clocks/timer interface. Why would
only having a read-time interface not suffice?
thanks
-john
^ permalink raw reply
* Re: [LOCKDEP BUG][2.6.36-rc1] xt_info_wrlock?
From: David Miller @ 2010-08-16 19:44 UTC (permalink / raw)
To: rostedt; +Cc: eric.dumazet, netdev, linux-kernel, kaber, mingo, peterz
In-Reply-To: <1281981301.3268.110.camel@gandalf.stny.rr.com>
From: Steven Rostedt <rostedt@goodmis.org>
Date: Mon, 16 Aug 2010 13:55:01 -0400
> Please tell me what prevents an interrupt going off after we grab the
> xt_info_wrlock(cpu) in get_counters().
He's only accessing the per-cpu counter locks of other cpus.
The per-cpu lock is only locally accessed by a cpu in software
interrupt context.
That is why his transformation is legal.
Lockdep simply hasn't been informed of this invariant and has
to assume the worst.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox