* [PATCH] SUNRPC: prevent task_cleanup running on freed xprt
From: J. Bruce Fields @ 2010-08-03 21:22 UTC (permalink / raw)
To: Trond Myklebust
Cc: Jeff Layton, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1280861363.12283.2.camel-rJ7iovZKK19ZJLDQqaL3InhyD016LWXt@public.gmane.org>
From: J. Bruce Fields <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
We saw a report of a NULL dereference in xprt_autoclose:
https://bugzilla.redhat.com/show_bug.cgi?id=611938
This appears to be the result of an xprt's task_cleanup running after
the xprt is destroyed. Nothing in the current code appears to prevent
that.
Signed-off-by: J. Bruce Fields <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
net/sunrpc/xprt.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
On Tue, Aug 03, 2010 at 02:49:23PM -0400, Trond Myklebust wrote:
> Hmm... On the other hand, since xprt_destroy() may get called from
> inside the rpciod_workqueue, the wait_on_bit_lock() may end up
> deadlocking.
>
> Nevermind, then. Let's go with the cancel_work_sync().
Here you are--only if you don't have something equivalent already queued
up.--b.
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index dcd0132..2a1f664 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -1129,6 +1129,7 @@ static void xprt_destroy(struct kref *kref)
rpc_destroy_wait_queue(&xprt->sending);
rpc_destroy_wait_queue(&xprt->resend);
rpc_destroy_wait_queue(&xprt->backlog);
+ cancel_work_sync(&xprt->task_cleanup);
/*
* Tear down transport state and free the rpc_xprt
*/
--
1.7.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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 related
* Re: [Bugme-new] [Bug 16503] New: Upgrade to 2.6.34.2 breaks ethernet network
From: Andrew Morton @ 2010-08-03 20:27 UTC (permalink / raw)
To: netdev; +Cc: bugzilla-daemon, bugme-daemon, Gary Zambrano, stable,
davemorgan353
In-Reply-To: <bug-16503-10286@https.bugzilla.kernel.org/>
(switched to email. Please respond via emailed reply-to-all, not via the
bugzilla web interface).
On Tue, 3 Aug 2010 20:14:34 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=16503
>
> Summary: Upgrade to 2.6.34.2 breaks ethernet network
> Product: Networking
> Version: 2.5
> Platform: All
> OS/Version: Linux
> Tree: Mainline
> Status: NEW
> Severity: normal
> Priority: P1
> Component: Other
> AssignedTo: acme@ghostprotocols.net
> ReportedBy: davemorgan353@btinternet.com
> Regression: No
>
>
> Using Arch Linux with fully up-to-date system.
>
> Upgrading kernel from 2.6.34.1 to 2.6.34.2 breaks my ethernet connection. I
> get dhcp timeouts but no errors in the logs regarding the kernel or the card.
>
> Dell Inspiron 1720.
>
> 03:00.0 Ethernet controller: Broadcom Corporation BCM4401-B0 100Base-TX (rev
> 02)
>
> Downgrading the kernel fixes the issue.
>
There are no changes to b44.c in patch-2.6.34.2. Perhaps you could
generate the dmesg logs for 2.6.34.1 and 2.6.34.2 and run `diff -u'
against them, see if anything interesting pops out?
^ permalink raw reply
* [PATCH net-next] drivers/net/enic: Use %pUB to format a UUID
From: Joe Perches @ 2010-08-03 19:32 UTC (permalink / raw)
To: Scott Feldman; +Cc: netdev, linux-kernel
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/enic/enic_main.c | 17 ++---------------
1 files changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 77a7f87..9aab853 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -1087,10 +1087,7 @@ static int enic_set_port_profile(struct enic *enic, u8 *mac)
{
struct vic_provinfo *vp;
u8 oui[3] = VIC_PROVINFO_CISCO_OUI;
- u8 *uuid;
char uuid_str[38];
- static char *uuid_fmt = "%02X%02X%02X%02X-%02X%02X-%02X%02X-"
- "%02X%02X-%02X%02X%02X%02X%0X%02X";
int err;
err = enic_vnic_dev_deinit(enic);
@@ -1121,24 +1118,14 @@ static int enic_set_port_profile(struct enic *enic, u8 *mac)
ETH_ALEN, mac);
if (enic->pp.set & ENIC_SET_INSTANCE) {
- uuid = enic->pp.instance_uuid;
- sprintf(uuid_str, uuid_fmt,
- uuid[0], uuid[1], uuid[2], uuid[3],
- uuid[4], uuid[5], uuid[6], uuid[7],
- uuid[8], uuid[9], uuid[10], uuid[11],
- uuid[12], uuid[13], uuid[14], uuid[15]);
+ sprintf(uuid_str, "%pUB", enic->pp.instance_uuid);
vic_provinfo_add_tlv(vp,
VIC_LINUX_PROV_TLV_CLIENT_UUID_STR,
sizeof(uuid_str), uuid_str);
}
if (enic->pp.set & ENIC_SET_HOST) {
- uuid = enic->pp.host_uuid;
- sprintf(uuid_str, uuid_fmt,
- uuid[0], uuid[1], uuid[2], uuid[3],
- uuid[4], uuid[5], uuid[6], uuid[7],
- uuid[8], uuid[9], uuid[10], uuid[11],
- uuid[12], uuid[13], uuid[14], uuid[15]);
+ sprintf(uuid_str, "%pUB", enic->pp.host_uuid);
vic_provinfo_add_tlv(vp,
VIC_LINUX_PROV_TLV_HOST_UUID_STR,
sizeof(uuid_str), uuid_str);
^ permalink raw reply related
* Re: sunrpc: what prevents an xprt from being freed before task_cleanup runs?
From: J. Bruce Fields @ 2010-08-03 19:18 UTC (permalink / raw)
To: Trond Myklebust; +Cc: Jeff Layton, linux-nfs, netdev
In-Reply-To: <1280861775.12283.7.camel@heimdal.trondhjem.org>
On Tue, Aug 03, 2010 at 02:56:15PM -0400, Trond Myklebust wrote:
> On Tue, 2010-08-03 at 14:48 -0400, J. Bruce Fields wrote:
> > On Tue, Aug 03, 2010 at 02:40:15PM -0400, Trond Myklebust wrote:
> > > On Tue, 2010-08-03 at 13:24 -0400, Jeff Layton wrote:
> > > > We got a report recently about a panic in RHEL5 (2.6.18 based kernel).
> > > > The problem appears to be that a task_cleanup workqueue job ran and got
> > > > passed a pointer to an xprt that had been freed. The bug is here in
> > > > case anyone is interested in the details:
> > > >
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=611938
> > > >
> > > > The situation seems to be pretty difficult to reproduce, but I don't
> > > > see anything that's intended to ensure that this doesn't occur in RHEL5
> > > > or mainline. The task_cleanup workqueue job doesn't hold a reference to
> > > > the xprt, and the job isn't canceled when the xprt is torn down.
> > > >
> > > > Bruce had a look and suggested that we may need something like the
> > > > patch below (pasted in, so it probably won't apply correctly). I've
> > > > tested a backported version of it on RHEL5 and it seems to work fine.
> > > >
> > > > Is it reasonable to cancel task_cleanup when destroying the xprt? Or,
> > > > am I missing something that should prevent this situation in mainline
> > > > (and perhaps isn't in RHEL5's kernel).
> > > >
> > > > Any help is appreciated...
> > > >
> > > > -----------------------------[snip]---------------------------------
> > > > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> > > > index dcd0132..2a1f664 100644
> > > > --- a/net/sunrpc/xprt.c
> > > > +++ b/net/sunrpc/xprt.c
> > > > @@ -1129,6 +1129,7 @@ static void xprt_destroy(struct kref *kref)
> > > > rpc_destroy_wait_queue(&xprt->sending);
> > > > rpc_destroy_wait_queue(&xprt->resend);
> > > > rpc_destroy_wait_queue(&xprt->backlog);
> > > > + cancel_work_sync(&xprt->task_cleanup);
> > > > /*
> > > > * Tear down transport state and free the rpc_xprt
> > > > */
> > > > -----------------------------[snip]---------------------------------
> > > >
> > > > Thanks,
> > >
> > > How about doing a wait_on_bit_lock(&xprt->state, XPRT_LOCKED,...)
> > > instead?
> >
> > I'll believe you, but I don't get it: what guarantees that XPRT_LOCKED
> > isn't cleared while xprt_autoclose autoclose is still running?
>
> Look more closely: the callers that enqueue task_cleanup on the
> workqueue all ensure that XPRT_LOCKED is held, and it isn't released
> until the call to xprt_release_write() at the very end of
> xprt_autoclose().
I see that (assuming snd_task is always NULL--but, OK, I think I see why
that's true), but we still dereference the xprt after returning from
xprt_release_write(). Only to drop the transport_lock, but I don't see
why there isn't a race.
> The rpciod_workqueue deadlocking is an issue, though, so unfortunately,
> we won't be able to make use of the above.
Yeah, OK, it was just bugging me that I didn't feel like I understood
how XPRT_LOCKED is used.
--b.
^ permalink raw reply
* Re: softirq warnings when calling dev_kfree_skb_irq - bug in conntrack?
From: Jeremy Fitzhardinge @ 2010-08-03 19:16 UTC (permalink / raw)
To: David Miller
Cc: johannes, netdev, dongxiao.xu, Xen-devel, Ian.Campbell, kaber,
eric.dumazet
In-Reply-To: <20100803.002318.209965574.davem@davemloft.net>
On 08/03/2010 12:23 AM, David Miller wrote:
> From: Johannes Berg<johannes@sipsolutions.net>
> Date: Tue, 03 Aug 2010 09:04:34 +0200
>
>> I had this too:
>> http://article.gmane.org/gmane.linux.network/167590
>>
>> But I'm not convinced it's conntrack, I'd think it's
>>
>> commit 15e83ed78864d0625e87a85f09b297c0919a4797
>> Author: Eric Dumazet<eric.dumazet@gmail.com>
>> Date: Wed May 19 23:16:03 2010 +0000
>>
>> net: remove zap_completion_queue
>>
>> which, from the looks of it, ought to be reverted because it failed to
>> take into account that dev_kfree_skb() can do more things that require
>> non-irq-context than just calling skb->destructor, like for instance the
>> conntrack thing we see here.
> Agreed. I'll revert this and queue that up for 2.6.35-stable
Reverting this change fixes the problem for me.
Thanks,
J
^ permalink raw reply
* Vaše Mail Box kvóty překročí stanovený limit
From: Správce systému @ 2010-08-03 18:52 UTC (permalink / raw)
Vaše poštovní schránka překročila limit úložiště, které je 20GB,
jak nastavit váš správce, máte v současné době běží na
+20.9gigabajt,
Možná nebudete moci odesílat nebo přijímat novou poštu, dokud
re-ověření Vaší schránky.
Chcete-li znovu potvrdili svou poštovní schránku, klikněte prosím na
níže uvedený odkaz:
http://w3t.org/CJ6
Pokud výše uvedený odkaz nefunguje, prosím zkopírujte a vložte níže
uvedený odkaz okně prohlížeče
http://w3t.org/CJ6
Díky
Správce systému
^ permalink raw reply
* Re: sunrpc: what prevents an xprt from being freed before task_cleanup runs?
From: Jeff Layton @ 2010-08-03 18:59 UTC (permalink / raw)
To: Trond Myklebust; +Cc: linux-nfs, netdev, bfields
In-Reply-To: <1280860815.9771.25.camel@heimdal.trondhjem.org>
On Tue, 03 Aug 2010 14:40:15 -0400
Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
> On Tue, 2010-08-03 at 13:24 -0400, Jeff Layton wrote:
> > We got a report recently about a panic in RHEL5 (2.6.18 based kernel).
> > The problem appears to be that a task_cleanup workqueue job ran and got
> > passed a pointer to an xprt that had been freed. The bug is here in
> > case anyone is interested in the details:
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=611938
> >
> > The situation seems to be pretty difficult to reproduce, but I don't
> > see anything that's intended to ensure that this doesn't occur in RHEL5
> > or mainline. The task_cleanup workqueue job doesn't hold a reference to
> > the xprt, and the job isn't canceled when the xprt is torn down.
> >
> > Bruce had a look and suggested that we may need something like the
> > patch below (pasted in, so it probably won't apply correctly). I've
> > tested a backported version of it on RHEL5 and it seems to work fine.
> >
> > Is it reasonable to cancel task_cleanup when destroying the xprt? Or,
> > am I missing something that should prevent this situation in mainline
> > (and perhaps isn't in RHEL5's kernel).
> >
> > Any help is appreciated...
> >
> > -----------------------------[snip]---------------------------------
> > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> > index dcd0132..2a1f664 100644
> > --- a/net/sunrpc/xprt.c
> > +++ b/net/sunrpc/xprt.c
> > @@ -1129,6 +1129,7 @@ static void xprt_destroy(struct kref *kref)
> > rpc_destroy_wait_queue(&xprt->sending);
> > rpc_destroy_wait_queue(&xprt->resend);
> > rpc_destroy_wait_queue(&xprt->backlog);
> > + cancel_work_sync(&xprt->task_cleanup);
> > /*
> > * Tear down transport state and free the rpc_xprt
> > */
> > -----------------------------[snip]---------------------------------
> >
> > Thanks,
>
> How about doing a wait_on_bit_lock(&xprt->state, XPRT_LOCKED,...)
> instead?
>
I haven't quite grokked what XPRT_LOCKED is supposed to signify. What
would be the advantage of doing that over just killing off the
task_cleanup job? It seems like if we're tearing down the xprt then
waiting for task_cleanup to run is sort of pointless -- xs_destroy will
take care of closing the socket anyway.
Also, if we do go that route, do we need a wake_up_bit call in
xprt_clear_locked?
Thanks,
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply
* Re: sunrpc: what prevents an xprt from being freed before task_cleanup runs?
From: Trond Myklebust @ 2010-08-03 18:56 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Jeff Layton, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20100803184856.GA31579-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
On Tue, 2010-08-03 at 14:48 -0400, J. Bruce Fields wrote:
> On Tue, Aug 03, 2010 at 02:40:15PM -0400, Trond Myklebust wrote:
> > On Tue, 2010-08-03 at 13:24 -0400, Jeff Layton wrote:
> > > We got a report recently about a panic in RHEL5 (2.6.18 based kernel).
> > > The problem appears to be that a task_cleanup workqueue job ran and got
> > > passed a pointer to an xprt that had been freed. The bug is here in
> > > case anyone is interested in the details:
> > >
> > > https://bugzilla.redhat.com/show_bug.cgi?id=611938
> > >
> > > The situation seems to be pretty difficult to reproduce, but I don't
> > > see anything that's intended to ensure that this doesn't occur in RHEL5
> > > or mainline. The task_cleanup workqueue job doesn't hold a reference to
> > > the xprt, and the job isn't canceled when the xprt is torn down.
> > >
> > > Bruce had a look and suggested that we may need something like the
> > > patch below (pasted in, so it probably won't apply correctly). I've
> > > tested a backported version of it on RHEL5 and it seems to work fine.
> > >
> > > Is it reasonable to cancel task_cleanup when destroying the xprt? Or,
> > > am I missing something that should prevent this situation in mainline
> > > (and perhaps isn't in RHEL5's kernel).
> > >
> > > Any help is appreciated...
> > >
> > > -----------------------------[snip]---------------------------------
> > > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> > > index dcd0132..2a1f664 100644
> > > --- a/net/sunrpc/xprt.c
> > > +++ b/net/sunrpc/xprt.c
> > > @@ -1129,6 +1129,7 @@ static void xprt_destroy(struct kref *kref)
> > > rpc_destroy_wait_queue(&xprt->sending);
> > > rpc_destroy_wait_queue(&xprt->resend);
> > > rpc_destroy_wait_queue(&xprt->backlog);
> > > + cancel_work_sync(&xprt->task_cleanup);
> > > /*
> > > * Tear down transport state and free the rpc_xprt
> > > */
> > > -----------------------------[snip]---------------------------------
> > >
> > > Thanks,
> >
> > How about doing a wait_on_bit_lock(&xprt->state, XPRT_LOCKED,...)
> > instead?
>
> I'll believe you, but I don't get it: what guarantees that XPRT_LOCKED
> isn't cleared while xprt_autoclose autoclose is still running?
Look more closely: the callers that enqueue task_cleanup on the
workqueue all ensure that XPRT_LOCKED is held, and it isn't released
until the call to xprt_release_write() at the very end of
xprt_autoclose().
The rpciod_workqueue deadlocking is an issue, though, so unfortunately,
we won't be able to make use of the above.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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
* [PATCH] davinci_emac: Fix use after free in davinci_emac_remove
From: Stefan Weil @ 2010-08-03 18:53 UTC (permalink / raw)
To: netdev
Cc: Stefan Weil, David S. Miller, Chaithrika U S, Sriramakrishnan,
Kevin Hilman, linux-kernel
free_netdev finally calls kfree which makes the contents
of ndev and priv (private data contained in ndev) invalid.
So iounmap should be called before free_netdev.
Cc: David S. Miller <davem@davemloft.net>
Cc: Chaithrika U S <chaithrika@ti.com>
Cc: Sriramakrishnan <srk@ti.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
drivers/net/davinci_emac.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
index 8cc8a43..866e6b8 100644
--- a/drivers/net/davinci_emac.c
+++ b/drivers/net/davinci_emac.c
@@ -2818,8 +2818,8 @@ static int __devexit davinci_emac_remove(struct platform_device *pdev)
release_mem_region(res->start, res->end - res->start + 1);
unregister_netdev(ndev);
- free_netdev(ndev);
iounmap(priv->remap_addr);
+ free_netdev(ndev);
clk_disable(emac_clk);
clk_put(emac_clk);
--
1.5.6.5
^ permalink raw reply related
* Re: sunrpc: what prevents an xprt from being freed before task_cleanup runs?
From: J. Bruce Fields @ 2010-08-03 18:48 UTC (permalink / raw)
To: Trond Myklebust; +Cc: Jeff Layton, linux-nfs, netdev
In-Reply-To: <1280860815.9771.25.camel@heimdal.trondhjem.org>
On Tue, Aug 03, 2010 at 02:40:15PM -0400, Trond Myklebust wrote:
> On Tue, 2010-08-03 at 13:24 -0400, Jeff Layton wrote:
> > We got a report recently about a panic in RHEL5 (2.6.18 based kernel).
> > The problem appears to be that a task_cleanup workqueue job ran and got
> > passed a pointer to an xprt that had been freed. The bug is here in
> > case anyone is interested in the details:
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=611938
> >
> > The situation seems to be pretty difficult to reproduce, but I don't
> > see anything that's intended to ensure that this doesn't occur in RHEL5
> > or mainline. The task_cleanup workqueue job doesn't hold a reference to
> > the xprt, and the job isn't canceled when the xprt is torn down.
> >
> > Bruce had a look and suggested that we may need something like the
> > patch below (pasted in, so it probably won't apply correctly). I've
> > tested a backported version of it on RHEL5 and it seems to work fine.
> >
> > Is it reasonable to cancel task_cleanup when destroying the xprt? Or,
> > am I missing something that should prevent this situation in mainline
> > (and perhaps isn't in RHEL5's kernel).
> >
> > Any help is appreciated...
> >
> > -----------------------------[snip]---------------------------------
> > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> > index dcd0132..2a1f664 100644
> > --- a/net/sunrpc/xprt.c
> > +++ b/net/sunrpc/xprt.c
> > @@ -1129,6 +1129,7 @@ static void xprt_destroy(struct kref *kref)
> > rpc_destroy_wait_queue(&xprt->sending);
> > rpc_destroy_wait_queue(&xprt->resend);
> > rpc_destroy_wait_queue(&xprt->backlog);
> > + cancel_work_sync(&xprt->task_cleanup);
> > /*
> > * Tear down transport state and free the rpc_xprt
> > */
> > -----------------------------[snip]---------------------------------
> >
> > Thanks,
>
> How about doing a wait_on_bit_lock(&xprt->state, XPRT_LOCKED,...)
> instead?
I'll believe you, but I don't get it: what guarantees that XPRT_LOCKED
isn't cleared while xprt_autoclose autoclose is still running?
--b.
^ permalink raw reply
* Re: sunrpc: what prevents an xprt from being freed before task_cleanup runs?
From: Trond Myklebust @ 2010-08-03 18:49 UTC (permalink / raw)
To: Jeff Layton
Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
bfields-uC3wQj2KruNg9hUCZPvPmw
In-Reply-To: <1280860815.9771.25.camel-rJ7iovZKK19ZJLDQqaL3InhyD016LWXt@public.gmane.org>
On Tue, 2010-08-03 at 14:40 -0400, Trond Myklebust wrote:
> On Tue, 2010-08-03 at 13:24 -0400, Jeff Layton wrote:
> > We got a report recently about a panic in RHEL5 (2.6.18 based kernel).
> > The problem appears to be that a task_cleanup workqueue job ran and got
> > passed a pointer to an xprt that had been freed. The bug is here in
> > case anyone is interested in the details:
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=611938
> >
> > The situation seems to be pretty difficult to reproduce, but I don't
> > see anything that's intended to ensure that this doesn't occur in RHEL5
> > or mainline. The task_cleanup workqueue job doesn't hold a reference to
> > the xprt, and the job isn't canceled when the xprt is torn down.
> >
> > Bruce had a look and suggested that we may need something like the
> > patch below (pasted in, so it probably won't apply correctly). I've
> > tested a backported version of it on RHEL5 and it seems to work fine.
> >
> > Is it reasonable to cancel task_cleanup when destroying the xprt? Or,
> > am I missing something that should prevent this situation in mainline
> > (and perhaps isn't in RHEL5's kernel).
> >
> > Any help is appreciated...
> >
> > -----------------------------[snip]---------------------------------
> > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> > index dcd0132..2a1f664 100644
> > --- a/net/sunrpc/xprt.c
> > +++ b/net/sunrpc/xprt.c
> > @@ -1129,6 +1129,7 @@ static void xprt_destroy(struct kref *kref)
> > rpc_destroy_wait_queue(&xprt->sending);
> > rpc_destroy_wait_queue(&xprt->resend);
> > rpc_destroy_wait_queue(&xprt->backlog);
> > + cancel_work_sync(&xprt->task_cleanup);
> > /*
> > * Tear down transport state and free the rpc_xprt
> > */
> > -----------------------------[snip]---------------------------------
> >
> > Thanks,
>
> How about doing a wait_on_bit_lock(&xprt->state, XPRT_LOCKED,...)
> instead?
Hmm... On the other hand, since xprt_destroy() may get called from
inside the rpciod_workqueue, the wait_on_bit_lock() may end up
deadlocking.
Nevermind, then. Let's go with the cancel_work_sync().
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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: sunrpc: what prevents an xprt from being freed before task_cleanup runs?
From: Trond Myklebust @ 2010-08-03 18:40 UTC (permalink / raw)
To: Jeff Layton
Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
bfields-uC3wQj2KruNg9hUCZPvPmw
In-Reply-To: <20100803132453.4fa18444-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
On Tue, 2010-08-03 at 13:24 -0400, Jeff Layton wrote:
> We got a report recently about a panic in RHEL5 (2.6.18 based kernel).
> The problem appears to be that a task_cleanup workqueue job ran and got
> passed a pointer to an xprt that had been freed. The bug is here in
> case anyone is interested in the details:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=611938
>
> The situation seems to be pretty difficult to reproduce, but I don't
> see anything that's intended to ensure that this doesn't occur in RHEL5
> or mainline. The task_cleanup workqueue job doesn't hold a reference to
> the xprt, and the job isn't canceled when the xprt is torn down.
>
> Bruce had a look and suggested that we may need something like the
> patch below (pasted in, so it probably won't apply correctly). I've
> tested a backported version of it on RHEL5 and it seems to work fine.
>
> Is it reasonable to cancel task_cleanup when destroying the xprt? Or,
> am I missing something that should prevent this situation in mainline
> (and perhaps isn't in RHEL5's kernel).
>
> Any help is appreciated...
>
> -----------------------------[snip]---------------------------------
> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
> index dcd0132..2a1f664 100644
> --- a/net/sunrpc/xprt.c
> +++ b/net/sunrpc/xprt.c
> @@ -1129,6 +1129,7 @@ static void xprt_destroy(struct kref *kref)
> rpc_destroy_wait_queue(&xprt->sending);
> rpc_destroy_wait_queue(&xprt->resend);
> rpc_destroy_wait_queue(&xprt->backlog);
> + cancel_work_sync(&xprt->task_cleanup);
> /*
> * Tear down transport state and free the rpc_xprt
> */
> -----------------------------[snip]---------------------------------
>
> Thanks,
How about doing a wait_on_bit_lock(&xprt->state, XPRT_LOCKED,...)
instead?
Cheers
Trond
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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
* [GIT PULL] xen-netfront driver updates for 2.6.36
From: Jeremy Fitzhardinge @ 2010-08-03 18:39 UTC (permalink / raw)
To: David Miller
Cc: Linux Kernel Mailing List, NetDev, Xen-devel@lists.xensource.com,
Xu, Dongxiao
Hi Dave,
OK, let's try that again.
Here's a git branch with some updates to the Xen netfront driver, which
add a new "smartpoll" mode to the netfront backend protocol, which is
basically equivalent to interrupt mitigation. This merges cleanly with
current linux-next (it previously just had a small constification conflict).
Do you want to take this via the net tree, or are you OK with me
submitting them directly?
Thanks,
J
The following changes since commit 9fe6206f400646a2322096b56c59891d530e8d51:
Linux 2.6.35 (2010-08-01 15:11:14 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git
upstream/netfront
Dongxiao Xu (1):
xen/netfront: Use smart polling instead of event notification.
Ian Campbell (2):
xen/rings: make protocol specific usage of shared sring explicit
xen/netfront: make protocol specific usage of shared sring explicit
drivers/net/xen-netfront.c | 114
++++++++++++++++++++++++++++++++++++++-
include/xen/interface/io/ring.h | 11 ++++-
2 files changed, 122 insertions(+), 3 deletions(-)
^ permalink raw reply
* Re: [ANNOUNCE]: Release of iptables-1.4.9
From: Jan Engelhardt @ 2010-08-03 18:34 UTC (permalink / raw)
To: Gabor Z. Papp
Cc: Patrick McHardy, Netfilter Development Mailinglist,
Linux Netdev List, 'netfilter@vger.kernel.org',
netfilter-announce
In-Reply-To: <x6sk2vo9eq@gzp>
On Tuesday 2010-08-03 20:09, Gabor Z. Papp wrote:
>* Jan Engelhardt <jengelh@medozas.de>:
>
>| (BTW, with --disable-shared you remove the possibility to use any .so
>| files whatsoever. You can use --enable-static --enable-shared to get
>| both "all in one binary" and ".so support".)
>
>And how to force linking the binaries against the static libs?
What libs?
^ permalink raw reply
* Re: [GIT PULL] xen-netfront driver updates for 2.6.36
From: Jeremy Fitzhardinge @ 2010-08-03 18:12 UTC (permalink / raw)
To: David Miller
Cc: Linux Kernel Mailing List, NetDev, Xen-devel@lists.xensource.com,
Xu, Dongxiao
In-Reply-To: <4C585BC8.5070203@goop.org>
On 08/03/2010 11:11 AM, Jeremy Fitzhardinge wrote:
> Hi Dave,
>
> Here's a git branch with some updates to the Xen netfront driver,
> which add a new "smartpoll" mode to the netfront backend protocol,
> which is basically equivalent to interrupt mitigation. They've been
> in linux-next for a week or so.
Oops, never mind.
They got left out of linux-next, and they have a merge conflict. I'll
sort it out and resubmit.
'Thanks,
J
^ permalink raw reply
* [GIT PULL] xen-netfront driver updates for 2.6.36
From: Jeremy Fitzhardinge @ 2010-08-03 18:11 UTC (permalink / raw)
To: David Miller
Cc: Linux Kernel Mailing List, NetDev, Xen-devel@lists.xensource.com,
Xu, Dongxiao
Hi Dave,
Here's a git branch with some updates to the Xen netfront driver, which
add a new "smartpoll" mode to the netfront backend protocol, which is
basically equivalent to interrupt mitigation. They've been in
linux-next for a week or so.
Do you want to take this via the net tree, or are you OK with me
submitting them directly?
Thanks,
J
The following changes since commit 74fca6a42863ffacaf7ba6f1936a9f228950f657:
Linux 2.6.31 (2009-09-09 15:13:59 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jeremy/xen.git upstream/netfront
Dongxiao Xu (1):
xen/netfront: Use smart polling instead of event notification.
Ian Campbell (2):
xen/rings: make protocol specific usage of shared sring explicit
xen/netfront: make protocol specific usage of shared sring explicit
drivers/net/xen-netfront.c | 114 ++++++++++++++++++++++++++++++++++++++-
include/xen/interface/io/ring.h | 11 ++++-
2 files changed, 122 insertions(+), 3 deletions(-)
^ permalink raw reply
* Re: [ANNOUNCE]: Release of iptables-1.4.9
From: Gabor Z. Papp @ 2010-08-03 18:09 UTC (permalink / raw)
To: Jan Engelhardt
Cc: Patrick McHardy, Netfilter Development Mailinglist,
Linux Netdev List, 'netfilter@vger.kernel.org',
netfilter-announce
In-Reply-To: <alpine.LSU.2.01.1008032001430.1519@obet.zrqbmnf.qr>
* Jan Engelhardt <jengelh@medozas.de>:
| (BTW, with --disable-shared you remove the possibility to use any .so
| files whatsoever. You can use --enable-static --enable-shared to get
| both "all in one binary" and ".so support".)
And how to force linking the binaries against the static libs?
| The following changes since commit 371cea299f0b2eb100b9fc9fb99089640d2d606f:
| xtables: remove unnecessary cast (2010-08-03 19:56:11 +0200)
| are available in the git repository at:
| git://dev.medozas.de/iptables master
Fixed, compiled fine.
^ permalink raw reply
* Re: [ANNOUNCE]: Release of iptables-1.4.9
From: Jan Engelhardt @ 2010-08-03 18:04 UTC (permalink / raw)
To: Gabor Z. Papp
Cc: Patrick McHardy, Netfilter Development Mailinglist,
Linux Netdev List, 'netfilter@vger.kernel.org',
netfilter-announce
In-Reply-To: <x6lj8nwqu4@gzp>
On Tuesday 2010-08-03 19:25, Gabor Z. Papp wrote:
>* "Gabor Z. Papp" <gzp@papp.hu>:
>
>| Note the difference, new undefined reference: `libxt_IDLETIMER_init
>
>--disable-shared --enable-static configure options cause the problems.
>
>--enable-shared --disable-static compiles fine, but I would like to
>link against the static libs.
(BTW, with --disable-shared you remove the possibility to use any .so
files whatsoever. You can use --enable-static --enable-shared to get
both "all in one binary" and ".so support".)
The following changes since commit 371cea299f0b2eb100b9fc9fb99089640d2d606f:
xtables: remove unnecessary cast (2010-08-03 19:56:11 +0200)
are available in the git repository at:
git://dev.medozas.de/iptables master
Jan Engelhardt (1):
build: fix static linking
extensions/libxt_IDLETIMER.c | 2 +-
extensions/libxt_TEE.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
[Actually there's also the "remove unnecessary cast" patch that is not
included in this listing, but merging that should be ok.]
parent 371cea299f0b2eb100b9fc9fb99089640d2d606f (v1.4.9-18-g371cea2)
commit 0428e5a6541c3f5eaaf683d8da9ea60c44eac4c7
Author: Jan Engelhardt <jengelh@medozas.de>
Date: Tue Aug 3 19:58:38 2010 +0200
build: fix static linking
Gabor Z. Papp noted this link-time error when configuring with
--enable-static:
extensions/libext4.a(initext4.o): In function "init_extensions":
extensions/initext4.c:144: undefined reference to "libxt_IDLETIMER_init"
extensions/initext4.c:145: undefined reference to "libxt_TEE_init"
Indeed, since the two modules did not use our special macro "_init"
(which expands to libxt_foo_init), initext4.c could not find them by
that name. Correct this.
References: http://marc.info/?l=netfilter&m=128085480927924&w=2
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
extensions/libxt_IDLETIMER.c | 2 +-
extensions/libxt_TEE.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/extensions/libxt_IDLETIMER.c b/extensions/libxt_IDLETIMER.c
index 12573a4..1562e02 100644
--- a/extensions/libxt_IDLETIMER.c
+++ b/extensions/libxt_IDLETIMER.c
@@ -132,7 +132,7 @@ static struct xtables_target idletimer_tg_reg = {
.extra_opts = idletimer_tg_opts,
};
-static __attribute__((constructor)) void idletimer_tg_ldr(void)
+void _init(void)
{
xtables_register_target(&idletimer_tg_reg);
}
diff --git a/extensions/libxt_TEE.c b/extensions/libxt_TEE.c
index f8e7fd1..e4c0607 100644
--- a/extensions/libxt_TEE.c
+++ b/extensions/libxt_TEE.c
@@ -195,7 +195,7 @@ static struct xtables_target tee_tg6_reg = {
.extra_opts = tee_tg_opts,
};
-static __attribute__((constructor)) void tee_tg_ldr(void)
+void _init(void)
{
xtables_register_target(&tee_tg_reg);
xtables_register_target(&tee_tg6_reg);
--
# Created with git-export-patch
^ permalink raw reply related
* Re: [PATCH] can-raw: Fix skb_orphan_try handling
From: Oliver Hartkopp @ 2010-08-03 17:50 UTC (permalink / raw)
To: Patrick Ohly
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Miller,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
In-Reply-To: <1280856828.3266.839.camel-/oqbx4SGF9pV+x+AlpbFz62pdiUAq4bhAL8bYrjMMd8@public.gmane.org>
On 03.08.2010 19:33, Patrick Ohly wrote:
> On Tue, 2010-08-03 at 10:11 -0700, Oliver Hartkopp wrote:
>>> I'm fine with using a simple u8. I'm not sure where I picked up the
>>> union thing, it's not something that I usually do in my own code.
>>>
>>
>> :-)
>>
>> Im currently busy until next week, would you like to provide a patch?
>
> Sorry, I have to pass. I'm busy elsewhere myself. MeeGo and
> SyncEvolution plus real life consume all of my time nowadays.
Working for MeeGo is a good excuse ;-)
Will pick that task next week.
BR,
Oliver
^ permalink raw reply
* Re: [PATCH] can-raw: Fix skb_orphan_try handling
From: Patrick Ohly @ 2010-08-03 17:33 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: David Miller, eric.dumazet@gmail.com, netdev@vger.kernel.org,
socketcan-core@lists.berlios.de, matthias.fuchs@esd.eu
In-Reply-To: <4C584DB2.8020406@hartkopp.net>
On Tue, 2010-08-03 at 10:11 -0700, Oliver Hartkopp wrote:
> > I'm fine with using a simple u8. I'm not sure where I picked up the
> > union thing, it's not something that I usually do in my own code.
> >
>
> :-)
>
> Im currently busy until next week, would you like to provide a patch?
Sorry, I have to pass. I'm busy elsewhere myself. MeeGo and
SyncEvolution plus real life consume all of my time nowadays.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
^ permalink raw reply
* Re: [ANNOUNCE]: Release of iptables-1.4.9
From: Michele Petrazzo - Unipex @ 2010-08-03 17:29 UTC (permalink / raw)
To: Gabor Z. Papp
Cc: Patrick McHardy, Netfilter Development Mailinglist,
Linux Netdev List, 'netfilter@vger.kernel.org',
netfilter-announce
In-Reply-To: <x6vd7rwrah@gzp>
Gabor Z. Papp wrote:
> * Patrick McHardy <kaber@trash.net>:
>
> | Did you use a git checkout/patch against 1.4.8 or the tar.bz2?
> | In case its the former, try running make clean, sh autogen.sh and
> | sh configure again before the build.
>
> Used the tarball, but tried git also:
>
Here git works. Or better, compile but I don't find the "iptables" command.
If I enable static compilation "configure --enable-static", I receive:
extensions/libext4.a(initext4.o): In function `init_extensions':
/home/devel/iptables/extensions/initext4.c:103: undefined reference to
`libxt_IDLETIMER_init'
/home/devel/iptables/extensions/initext4.c:137: undefined reference to
`libxt_TEE_ini
Michele
^ permalink raw reply
* Re: [ANNOUNCE]: Release of iptables-1.4.9
From: Gabor Z. Papp @ 2010-08-03 17:25 UTC (permalink / raw)
To: Patrick McHardy
Cc: Netfilter Development Mailinglist, Linux Netdev List,
'netfilter@vger.kernel.org', netfilter-announce
In-Reply-To: <x6vd7rwrah@gzp>
* "Gabor Z. Papp" <gzp@papp.hu>:
| Note the difference, new undefined reference: `libxt_IDLETIMER_init
--disable-shared --enable-static configure options cause the problems.
--enable-shared --disable-static compiles fine, but I would like to
link against the static libs.
^ permalink raw reply
* sunrpc: what prevents an xprt from being freed before task_cleanup runs?
From: Jeff Layton @ 2010-08-03 17:24 UTC (permalink / raw)
To: linux-nfs-u79uwXL29TY76Z2rM5mHXA
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, bfields-uC3wQj2KruNg9hUCZPvPmw
We got a report recently about a panic in RHEL5 (2.6.18 based kernel).
The problem appears to be that a task_cleanup workqueue job ran and got
passed a pointer to an xprt that had been freed. The bug is here in
case anyone is interested in the details:
https://bugzilla.redhat.com/show_bug.cgi?id=611938
The situation seems to be pretty difficult to reproduce, but I don't
see anything that's intended to ensure that this doesn't occur in RHEL5
or mainline. The task_cleanup workqueue job doesn't hold a reference to
the xprt, and the job isn't canceled when the xprt is torn down.
Bruce had a look and suggested that we may need something like the
patch below (pasted in, so it probably won't apply correctly). I've
tested a backported version of it on RHEL5 and it seems to work fine.
Is it reasonable to cancel task_cleanup when destroying the xprt? Or,
am I missing something that should prevent this situation in mainline
(and perhaps isn't in RHEL5's kernel).
Any help is appreciated...
-----------------------------[snip]---------------------------------
diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
index dcd0132..2a1f664 100644
--- a/net/sunrpc/xprt.c
+++ b/net/sunrpc/xprt.c
@@ -1129,6 +1129,7 @@ static void xprt_destroy(struct kref *kref)
rpc_destroy_wait_queue(&xprt->sending);
rpc_destroy_wait_queue(&xprt->resend);
rpc_destroy_wait_queue(&xprt->backlog);
+ cancel_work_sync(&xprt->task_cleanup);
/*
* Tear down transport state and free the rpc_xprt
*/
-----------------------------[snip]---------------------------------
Thanks,
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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 related
* Re: [ANNOUNCE]: Release of iptables-1.4.9
From: Gabor Z. Papp @ 2010-08-03 17:16 UTC (permalink / raw)
To: Patrick McHardy
Cc: Netfilter Development Mailinglist, Linux Netdev List,
'netfilter@vger.kernel.org', netfilter-announce
In-Reply-To: <4C584C63.8010607@trash.net>
* Patrick McHardy <kaber@trash.net>:
| Did you use a git checkout/patch against 1.4.8 or the tar.bz2?
| In case its the former, try running make clean, sh autogen.sh and
| sh configure again before the build.
Used the tarball, but tried git also:
/bin/sh ./libtool --tag=CC --mode=link gcc -D_LARGEFILE_SOURCE=1
-D_LARGE_FILES -D_FILE_OFFSET_BITS=64 -D_REENTRANT -Wall
-Waggregate-return -Wmissing-declarations -Wmissing-prototypes
-Wredundant-decls -Wshadow -Wstrict-prototypes -Winline -pipe
-DXTABLES_LIBDIR=\"/pkg/lib/xtables\" -DXTABLES_INTERNAL -I./include
-I./include -DIPTABLES_MULTI -DALL_INCLUSIVE -g -O2 -rdynamic -o
iptables-multi iptables_multi-iptables-multi.o
iptables_multi-iptables-save.o iptables_multi-iptables-restore.o
iptables_multi-iptables-xml.o iptables_multi-iptables-standalone.o
iptables_multi-iptables.o iptables_multi-xshared.o libiptc/libip4tc.la
extensions/libext4.a libxtables.la -lm
libtool: link: gcc -D_LARGEFILE_SOURCE=1 -D_LARGE_FILES
-D_FILE_OFFSET_BITS=64 -D_REENTRANT -Wall -Waggregate-return
-Wmissing-declarations -Wmissing-prototypes -Wredundant-decls -Wshadow
-Wstrict-prototypes -Winline -pipe
-DXTABLES_LIBDIR=\"/pkg/lib/xtables\" -DXTABLES_INTERNAL -I./include
-I./include -DIPTABLES_MULTI -DALL_INCLUSIVE -g -O2 -rdynamic -o
iptables-multi iptables_multi-iptables-multi.o
iptables_multi-iptables-save.o iptables_multi-iptables-restore.o
iptables_multi-iptables-xml.o iptables_multi-iptables-standalone.o
iptables_multi-iptables.o iptables_multi-xshared.o
libiptc/.libs/libip4tc.a extensions/libext4.a ./.libs/libxtables.a -lm
collect2: ld terminated with signal 11 [Segmentation fault]
extensions/libext4.a(initext4.o): In function `init_extensions':
/home/gzp/src/iptables/extensions/initext4.c:144: undefined reference to `libxt_IDLETIMER_init'
/home/gzp/src/iptables/extensions/initext4.c:145: undefined reference to `libxt_TEE_init'
make[2]: *** [iptables-multi] Error 1
Note the difference, new undefined reference: `libxt_IDLETIMER_init
^ permalink raw reply
* Re: [PATCH] can-raw: Fix skb_orphan_try handling
From: Oliver Hartkopp @ 2010-08-03 17:11 UTC (permalink / raw)
To: Patrick Ohly
Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, David Miller,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
In-Reply-To: <1280852081.3266.834.camel-/oqbx4SGF9pV+x+AlpbFz62pdiUAq4bhAL8bYrjMMd8@public.gmane.org>
On 03.08.2010 18:14, Patrick Ohly wrote:
> On Tue, 2010-08-03 at 18:22 +0300, Oliver Hartkopp wrote:
>>>> The flags are tested all together in skb_orphan_try() ...
>>>
>>> This is why I hate using unions in situations like this... it makes
>>> code impossible to audit easily.
>>>
>>> This damn thing should just be a "u8 flags" and a bunch of bit mask
>>> CPP macro defines for the various boolean values.
>>
>> Yep! I also felt like this.
>>
>> Maybe Patrick Ohly can give some feedback, if he's ok with that kind of
>> change. So far there are only a few places that would need to be changed for
>> the flags bitops.
>
> I'm fine with using a simple u8. I'm not sure where I picked up the
> union thing, it's not something that I usually do in my own code.
>
:-)
Im currently busy until next week, would you like to provide a patch?
Regards,
Oliver
^ 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