* [Fwd: [Bug 3003] New: might_sleep warning when setting up IPSec with IPCOMP]
@ 2004-07-02 17:56 Nivedita Singhvi
2004-07-02 17:58 ` Christophe Saout
2004-07-02 21:27 ` Nivedita Singhvi
0 siblings, 2 replies; 7+ messages in thread
From: Nivedita Singhvi @ 2004-07-02 17:56 UTC (permalink / raw)
To: James Morris, akpm; +Cc: netdev, christophe
James, Andrew,
Looks like deflate_comp_init() is not calling
__vmalloc in a kosher way.
We are not in softirq, so deflate_gfp() is going
to return GFP_KERNEL, so this is essentially
__vmalloc() with GFP_KERNEL|__GFP_HIGHMEM, PAGE_KERNEL.
Should we be using __vmalloc here, or is this a vm issue?
thanks,
Nivedita
-------- Original Message --------
Subject: [Bug 3003] New: might_sleep warning when setting up IPSec with IPCOMP
Date: Fri, 2 Jul 2004 10:32:44 -0700
From: bugme-daemon@osdl.org
To: niv@us.ibm.com
http://bugme.osdl.org/show_bug.cgi?id=3003
Summary: might_sleep warning when setting up IPSec with IPCOMP
Kernel Version: 2.6.7
Status: NEW
Severity: normal
Owner: niv@us.ibm.com
Submitter: christophe@saout.de
CC: christophe@saout.de
Linux 2.6.7 on x86, UP preempt
Using OpenSWAN for IPSec tunnel with IP-Compression enabled.
Often gives the following warning shortly after a connection has been set up:
I'm not exactly sure whether this is cryptoapi or the compression module or
IPCOMP's fault.
Jul 2 18:39:04 websrv Debug: sleeping function called from invalid context at
mm/slab.c:1994
Jul 2 18:39:04 websrv in_atomic():1, irqs_disabled():0
Jul 2 18:39:04 websrv [<c0106c37>] dump_stack+0x17/0x20
Jul 2 18:39:04 websrv [<c0118fb4>] __might_sleep+0xb4/0xe0
Jul 2 18:39:04 websrv [<c0139fbc>] kmem_cache_alloc+0x5c/0x60
Jul 2 18:39:04 websrv [<c0147bc0>] __get_vm_area+0x20/0xf0
Jul 2 18:39:04 websrv [<c0147cb4>] get_vm_area+0x24/0x30
Jul 2 18:39:04 websrv [<c0147f1c>] __vmalloc+0x3c/0x100
Jul 2 18:39:04 websrv [<c01ecdca>] deflate_comp_init+0x4a/0xf0
Jul 2 18:39:04 websrv [<c01ecf44>] deflate_compress+0x24/0x80
Jul 2 18:39:04 websrv [<c01e8a84>] crypto_compress+0x24/0x30
Jul 2 18:39:04 websrv [<c031744c>] ipcomp_compress+0x6c/0x110
Jul 2 18:39:04 websrv [<c0317681>] ipcomp_output+0xc1/0x370
Jul 2 18:39:04 websrv [<c02e684e>] dst_output+0xe/0x30
Jul 2 18:39:04 websrv [<c02d684a>] nf_hook_slow+0xfa/0x170
Jul 2 18:39:04 websrv [<c02e4d12>] ip_queue_xmit+0x452/0x540
Jul 2 18:39:04 websrv [<c02f48af>] tcp_transmit_skb+0x3ef/0x660
Jul 2 18:39:04 websrv [<c02f558b>] tcp_write_xmit+0x15b/0x2c0
Jul 2 18:39:04 websrv [<c02ea255>] tcp_sendmsg+0x4e5/0x1030
Jul 2 18:39:04 websrv [<c0309ec7>] inet_sendmsg+0x47/0x60
Jul 2 18:39:04 websrv [<c02c5fee>] sock_aio_write+0xae/0xd0
Jul 2 18:39:04 websrv [<c014d1b6>] do_sync_write+0x76/0xb0
Jul 2 18:39:04 websrv [<c014d2bb>] vfs_write+0xcb/0xf0
Jul 2 18:39:04 websrv [<c014d375>] sys_write+0x35/0x60
Jul 2 18:39:04 websrv [<c0105e3d>] sysenter_past_esp+0x52/0x71
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Fwd: [Bug 3003] New: might_sleep warning when setting up IPSec with IPCOMP] 2004-07-02 17:56 [Fwd: [Bug 3003] New: might_sleep warning when setting up IPSec with IPCOMP] Nivedita Singhvi @ 2004-07-02 17:58 ` Christophe Saout 2004-07-02 21:27 ` Nivedita Singhvi 1 sibling, 0 replies; 7+ messages in thread From: Christophe Saout @ 2004-07-02 17:58 UTC (permalink / raw) To: Nivedita Singhvi; +Cc: James Morris, akpm, netdev [-- Attachment #1: Type: text/plain, Size: 571 bytes --] Am Fr, den 02.07.2004 um 10:56 Uhr -0700 schrieb Nivedita Singhvi: > Looks like deflate_comp_init() is not calling > __vmalloc in a kosher way. > > [...] > Jul 2 18:39:04 websrv [<c0139fbc>] kmem_cache_alloc+0x5c/0x60 > Jul 2 18:39:04 websrv [<c0147bc0>] __get_vm_area+0x20/0xf0 > Jul 2 18:39:04 websrv [<c0147cb4>] get_vm_area+0x24/0x30 > Jul 2 18:39:04 websrv [<c0147f1c>] __vmalloc+0x3c/0x100 > [...] __get_vm_area does a area = kmalloc(sizeof(*area), GFP_KERNEL); So it will produce a warning no matter what flags are passed to __vmalloc. [-- Attachment #2: Dies ist ein digital signierter Nachrichtenteil --] [-- Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Fwd: [Bug 3003] New: might_sleep warning when setting up IPSec with IPCOMP] 2004-07-02 17:56 [Fwd: [Bug 3003] New: might_sleep warning when setting up IPSec with IPCOMP] Nivedita Singhvi 2004-07-02 17:58 ` Christophe Saout @ 2004-07-02 21:27 ` Nivedita Singhvi 2004-07-02 21:35 ` Christophe Saout 2004-07-02 21:39 ` Andrew Morton 1 sibling, 2 replies; 7+ messages in thread From: Nivedita Singhvi @ 2004-07-02 21:27 UTC (permalink / raw) To: Nivedita Singhvi; +Cc: James Morris, akpm, netdev, christophe, mjbligh Nivedita Singhvi wrote: > James, Andrew, > > Looks like deflate_comp_init() is not calling > __vmalloc in a kosher way. > Jul 2 18:39:04 websrv Debug: sleeping function called from invalid > context at > mm/slab.c:1994 > Jul 2 18:39:04 websrv in_atomic():1, irqs_disabled():0 > Jul 2 18:39:04 websrv [<c0106c37>] dump_stack+0x17/0x20 > Jul 2 18:39:04 websrv [<c0118fb4>] __might_sleep+0xb4/0xe0 > Jul 2 18:39:04 websrv [<c0139fbc>] kmem_cache_alloc+0x5c/0x60 > Jul 2 18:39:04 websrv [<c0147bc0>] __get_vm_area+0x20/0xf0 > Jul 2 18:39:04 websrv [<c0147cb4>] get_vm_area+0x24/0x30 > Jul 2 18:39:04 websrv [<c0147f1c>] __vmalloc+0x3c/0x100 > Jul 2 18:39:04 websrv [<c01ecdca>] deflate_comp_init+0x4a/0xf0 > Jul 2 18:39:04 websrv [<c01ecf44>] deflate_compress+0x24/0x80 > Jul 2 18:39:04 websrv [<c01e8a84>] crypto_compress+0x24/0x30 > Jul 2 18:39:04 websrv [<c031744c>] ipcomp_compress+0x6c/0x110 > Jul 2 18:39:04 websrv [<c0317681>] ipcomp_output+0xc1/0x370 We are grabbing dst->xfrm lock in ipcomp_output(), and have it held when we call ipcomp_compress(). Is that the issue? I don't have the crypto module code, but in_atomic() will be true. thanks, Nivedita ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Fwd: [Bug 3003] New: might_sleep warning when setting up IPSec with IPCOMP] 2004-07-02 21:27 ` Nivedita Singhvi @ 2004-07-02 21:35 ` Christophe Saout 2004-07-02 21:39 ` Andrew Morton 1 sibling, 0 replies; 7+ messages in thread From: Christophe Saout @ 2004-07-02 21:35 UTC (permalink / raw) To: Nivedita Singhvi; +Cc: James Morris, akpm, netdev, mjbligh [-- Attachment #1: Type: text/plain, Size: 799 bytes --] Am Fr, den 02.07.2004 um 14:27 Uhr -0700 schrieb Nivedita Singhvi: > We are grabbing dst->xfrm lock in ipcomp_output(), > and have it held when we call ipcomp_compress(). > > Is that the issue? I don't have the crypto module > code, but in_atomic() will be true. Yes. But the code might also be called from softirq context, when a packed from the NIC gets handled or when a slot in the queue becomes free. (I've also got warnings from those two cases in my logs) The compress/decompress calls should be able to be run from softirq (atomic) context just like encrypt/decrypt. I'm just wondering, why does deflate_compress call deflate_comp_init when it is called the first time, but deflate_init is a noop? Shouldn't the deflate_comp_init call just be moved to deflate_init? [-- Attachment #2: Dies ist ein digital signierter Nachrichtenteil --] [-- Type: application/pgp-signature, Size: 189 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Fwd: [Bug 3003] New: might_sleep warning when setting up IPSec with IPCOMP] 2004-07-02 21:27 ` Nivedita Singhvi 2004-07-02 21:35 ` Christophe Saout @ 2004-07-02 21:39 ` Andrew Morton 2004-07-09 4:20 ` James Morris 1 sibling, 1 reply; 7+ messages in thread From: Andrew Morton @ 2004-07-02 21:39 UTC (permalink / raw) To: Nivedita Singhvi; +Cc: niv, jmorris, netdev, christophe, mjbligh Nivedita Singhvi <niv@us.ibm.com> wrote: > > Nivedita Singhvi wrote: > > > James, Andrew, > > > > Looks like deflate_comp_init() is not calling > > __vmalloc in a kosher way. > > > Jul 2 18:39:04 websrv Debug: sleeping function called from invalid > > context at > > mm/slab.c:1994 > > Jul 2 18:39:04 websrv in_atomic():1, irqs_disabled():0 > > Jul 2 18:39:04 websrv [<c0106c37>] dump_stack+0x17/0x20 > > Jul 2 18:39:04 websrv [<c0118fb4>] __might_sleep+0xb4/0xe0 > > Jul 2 18:39:04 websrv [<c0139fbc>] kmem_cache_alloc+0x5c/0x60 > > Jul 2 18:39:04 websrv [<c0147bc0>] __get_vm_area+0x20/0xf0 > > Jul 2 18:39:04 websrv [<c0147cb4>] get_vm_area+0x24/0x30 > > Jul 2 18:39:04 websrv [<c0147f1c>] __vmalloc+0x3c/0x100 > > Jul 2 18:39:04 websrv [<c01ecdca>] deflate_comp_init+0x4a/0xf0 > > Jul 2 18:39:04 websrv [<c01ecf44>] deflate_compress+0x24/0x80 > > Jul 2 18:39:04 websrv [<c01e8a84>] crypto_compress+0x24/0x30 > > Jul 2 18:39:04 websrv [<c031744c>] ipcomp_compress+0x6c/0x110 > > Jul 2 18:39:04 websrv [<c0317681>] ipcomp_output+0xc1/0x370 > > We are grabbing dst->xfrm lock in ipcomp_output(), > and have it held when we call ipcomp_compress(). > > Is that the issue? I don't have the crypto module > code, but in_atomic() will be true. > Yes, that is the issue. >From a design point-of-view, it is not idiomatic for deflate_compress() to be doing this magical lazy initialisation. It would be better if the client of the deflate.c code were to explicitly initialise the context before diving in and using it. /* * Lazy initialization to make interface simple without allocating * un-needed workspaces. Thus can be called in softirq context. */ static int deflate_comp_init(struct deflate_ctx *ctx) Well no. Those games with deflate_gfp() really need to go away. in_atomic() works OK if CONFIG_PREEMPT is enabled. But with CONFIG_PREEMPT=n, in_atomic() returns false inside spinlock. And in_atomic()'s return value is unaffected by local_irq_disable(). This all needs to be redesigned, sorry. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Fwd: [Bug 3003] New: might_sleep warning when setting up IPSec with IPCOMP] 2004-07-02 21:39 ` Andrew Morton @ 2004-07-09 4:20 ` James Morris 2004-07-09 23:58 ` David S. Miller 0 siblings, 1 reply; 7+ messages in thread From: James Morris @ 2004-07-09 4:20 UTC (permalink / raw) To: Andrew Morton, David S. Miller Cc: Nivedita Singhvi, jmorris, netdev, christophe, mjbligh On Fri, 2 Jul 2004, Andrew Morton wrote: > Well no. Those games with deflate_gfp() really need to go away. > in_atomic() works OK if CONFIG_PREEMPT is enabled. But with > CONFIG_PREEMPT=n, in_atomic() returns false inside spinlock. And > in_atomic()'s return value is unaffected by local_irq_disable(). > > This all needs to be redesigned, sorry. Ok, fixed. Lazy allocation is gone. Please apply. Signed-off-by: James Morris <jmorris@redhat.com> diff -urN -X dontdiff linux-2.6.7-bk20.o/crypto/deflate.c linux-2.6.7-bk20.w/crypto/deflate.c --- linux-2.6.7-bk20.o/crypto/deflate.c 2004-06-16 01:20:04.000000000 -0400 +++ linux-2.6.7-bk20.w/crypto/deflate.c 2004-07-09 00:38:43.137189624 -0400 @@ -39,44 +39,16 @@ #define DEFLATE_DEF_MEMLEVEL MAX_MEM_LEVEL struct deflate_ctx { - int comp_initialized; - int decomp_initialized; struct z_stream_s comp_stream; struct z_stream_s decomp_stream; }; -static inline int deflate_gfp(void) -{ - return in_softirq() ? GFP_ATOMIC : GFP_KERNEL; -} - -static int deflate_init(void *ctx) -{ - return 0; -} - -static void deflate_exit(void *ctx) -{ - struct deflate_ctx *dctx = ctx; - - if (dctx->comp_initialized) - vfree(dctx->comp_stream.workspace); - if (dctx->decomp_initialized) - kfree(dctx->decomp_stream.workspace); -} - -/* - * Lazy initialization to make interface simple without allocating - * un-needed workspaces. Thus can be called in softirq context. - */ static int deflate_comp_init(struct deflate_ctx *ctx) { int ret = 0; struct z_stream_s *stream = &ctx->comp_stream; - stream->workspace = __vmalloc(zlib_deflate_workspacesize(), - deflate_gfp()|__GFP_HIGHMEM, - PAGE_KERNEL); + stream->workspace = vmalloc(zlib_deflate_workspacesize()); if (!stream->workspace ) { ret = -ENOMEM; goto out; @@ -89,7 +61,6 @@ ret = -EINVAL; goto out_free; } - ctx->comp_initialized = 1; out: return ret; out_free: @@ -102,8 +73,7 @@ int ret = 0; struct z_stream_s *stream = &ctx->decomp_stream; - stream->workspace = kmalloc(zlib_inflate_workspacesize(), - deflate_gfp()); + stream->workspace = kmalloc(zlib_inflate_workspacesize(), GFP_KERNEL); if (!stream->workspace ) { ret = -ENOMEM; goto out; @@ -114,7 +84,6 @@ ret = -EINVAL; goto out_free; } - ctx->decomp_initialized = 1; out: return ret; out_free: @@ -122,6 +91,36 @@ goto out; } +static void deflate_comp_exit(struct deflate_ctx *ctx) +{ + vfree(ctx->comp_stream.workspace); +} + +static void deflate_decomp_exit(struct deflate_ctx *ctx) +{ + kfree(ctx->decomp_stream.workspace); +} + +static int deflate_init(void *ctx) +{ + int ret; + + ret = deflate_comp_init(ctx); + if (ret) + goto out; + ret = deflate_decomp_init(ctx); + if (ret) + deflate_comp_exit(ctx); +out: + return ret; +} + +static void deflate_exit(void *ctx) +{ + deflate_comp_exit(ctx); + deflate_decomp_exit(ctx); +} + static int deflate_compress(void *ctx, const u8 *src, unsigned int slen, u8 *dst, unsigned int *dlen) { @@ -129,12 +128,6 @@ struct deflate_ctx *dctx = ctx; struct z_stream_s *stream = &dctx->comp_stream; - if (!dctx->comp_initialized) { - ret = deflate_comp_init(dctx); - if (ret) - goto out; - } - ret = zlib_deflateReset(stream); if (ret != Z_OK) { ret = -EINVAL; @@ -165,12 +158,6 @@ struct deflate_ctx *dctx = ctx; struct z_stream_s *stream = &dctx->decomp_stream; - if (!dctx->decomp_initialized) { - ret = deflate_decomp_init(dctx); - if (ret) - goto out; - } - ret = zlib_inflateReset(stream); if (ret != Z_OK) { ret = -EINVAL; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Fwd: [Bug 3003] New: might_sleep warning when setting up IPSec with IPCOMP] 2004-07-09 4:20 ` James Morris @ 2004-07-09 23:58 ` David S. Miller 0 siblings, 0 replies; 7+ messages in thread From: David S. Miller @ 2004-07-09 23:58 UTC (permalink / raw) To: James Morris; +Cc: akpm, niv, jmorris, netdev, christophe, mjbligh On Fri, 9 Jul 2004 00:20:56 -0400 (EDT) James Morris <jmorris@redhat.com> wrote: > On Fri, 2 Jul 2004, Andrew Morton wrote: > > > Well no. Those games with deflate_gfp() really need to go away. > > in_atomic() works OK if CONFIG_PREEMPT is enabled. But with > > CONFIG_PREEMPT=n, in_atomic() returns false inside spinlock. And > > in_atomic()'s return value is unaffected by local_irq_disable(). > > > > This all needs to be redesigned, sorry. > > Ok, fixed. Lazy allocation is gone. > > Please apply. Applied. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2004-07-09 23:58 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2004-07-02 17:56 [Fwd: [Bug 3003] New: might_sleep warning when setting up IPSec with IPCOMP] Nivedita Singhvi 2004-07-02 17:58 ` Christophe Saout 2004-07-02 21:27 ` Nivedita Singhvi 2004-07-02 21:35 ` Christophe Saout 2004-07-02 21:39 ` Andrew Morton 2004-07-09 4:20 ` James Morris 2004-07-09 23:58 ` David S. Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).