Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[David Brownell]

> > Jody - Is the 200K waste for sure or do you want me to verify it by some 
> > means? ( Reason I am asking is firstly, Dave Brownell was quite sure it 
> > wasn't that costly and secondly, I am hoping it isn't.. ;)
> 
> I'm not sure, but I looked through the code and it seems to allocate:
>  - 16 buffers of 2x PAGE_SIZE (= 131072 on i386)
>  - 16 buffers of PAGE_SIZE (= 65536 on i386)
>  - various other smaller structures.
> 
> I'm not sure how to actually _measure_ how much memory this is using.
> slabinfo isn't useful, at least on my system, because the 1394
> allocations get lost in the noise of other activity.

Those allocations could be from _using_ a dma pool ... but they're
not from just creating one!

The cost of creating the dma_pool is the cost of one small kmalloc()
plus (the expensive part) the /sys/devices/.../pools sysfs attribute
is created along with the first pool.  (Use that instead of slabinfo
for those pool allocations.)  That's why the normal spot to create and
destroy dma pools is in driver probe() and remove() methods.

If you want to allocate gobs of other stuff at the same time, that's
your choice ... but it'd be a separate issue.  Cost to create a
dma_pool is significantly less than PAGE_SIZE, and there's no good
reason to allocate or destroy those pools anywhere near an IRQ context.

- Dave

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Parag Warudkar]

On Saturday 19 February 2005 02:36 pm, David Brownell wrote:
> The cost of creating the dma_pool is the cost of one small kmalloc()
> plus (the expensive part) the /sys/devices/.../pools sysfs attribute
> is created along with the first pool.  (Use that instead of slabinfo
> for those pool allocations.)  That's why the normal spot to create and
> destroy dma pools is in driver probe() and remove() methods.

What's the format of /sys/devices/.../pools (Name of pool, ? ? ? ?) ?  Can the 
memory consumption be derived from it? 
Here is what the ohci pools look during data read (Kino->Capture) and after 
closing Kino -

During Kino Capture
[root@localhost pci0000:00]# cat ./0000:00:0a.0/0000:02:00.0/pools
poolinfo - 0.1
ohci1394 rcv prg   16  256   16  1 ------------------> This one is in question
ohci1394 trm prg   32   64   64  1
ohci1394 trm prg   32   64   64  1
ohci1394 rcv prg    4  256   16  1
ohci1394 rcv prg    4  256   16  1

After Closing Kino
[root@localhost pci0000:00]# cat ./0000:00:0a.0/0000:02:00.0/pools
poolinfo - 0.1
ohci1394 trm prg   32   64   64  1
ohci1394 trm prg   32   64   64  1
ohci1394 rcv prg    4  256   16  1
ohci1394 rcv prg    4  256   16  1

Parag

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[David Brownell]

On Saturday 19 February 2005 12:50 pm, Parag Warudkar wrote:
> On Saturday 19 February 2005 02:36 pm, David Brownell wrote:
> > The cost of creating the dma_pool is the cost of one small kmalloc()
> > plus (the expensive part) the /sys/devices/.../pools sysfs attribute
> > is created along with the first pool.  (Use that instead of slabinfo
> > for those pool allocations.)  That's why the normal spot to create and
> > destroy dma pools is in driver probe() and remove() methods.
> 
> What's the format of /sys/devices/.../pools (Name of pool, ? ? ? ?) ?  Can the 
> memory consumption be derived from it? 

See what drivers/base/dmapool.c tells you; yes that
consumption can be derived from the pool statistics.


> Here is what the ohci pools look during data read (Kino->Capture) and after 
> closing Kino -
> 
> During Kino Capture
> [root@localhost pci0000:00]# cat ./0000:00:0a.0/0000:02:00.0/pools
> poolinfo - 0.1
> ohci1394 rcv prg   16  256   16  1 ------------------> This one is in question
> ohci1394 trm prg   32   64   64  1
> ohci1394 trm prg   32   64   64  1
> ohci1394 rcv prg    4  256   16  1
> ohci1394 rcv prg    4  256   16  1

I suggest you get rid of the spaces in those names, to make it
easier to use things like "awk" to massage those numbers.  The
"ohci1394" is implied by the fact that no other driver could
be bound to that device, for example.

In this case, other than the fact that you've created multiple
pools with the same names (!), that line translates to 16 blocks
in use out of 256 available, 16 bytes per block, just one page.

I suspect that on some systems it should be bumping up the minimum
block size to match the CPU cacheline size.

- Dave

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Jody McIntyre]

On Sat, Feb 19, 2005 at 11:36:05AM -0800, David Brownell wrote:
> 
> Those allocations could be from _using_ a dma pool ... but they're
> not from just creating one!
> 
> The cost of creating the dma_pool is the cost of one small kmalloc()
> plus (the expensive part) the /sys/devices/.../pools sysfs attribute
> is created along with the first pool.  (Use that instead of slabinfo
> for those pool allocations.)  That's why the normal spot to create and
> destroy dma pools is in driver probe() and remove() methods.

OK, then I misread drivers/base/pool.c and my objection to the patch is
unfounded.

> If you want to allocate gobs of other stuff at the same time, that's
> your choice ... but it'd be a separate issue.  Cost to create a
> dma_pool is significantly less than PAGE_SIZE, and there's no good
> reason to allocate or destroy those pools anywhere near an IRQ context.

I agree.  raw1394 does far too much with irqs disabled, and moving this
stuff to probe() will fix part of that problem.

Jody

> 
> - Dave
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 

[PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Parag Warudkar]

[-- Attachment #1: Type: text/plain, Size: 1555 bytes --]

Problem - ohci1394.c:ohci_devctl ends up calling dma_pool_destroy from 
invalid context. Below is the dmesg output when I exit Kino after video 
capture -

Debug: sleeping function called from invalid context at 
include/asm/semaphore.h:107
in_atomic():1, irqs_disabled():1
 [<c0104c2e>] dump_stack+0x1e/0x20
 [<c011f8a2>] __might_sleep+0xa2/0xc0
 [<c028c660>] dma_pool_destroy+0x20/0x140
 [<f0b7affe>] free_dma_rcv_ctx+0x8e/0x150 [ohci1394]
 [<f0b774a4>] ohci_devctl+0x214/0x9b0 [ohci1394]
 [<f0e7aa99>] handle_iso_listen+0x2d9/0x310 [raw1394]
 [<f0e7f22b>] state_connected+0x29b/0x2b0 [raw1394]
 [<f0e7f2de>] raw1394_write+0x9e/0xd0 [raw1394]
 [<c0183ef2>] vfs_write+0xc2/0x170
 [<c018406b>] sys_write+0x4b/0x80
 [<c0103cb1>] sysenter_past_esp+0x52/0x75

Attached patch against 2.6.11-rc2 (tested to work normally with a  
camcorder device) enables ohci_devctl to defer the work of destroying 
the dma pool by using a work queue, so the dma pool is destroyed in a 
valid context and we no longer get an error in dmesg (duh :)

Note : There still exists  one more similar problem with ohci1394 - that 
is with dma_pool_create being called from invalid context - this happens 
in response to ISO_LISTEN_CHANNEL. I get dmesg debug for this case which 
is similar to the above. My analysis is that this one is non-trivial to 
fix. More on this in a separate mail.

Patch is attached since T'Bird messes up with the inlined one. (Pls. 
suggest a good email client for kernel patch stuff :)

Signed-off-by: Parag Warudkar (kernel-stuff@comcast.net)


Parag



[-- Attachment #2: patch-ohci1394 --]
[-- Type: text/plain, Size: 1596 bytes --]

--- drivers/ieee1394/ohci1394.c.orig	2004-12-24 16:35:25.000000000 -0500
+++ drivers/ieee1394/ohci1394.c	2005-01-30 15:46:34.000000000 -0500
@@ -99,6 +99,7 @@
 #include <asm/uaccess.h>
 #include <linux/delay.h>
 #include <linux/spinlock.h>
+#include <linux/workqueue.h>
 
 #include <asm/pgtable.h>
 #include <asm/page.h>
@@ -164,6 +165,7 @@
 static char version[] __devinitdata =
 	"$Rev: 1223 $ Ben Collins <bcollins@debian.org>";
 
+
 /* Module Parameters */
 static int phys_dma = 1;
 module_param(phys_dma, int, 0644);
@@ -184,6 +186,8 @@
 
 static void ohci1394_pci_remove(struct pci_dev *pdev);
 
+static void ohci_free_dma_work_fn(void* data);
+
 #ifndef __LITTLE_ENDIAN
 static unsigned hdr_sizes[] =
 {
@@ -1130,6 +1134,13 @@
 	return retval;
 }
 
+static void ohci_free_dma_work_fn(void* data)
+{
+	struct pci_pool* prg_pool = (struct pci_pool*) data;
+	pci_pool_destroy(prg_pool);
+	OHCI_DMA_FREE("dma_rcv prg pool");
+}
+
 /***********************************
  * rawiso ISO reception            *
  ***********************************/
@@ -2898,13 +2909,14 @@
 		kfree(d->buf_bus);
 	}
 	if (d->prg_cpu) {
+		struct work_struct ohci_free_dma_work;
 		for (i=0; i<d->num_desc; i++)
 			if (d->prg_cpu[i] && d->prg_bus[i]) {
 				pci_pool_free(d->prg_pool, d->prg_cpu[i], d->prg_bus[i]);
 				OHCI_DMA_FREE("consistent dma_rcv prg[%d]", i);
 			}
-		pci_pool_destroy(d->prg_pool);
-		OHCI_DMA_FREE("dma_rcv prg pool");
+		INIT_WORK(&ohci_free_dma_work, ohci_free_dma_work_fn, (void*) d->prg_pool);
+		schedule_work(&ohci_free_dma_work);
 		kfree(d->prg_cpu);
 		kfree(d->prg_bus);
 	}

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Andrew Morton]

Parag Warudkar <kernel-stuff@comcast.net> wrote:
>
> Problem - ohci1394.c:ohci_devctl ends up calling dma_pool_destroy from 
> invalid context. Below is the dmesg output when I exit Kino after video 
> capture -
> 
> Debug: sleeping function called from invalid context at 
> include/asm/semaphore.h:107
> in_atomic():1, irqs_disabled():1
>  [<c0104c2e>] dump_stack+0x1e/0x20
>  [<c011f8a2>] __might_sleep+0xa2/0xc0
>  [<c028c660>] dma_pool_destroy+0x20/0x140
>  [<f0b7affe>] free_dma_rcv_ctx+0x8e/0x150 [ohci1394]
>  [<f0b774a4>] ohci_devctl+0x214/0x9b0 [ohci1394]
>  [<f0e7aa99>] handle_iso_listen+0x2d9/0x310 [raw1394]
>  [<f0e7f22b>] state_connected+0x29b/0x2b0 [raw1394]
>  [<f0e7f2de>] raw1394_write+0x9e/0xd0 [raw1394]
>  [<c0183ef2>] vfs_write+0xc2/0x170
>  [<c018406b>] sys_write+0x4b/0x80
>  [<c0103cb1>] sysenter_past_esp+0x52/0x75

Yes, that's certainly wrong.

> Attached patch against 2.6.11-rc2 (tested to work normally with a  
> camcorder device) enables ohci_devctl to defer the work of destroying 
> the dma pool by using a work queue, so the dma pool is destroyed in a 
> valid context and we no longer get an error in dmesg (duh :)

yup.  But what happens if someone removes the module while
ohci_free_dma_work_fn() is still pending?

Suggestions:

- The work_struct cannot be on the stack.  The code as you have it will
  read gunk from the stack when the delayed work executes.  The work_struct
  needs to be placed into some ohci data structure which has the appropriate
  lifetime.  That might be struct ti_ohci.  Or not.

- We'll need a flush_workqueue() in the teardown function for that data
  structure to ensure that any pending callbacks have completed before we
  free the storage.

  Care needs to be taken to ensure that the work_struct is suitably
  initialised so that the flush_workqueue() will work OK even if the
  callback has never been scheduled.

- You have several typecasts between struct pci_pool* and void*.  These
  defeat typechecking.  It's better to leave these casts out.

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Parag Warudkar]

Andrew Morton wrote:

>yup.  But what happens if someone removes the module while
>ohci_free_dma_work_fn() is still pending?
>
>Suggestions:
>
>- The work_struct cannot be on the stack.  The code as you have it will
>  read gunk from the stack when the delayed work executes.  The work_struct
>  needs to be placed into some ohci data structure which has the appropriate
>  lifetime.  That might be struct ti_ohci.  Or not.
>  
>
Ok, got it.

>- We'll need a flush_workqueue() in the teardown function for that data
>  structure to ensure that any pending callbacks have completed before we
>  free the storage.
>  
>
By saying flush_workqueue did you intend to suggest using separate work 
queue for ohci1394? I think that might be the way to go since otherwise 
ohci1394 would have to wait on all other irrelevant work elements in the 
global queue to be finished. This will help in solving the other problem 
of dma_pool_create as well - we could then schedule_work for all 
create_pools and just do a flush_workqueue before starting to actually 
use the device. (Might help in reducing those sound skips when I attach 
my camcorder :) Error handling has to change a good amount (right now 
the init function returns ENOMEM if dma_pool_create fails and all is 
done. With this approach  of asynch  alloc , init function will not know 
if the allocation failed / succeeded. All other functions (like say 
resulting from sys_read) will have to explicitly check and return error 
to user if the pool create had failed.)

>  Care needs to be taken to ensure that the work_struct is suitably
>  initialised so that the flush_workqueue() will work OK even if the
>  callback has never been scheduled.
>  
>
Didn't understand this one  - Is this about properly NULL'ing elements 
in work queue so flush_workqueue doesn't touch them? Can you elaborate 
please?

>- You have several typecasts between struct pci_pool* and void*.  These
>  defeat typechecking.  It's better to leave these casts out.
>  
>
Will leave them out.

Thanks

Parag

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Andrew Morton]

Parag Warudkar <kernel-stuff@comcast.net> wrote:
>
> Andrew Morton wrote:
> 
> ...
> >- We'll need a flush_workqueue() in the teardown function for that data
> >  structure to ensure that any pending callbacks have completed before we
> >  free the storage.
> >  
> >
> By saying flush_workqueue did you intend to suggest using separate work 
> queue for ohci1394?

No.  Using keventd and flush_scheduled_work() should be OK in this
application.  rmmod isn't very common.

> >  Care needs to be taken to ensure that the work_struct is suitably
> >  initialised so that the flush_workqueue() will work OK even if the
> >  callback has never been scheduled.
> >  
> >
> Didn't understand this one  - Is this about properly NULL'ing elements 
> in work queue so flush_workqueue doesn't touch them? Can you elaborate 
> please?

Well, it's just that the shutdown code needs to run flush_scheduled_work()
or flush_workqueue() on a work_struct which may or may not have been used. 
So we need to ensure that it has sane contents.  Just run INIT_WORK() on it
in the setup code.

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Parag Warudkar]

[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]

Attached is the reworked patch to take care of Andrew's suggestions -

1) Allocate the work struct dynamically in struct ti_ohci during  device 
probe, free it during device remove
2) In ohci1394_pci_remove, ensure queued work, if any, is flushed before 
the device is removed (As I understand, this should work for both device 
removal cases as well as module removal - correct?)
3) Rework the free_dma_rcv_ctx  - It is called in multiple contexts by 
different callers - teach it to free the dma according to caller's wish 
- either direct free where caller determines the context is safe to 
sleep OR delayed via schedule_work when caller wants to call it from a 
context where it cannot sleep.  So it now takes 2 extra arguments - 
method to free - direct or delayed and if it is to be  freed delayed, an 
appropriate work_struct.

Andrew - flush_scheduled_work does not touch work which isn't shceduled 
- so I don't think INIT_WORK in setup is necessary. Let me know if I am 
missing something here. (I tried INIT_WORK in ochi1394_pci_probe and 
putting flush_scheduled_work in ohci1394_pci_remove - It did not result 
in calling the work function after I did rmmod, since it wasn't scheduled.)

Parag



[-- Attachment #2: patch-ohci1394 --]
[-- Type: text/plain, Size: 5918 bytes --]

--- drivers/ieee1394/ohci1394.c.orig	2004-12-24 16:35:25.000000000 -0500
+++ drivers/ieee1394/ohci1394.c	2005-01-30 20:00:42.000000000 -0500
@@ -99,6 +99,7 @@
 #include <asm/uaccess.h>
 #include <linux/delay.h>
 #include <linux/spinlock.h>
+#include <linux/workqueue.h>
 
 #include <asm/pgtable.h>
 #include <asm/page.h>
@@ -161,6 +162,10 @@
 #define PRINT(level, fmt, args...) \
 printk(level "%s: fw-host%d: " fmt "\n" , OHCI1394_DRIVER_NAME, ohci->host->id , ## args)
 
+/* Instruct free_dma_rcv_ctx how to free dma pools */
+#define OHCI_FREE_DMA_CTX_DIRECT 0
+#define OHCI_FREE_DMA_CTX_DELAYED 1
+
 static char version[] __devinitdata =
 	"$Rev: 1223 $ Ben Collins <bcollins@debian.org>";
 
@@ -176,7 +181,8 @@
 			     enum context_type type, int ctx, int num_desc,
 			     int buf_size, int split_buf_size, int context_base);
 static void stop_dma_rcv_ctx(struct dma_rcv_ctx *d);
-static void free_dma_rcv_ctx(struct dma_rcv_ctx *d);
+static void free_dma_rcv_ctx(struct dma_rcv_ctx *d , int method,
+			     struct work_struct* work);
 
 static int alloc_dma_trm_ctx(struct ti_ohci *ohci, struct dma_trm_ctx *d,
 			     enum context_type type, int ctx, int num_desc,
@@ -184,6 +190,8 @@
 
 static void ohci1394_pci_remove(struct pci_dev *pdev);
 
+static void ohci_free_irlc_pci_pool(void* data);
+
 #ifndef __LITTLE_ENDIAN
 static unsigned hdr_sizes[] =
 {
@@ -1117,7 +1125,8 @@
 
 		if (ohci->ir_legacy_channels == 0) {
 			stop_dma_rcv_ctx(&ohci->ir_legacy_context);
-			free_dma_rcv_ctx(&ohci->ir_legacy_context);
+			INIT_WORK(ohci->irlc_free_dma, ohci_free_irlc_pci_pool, ohci->ir_legacy_context.prg_pool);
+			free_dma_rcv_ctx(&ohci->ir_legacy_context, OHCI_FREE_DMA_CTX_DELAYED, ohci->irlc_free_dma);
 			DBGMSG("ISO receive legacy context deactivated");
 		}
                 break;
@@ -2876,7 +2885,8 @@
 }
 
 
-static void free_dma_rcv_ctx(struct dma_rcv_ctx *d)
+static void free_dma_rcv_ctx(struct dma_rcv_ctx *d, int method,
+		struct work_struct* work)
 {
 	int i;
 	struct ti_ohci *ohci = d->ohci;
@@ -2903,8 +2913,20 @@
 				pci_pool_free(d->prg_pool, d->prg_cpu[i], d->prg_bus[i]);
 				OHCI_DMA_FREE("consistent dma_rcv prg[%d]", i);
 			}
-		pci_pool_destroy(d->prg_pool);
-		OHCI_DMA_FREE("dma_rcv prg pool");
+		if(method == OHCI_FREE_DMA_CTX_DIRECT)
+		{
+			pci_pool_destroy(d->prg_pool);
+			OHCI_DMA_FREE("dma_rcv prg pool");
+		}
+		
+		else if(method == OHCI_FREE_DMA_CTX_DELAYED)
+		{
+			schedule_work(work);
+		}
+
+		else
+			PRINT(KERN_ERR, "Invalid method passed - %d", method);
+
 		kfree(d->prg_cpu);
 		kfree(d->prg_bus);
 	}
@@ -2938,7 +2960,7 @@
 
 	if (d->buf_cpu == NULL || d->buf_bus == NULL) {
 		PRINT(KERN_ERR, "Failed to allocate dma buffer");
-		free_dma_rcv_ctx(d);
+		free_dma_rcv_ctx(d, OHCI_FREE_DMA_CTX_DIRECT, NULL);
 		return -ENOMEM;
 	}
 	memset(d->buf_cpu, 0, d->num_desc * sizeof(quadlet_t*));
@@ -2950,7 +2972,7 @@
 
 	if (d->prg_cpu == NULL || d->prg_bus == NULL) {
 		PRINT(KERN_ERR, "Failed to allocate dma prg");
-		free_dma_rcv_ctx(d);
+		free_dma_rcv_ctx(d, OHCI_FREE_DMA_CTX_DIRECT, NULL);
 		return -ENOMEM;
 	}
 	memset(d->prg_cpu, 0, d->num_desc * sizeof(struct dma_cmd*));
@@ -2960,7 +2982,7 @@
 
 	if (d->spb == NULL) {
 		PRINT(KERN_ERR, "Failed to allocate split buffer");
-		free_dma_rcv_ctx(d);
+		free_dma_rcv_ctx(d, OHCI_FREE_DMA_CTX_DIRECT, NULL);
 		return -ENOMEM;
 	}
 
@@ -2979,7 +3001,7 @@
 		} else {
 			PRINT(KERN_ERR,
 			      "Failed to allocate dma buffer");
-			free_dma_rcv_ctx(d);
+			free_dma_rcv_ctx(d, OHCI_FREE_DMA_CTX_DIRECT, NULL);
 			return -ENOMEM;
 		}
 
@@ -2991,7 +3013,7 @@
 		} else {
 			PRINT(KERN_ERR,
 			      "Failed to allocate dma prg");
-			free_dma_rcv_ctx(d);
+			free_dma_rcv_ctx(d, OHCI_FREE_DMA_CTX_DIRECT, NULL);
 			return -ENOMEM;
 		}
 	}
@@ -3005,7 +3027,7 @@
 		if (ohci1394_register_iso_tasklet(ohci,
 						  &ohci->ir_legacy_tasklet) < 0) {
 			PRINT(KERN_ERR, "No IR DMA context available");
-			free_dma_rcv_ctx(d);
+			free_dma_rcv_ctx(d, OHCI_FREE_DMA_CTX_DIRECT, NULL);
 			return -EBUSY;
 		}
 
@@ -3355,6 +3377,7 @@
 
 	/* the IR DMA context is allocated on-demand; mark it inactive */
 	ohci->ir_legacy_context.ohci = NULL;
+	ohci->ir_legacy_context.prg_pool = NULL;
 
 	/* same for the IT DMA context */
 	ohci->it_legacy_context.ohci = NULL;
@@ -3377,16 +3400,32 @@
 	if (hpsb_add_host(host))
 		FAIL(-ENOMEM, "Failed to register host with highlevel");
 
+	ohci->irlc_free_dma = kmalloc(sizeof(struct work_struct), GFP_KERNEL);
+	if(ohci->irlc_free_dma == NULL)
+		FAIL(-ENOMEM, "Failed to allocate memory for ohci->irlc_free_dma");
+
 	ohci->init_state = OHCI_INIT_DONE;
 
 	return 0;
 #undef FAIL
 }
 
+static void ohci_free_irlc_pci_pool(void *data)
+{
+	if(data == NULL)
+		return;
+	pci_pool_destroy(data);
+	OHCI_DMA_FREE("dma_rcv prg pool");
+	return;
+}
+	
 static void ohci1394_pci_remove(struct pci_dev *pdev)
 {
 	struct ti_ohci *ohci;
 	struct device *dev;
+	
+	/* Ensure all dma ctx are freed */
+	flush_scheduled_work();
 
 	ohci = pci_get_drvdata(pdev);
 	if (!ohci)
@@ -3432,18 +3471,21 @@
 		/* The ohci_soft_reset() stops all DMA contexts, so we
 		 * dont need to do this.  */
 		/* Free AR dma */
-		free_dma_rcv_ctx(&ohci->ar_req_context);
-		free_dma_rcv_ctx(&ohci->ar_resp_context);
+		free_dma_rcv_ctx(&ohci->ar_req_context, OHCI_FREE_DMA_CTX_DIRECT, NULL);
+		free_dma_rcv_ctx(&ohci->ar_resp_context, OHCI_FREE_DMA_CTX_DIRECT, NULL);
 
 		/* Free AT dma */
 		free_dma_trm_ctx(&ohci->at_req_context);
 		free_dma_trm_ctx(&ohci->at_resp_context);
 
 		/* Free IR dma */
-		free_dma_rcv_ctx(&ohci->ir_legacy_context);
+		free_dma_rcv_ctx(&ohci->ir_legacy_context, OHCI_FREE_DMA_CTX_DIRECT, NULL);
 
 		/* Free IT dma */
 		free_dma_trm_ctx(&ohci->it_legacy_context);
+		
+		/* Free work struct for IR Legacy DMA */
+		kfree(ohci->irlc_free_dma);
 
 	case OHCI_INIT_HAVE_SELFID_BUFFER:
 		pci_free_consistent(ohci->dev, OHCI1394_SI_DMA_BUF_SIZE,

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Parag Warudkar]

[-- Attachment #1: Type: text/plain, Size: 8257 bytes --]

Forgot to attach ohci.h diff which is affected by this change as well.

Meanwhile, David Brownell suggested it would be much simpler to move the 
dma allocations to _probe and  deallocations to _remove.
I am working on it and so far haven't got it to work.

Andrew - What do you think? If you too feel allocating pci_pool for the 
legacy contexts (only disadvantage I see is that the pool will be 
allocated even if it is not used, that is if ISO_LISTEN_CHANNEL isn't 
called any time)  in _probe an removing it in _remove is a better thing 
to do, I will concentrate on getting that to work and we can forget 
about this patch. I feel there might be some reason why the current code 
treats the legacy IR and IT DMA ctxs differently (on-demand  
allocation).. Ben?

Parag

Parag Warudkar wrote:

> Attached is the reworked patch to take care of Andrew's suggestions -
>
> 1) Allocate the work struct dynamically in struct ti_ohci during  
> device probe, free it during device remove
> 2) In ohci1394_pci_remove, ensure queued work, if any, is flushed 
> before the device is removed (As I understand, this should work for 
> both device removal cases as well as module removal - correct?)
> 3) Rework the free_dma_rcv_ctx  - It is called in multiple contexts by 
> different callers - teach it to free the dma according to caller's 
> wish - either direct free where caller determines the context is safe 
> to sleep OR delayed via schedule_work when caller wants to call it 
> from a context where it cannot sleep.  So it now takes 2 extra 
> arguments - method to free - direct or delayed and if it is to be  
> freed delayed, an appropriate work_struct.
>
> Andrew - flush_scheduled_work does not touch work which isn't 
> shceduled - so I don't think INIT_WORK in setup is necessary. Let me 
> know if I am missing something here. (I tried INIT_WORK in 
> ochi1394_pci_probe and putting flush_scheduled_work in 
> ohci1394_pci_remove - It did not result in calling the work function 
> after I did rmmod, since it wasn't scheduled.)
>
> Parag
>
>
>------------------------------------------------------------------------
>
>--- drivers/ieee1394/ohci1394.c.orig	2004-12-24 16:35:25.000000000 -0500
>+++ drivers/ieee1394/ohci1394.c	2005-01-30 20:00:42.000000000 -0500
>@@ -99,6 +99,7 @@
> #include <asm/uaccess.h>
> #include <linux/delay.h>
> #include <linux/spinlock.h>
>+#include <linux/workqueue.h>
> 
> #include <asm/pgtable.h>
> #include <asm/page.h>
>@@ -161,6 +162,10 @@
> #define PRINT(level, fmt, args...) \
> printk(level "%s: fw-host%d: " fmt "\n" , OHCI1394_DRIVER_NAME, ohci->host->id , ## args)
> 
>+/* Instruct free_dma_rcv_ctx how to free dma pools */
>+#define OHCI_FREE_DMA_CTX_DIRECT 0
>+#define OHCI_FREE_DMA_CTX_DELAYED 1
>+
> static char version[] __devinitdata =
> 	"$Rev: 1223 $ Ben Collins <bcollins@debian.org>";
> 
>@@ -176,7 +181,8 @@
> 			     enum context_type type, int ctx, int num_desc,
> 			     int buf_size, int split_buf_size, int context_base);
> static void stop_dma_rcv_ctx(struct dma_rcv_ctx *d);
>-static void free_dma_rcv_ctx(struct dma_rcv_ctx *d);
>+static void free_dma_rcv_ctx(struct dma_rcv_ctx *d , int method,
>+			     struct work_struct* work);
> 
> static int alloc_dma_trm_ctx(struct ti_ohci *ohci, struct dma_trm_ctx *d,
> 			     enum context_type type, int ctx, int num_desc,
>@@ -184,6 +190,8 @@
> 
> static void ohci1394_pci_remove(struct pci_dev *pdev);
> 
>+static void ohci_free_irlc_pci_pool(void* data);
>+
> #ifndef __LITTLE_ENDIAN
> static unsigned hdr_sizes[] =
> {
>@@ -1117,7 +1125,8 @@
> 
> 		if (ohci->ir_legacy_channels == 0) {
> 			stop_dma_rcv_ctx(&ohci->ir_legacy_context);
>-			free_dma_rcv_ctx(&ohci->ir_legacy_context);
>+			INIT_WORK(ohci->irlc_free_dma, ohci_free_irlc_pci_pool, ohci->ir_legacy_context.prg_pool);
>+			free_dma_rcv_ctx(&ohci->ir_legacy_context, OHCI_FREE_DMA_CTX_DELAYED, ohci->irlc_free_dma);
> 			DBGMSG("ISO receive legacy context deactivated");
> 		}
>                 break;
>@@ -2876,7 +2885,8 @@
> }
> 
> 
>-static void free_dma_rcv_ctx(struct dma_rcv_ctx *d)
>+static void free_dma_rcv_ctx(struct dma_rcv_ctx *d, int method,
>+		struct work_struct* work)
> {
> 	int i;
> 	struct ti_ohci *ohci = d->ohci;
>@@ -2903,8 +2913,20 @@
> 				pci_pool_free(d->prg_pool, d->prg_cpu[i], d->prg_bus[i]);
> 				OHCI_DMA_FREE("consistent dma_rcv prg[%d]", i);
> 			}
>-		pci_pool_destroy(d->prg_pool);
>-		OHCI_DMA_FREE("dma_rcv prg pool");
>+		if(method == OHCI_FREE_DMA_CTX_DIRECT)
>+		{
>+			pci_pool_destroy(d->prg_pool);
>+			OHCI_DMA_FREE("dma_rcv prg pool");
>+		}
>+		
>+		else if(method == OHCI_FREE_DMA_CTX_DELAYED)
>+		{
>+			schedule_work(work);
>+		}
>+
>+		else
>+			PRINT(KERN_ERR, "Invalid method passed - %d", method);
>+
> 		kfree(d->prg_cpu);
> 		kfree(d->prg_bus);
> 	}
>@@ -2938,7 +2960,7 @@
> 
> 	if (d->buf_cpu == NULL || d->buf_bus == NULL) {
> 		PRINT(KERN_ERR, "Failed to allocate dma buffer");
>-		free_dma_rcv_ctx(d);
>+		free_dma_rcv_ctx(d, OHCI_FREE_DMA_CTX_DIRECT, NULL);
> 		return -ENOMEM;
> 	}
> 	memset(d->buf_cpu, 0, d->num_desc * sizeof(quadlet_t*));
>@@ -2950,7 +2972,7 @@
> 
> 	if (d->prg_cpu == NULL || d->prg_bus == NULL) {
> 		PRINT(KERN_ERR, "Failed to allocate dma prg");
>-		free_dma_rcv_ctx(d);
>+		free_dma_rcv_ctx(d, OHCI_FREE_DMA_CTX_DIRECT, NULL);
> 		return -ENOMEM;
> 	}
> 	memset(d->prg_cpu, 0, d->num_desc * sizeof(struct dma_cmd*));
>@@ -2960,7 +2982,7 @@
> 
> 	if (d->spb == NULL) {
> 		PRINT(KERN_ERR, "Failed to allocate split buffer");
>-		free_dma_rcv_ctx(d);
>+		free_dma_rcv_ctx(d, OHCI_FREE_DMA_CTX_DIRECT, NULL);
> 		return -ENOMEM;
> 	}
> 
>@@ -2979,7 +3001,7 @@
> 		} else {
> 			PRINT(KERN_ERR,
> 			      "Failed to allocate dma buffer");
>-			free_dma_rcv_ctx(d);
>+			free_dma_rcv_ctx(d, OHCI_FREE_DMA_CTX_DIRECT, NULL);
> 			return -ENOMEM;
> 		}
> 
>@@ -2991,7 +3013,7 @@
> 		} else {
> 			PRINT(KERN_ERR,
> 			      "Failed to allocate dma prg");
>-			free_dma_rcv_ctx(d);
>+			free_dma_rcv_ctx(d, OHCI_FREE_DMA_CTX_DIRECT, NULL);
> 			return -ENOMEM;
> 		}
> 	}
>@@ -3005,7 +3027,7 @@
> 		if (ohci1394_register_iso_tasklet(ohci,
> 						  &ohci->ir_legacy_tasklet) < 0) {
> 			PRINT(KERN_ERR, "No IR DMA context available");
>-			free_dma_rcv_ctx(d);
>+			free_dma_rcv_ctx(d, OHCI_FREE_DMA_CTX_DIRECT, NULL);
> 			return -EBUSY;
> 		}
> 
>@@ -3355,6 +3377,7 @@
> 
> 	/* the IR DMA context is allocated on-demand; mark it inactive */
> 	ohci->ir_legacy_context.ohci = NULL;
>+	ohci->ir_legacy_context.prg_pool = NULL;
> 
> 	/* same for the IT DMA context */
> 	ohci->it_legacy_context.ohci = NULL;
>@@ -3377,16 +3400,32 @@
> 	if (hpsb_add_host(host))
> 		FAIL(-ENOMEM, "Failed to register host with highlevel");
> 
>+	ohci->irlc_free_dma = kmalloc(sizeof(struct work_struct), GFP_KERNEL);
>+	if(ohci->irlc_free_dma == NULL)
>+		FAIL(-ENOMEM, "Failed to allocate memory for ohci->irlc_free_dma");
>+
> 	ohci->init_state = OHCI_INIT_DONE;
> 
> 	return 0;
> #undef FAIL
> }
> 
>+static void ohci_free_irlc_pci_pool(void *data)
>+{
>+	if(data == NULL)
>+		return;
>+	pci_pool_destroy(data);
>+	OHCI_DMA_FREE("dma_rcv prg pool");
>+	return;
>+}
>+	
> static void ohci1394_pci_remove(struct pci_dev *pdev)
> {
> 	struct ti_ohci *ohci;
> 	struct device *dev;
>+	
>+	/* Ensure all dma ctx are freed */
>+	flush_scheduled_work();
> 
> 	ohci = pci_get_drvdata(pdev);
> 	if (!ohci)
>@@ -3432,18 +3471,21 @@
> 		/* The ohci_soft_reset() stops all DMA contexts, so we
> 		 * dont need to do this.  */
> 		/* Free AR dma */
>-		free_dma_rcv_ctx(&ohci->ar_req_context);
>-		free_dma_rcv_ctx(&ohci->ar_resp_context);
>+		free_dma_rcv_ctx(&ohci->ar_req_context, OHCI_FREE_DMA_CTX_DIRECT, NULL);
>+		free_dma_rcv_ctx(&ohci->ar_resp_context, OHCI_FREE_DMA_CTX_DIRECT, NULL);
> 
> 		/* Free AT dma */
> 		free_dma_trm_ctx(&ohci->at_req_context);
> 		free_dma_trm_ctx(&ohci->at_resp_context);
> 
> 		/* Free IR dma */
>-		free_dma_rcv_ctx(&ohci->ir_legacy_context);
>+		free_dma_rcv_ctx(&ohci->ir_legacy_context, OHCI_FREE_DMA_CTX_DIRECT, NULL);
> 
> 		/* Free IT dma */
> 		free_dma_trm_ctx(&ohci->it_legacy_context);
>+		
>+		/* Free work struct for IR Legacy DMA */
>+		kfree(ohci->irlc_free_dma);
> 
> 	case OHCI_INIT_HAVE_SELFID_BUFFER:
> 		pci_free_consistent(ohci->dev, OHCI1394_SI_DMA_BUF_SIZE,
>  
>


[-- Attachment #2: ohci1394.h.patch --]
[-- Type: text/plain, Size: 375 bytes --]

--- drivers/ieee1394/ohci1394.h.orig	2004-12-24 16:34:00.000000000 -0500
+++ drivers/ieee1394/ohci1394.h	2005-01-31 18:16:31.000000000 -0500
@@ -197,6 +197,8 @@
 				   it is safe to free the legacy API context */
 
 	struct dma_rcv_ctx ir_legacy_context;
+	struct work_struct* irlc_free_dma;
+	
 	struct ohci1394_iso_tasklet ir_legacy_tasklet;
 
         /* iso transmit */

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Dan Dennedy]

On Sun, 2005-01-30 at 20:19 -0500, Parag Warudkar wrote:
> Attached is the reworked patch to take care of Andrew's suggestions -
> 
> 1) Allocate the work struct dynamically in struct ti_ohci during  device 
> probe, free it during device remove
> 2) In ohci1394_pci_remove, ensure queued work, if any, is flushed before 
> the device is removed (As I understand, this should work for both device 
> removal cases as well as module removal - correct?)
> 3) Rework the free_dma_rcv_ctx  - It is called in multiple contexts by 
> different callers - teach it to free the dma according to caller's wish 
> - either direct free where caller determines the context is safe to 
> sleep OR delayed via schedule_work when caller wants to call it from a 
> context where it cannot sleep.  So it now takes 2 extra arguments - 
> method to free - direct or delayed and if it is to be  freed delayed, an 
> appropriate work_struct.
> 
> Andrew - flush_scheduled_work does not touch work which isn't shceduled 
> - so I don't think INIT_WORK in setup is necessary. Let me know if I am 
> missing something here. (I tried INIT_WORK in ochi1394_pci_probe and 
> putting flush_scheduled_work in ohci1394_pci_remove - It did not result 
> in calling the work function after I did rmmod, since it wasn't scheduled.)
> 

I am testing this patch in the same manner as you: exiting Kino capture.
I am getting a similar error in a different location. Can you look into
it, please?

Debug: sleeping function called from invalid context at mm/slab.c:2082
in_atomic():1, irqs_disabled():1
 [<c0119eb1>] __might_sleep+0xa1/0xc0
 [<c0144914>] kmem_cache_alloc+0x64/0x80
 [<c037f07b>] dma_pool_create+0x7b/0x190
 [<e09ede32>] alloc_dma_rcv_ctx+0x1a2/0x400 [ohci1394]
 [<e09eb239>] ohci_devctl+0x3d9/0x640 [ohci1394]
 [<e0bc5d4e>] handle_iso_listen+0xee/0x160 [raw1394]
 [<e0bc878e>] state_connected+0x2de/0x2f0 [raw1394]
 [<e0bc884e>] raw1394_write+0xae/0xe0 [raw1394]
 [<c015c80c>] vfs_write+0x14c/0x160
 [<c015c8f1>] sys_write+0x51/0x80
 [<c0102a39>] sysenter_past_esp+0x52/0x75

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Jody McIntyre]

On Fri, Feb 11, 2005 at 10:35:33AM -0500, Dan Dennedy wrote:

> I am testing this patch in the same manner as you: exiting Kino capture.
> I am getting a similar error in a different location. Can you look into
> it, please?
> 
> Debug: sleeping function called from invalid context at mm/slab.c:2082
> in_atomic():1, irqs_disabled():1
>  [<c0119eb1>] __might_sleep+0xa1/0xc0
>  [<c0144914>] kmem_cache_alloc+0x64/0x80
>  [<c037f07b>] dma_pool_create+0x7b/0x190
>  [<e09ede32>] alloc_dma_rcv_ctx+0x1a2/0x400 [ohci1394]
>  [<e09eb239>] ohci_devctl+0x3d9/0x640 [ohci1394]
>  [<e0bc5d4e>] handle_iso_listen+0xee/0x160 [raw1394]
>  [<e0bc878e>] state_connected+0x2de/0x2f0 [raw1394]
>  [<e0bc884e>] raw1394_write+0xae/0xe0 [raw1394]
>  [<c015c80c>] vfs_write+0x14c/0x160
>  [<c015c8f1>] sys_write+0x51/0x80
>  [<c0102a39>] sysenter_past_esp+0x52/0x75

Does this happen on exit or on startup?  Looks like allocation problems,
which will be harder to fix, since you can't return to userland until
the allocation is complete.  AFAICT the correct fix is to use
finer-grained locks, hold them for less time, and not use _irq or
_irqsave unless necessary.  host_info_lock, in particular, is held for
far too long.

Jody

> 
> 
> 
> 
> -------------------------------------------------------
> SF email is sponsored by - The IT Product Guide
> Read honest & candid reviews on hundreds of IT Products from real users.
> Discover which products truly live up to the hype. Start reading now.
> http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
> _______________________________________________
> mailing list linux1394-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux1394-devel

-- 

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Parag Warudkar]

Jody,
This happens every time you connect a device which ends up doing
ISO_LISTEN_CHANNEL. We fixed the device disconnect case in -mm recently.

I had sent you and Andrew an alternative patch which fixes this
dma_pool_create case as well as the dma_pool_destroy case, albeit with a
disadvantage - The patch does pre-allocation of the IR Legacy DMA in
_pci_probe and deallocates it in _pci_remove. However I am not truly
happy with it since it possibly wastes 200K of memory for people who
don't have devices which need it.

As I said earlier, I think the way to fix this is via schedule_work
similar to the disconnect case, but it involves good amount of code
change. I am working on it - any better ideas most welcome.

Dan - Can you try the attached patch - on top current -mm1? (It's pretty
no brainer that it will fix both cases but two testing heads are better
than one.. :)

Thanks

Parag

On Fri, 2005-02-11 at 13:43 -0500, Jody McIntyre wrote:
> On Fri, Feb 11, 2005 at 10:35:33AM -0500, Dan Dennedy wrote:
> 
> > I am testing this patch in the same manner as you: exiting Kino capture.
> > I am getting a similar error in a different location. Can you look into
> > it, please?
> > 
> > Debug: sleeping function called from invalid context at mm/slab.c:2082
> > in_atomic():1, irqs_disabled():1
> >  [<c0119eb1>] __might_sleep+0xa1/0xc0
> >  [<c0144914>] kmem_cache_alloc+0x64/0x80
> >  [<c037f07b>] dma_pool_create+0x7b/0x190
> >  [<e09ede32>] alloc_dma_rcv_ctx+0x1a2/0x400 [ohci1394]
> >  [<e09eb239>] ohci_devctl+0x3d9/0x640 [ohci1394]
> >  [<e0bc5d4e>] handle_iso_listen+0xee/0x160 [raw1394]
> >  [<e0bc878e>] state_connected+0x2de/0x2f0 [raw1394]
> >  [<e0bc884e>] raw1394_write+0xae/0xe0 [raw1394]
> >  [<c015c80c>] vfs_write+0x14c/0x160
> >  [<c015c8f1>] sys_write+0x51/0x80
> >  [<c0102a39>] sysenter_past_esp+0x52/0x75
> 
> Does this happen on exit or on startup?  Looks like allocation problems,
> which will be harder to fix, since you can't return to userland until
> the allocation is complete.  AFAICT the correct fix is to use
> finer-grained locks, hold them for less time, and not use _irq or
> _irqsave unless necessary.  host_info_lock, in particular, is held for
> far too long.
> 
> Jody
> 
> > 
> > 
> > 
> > 
> > -------------------------------------------------------
> > SF email is sponsored by - The IT Product Guide
> > Read honest & candid reviews on hundreds of IT Products from real users.
> > Discover which products truly live up to the hype. Start reading now.
> > http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
> > _______________________________________________
> > mailing list linux1394-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux1394-devel
> 

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Dan Dennedy]

I have tested the patches (including for allocation), and it is working
great, but should I only commit for now the deallocation patch? Hmm..
which is worse the debug or the 200K waste?

On Fri, 2005-02-11 at 22:54 -0500, Parag Warudkar wrote:
> Jody,
> This happens every time you connect a device which ends up doing
> ISO_LISTEN_CHANNEL. We fixed the device disconnect case in -mm recently.
> 
> I had sent you and Andrew an alternative patch which fixes this
> dma_pool_create case as well as the dma_pool_destroy case, albeit with a
> disadvantage - The patch does pre-allocation of the IR Legacy DMA in
> _pci_probe and deallocates it in _pci_remove. However I am not truly
> happy with it since it possibly wastes 200K of memory for people who
> don't have devices which need it.
> 
> As I said earlier, I think the way to fix this is via schedule_work
> similar to the disconnect case, but it involves good amount of code
> change. I am working on it - any better ideas most welcome.
> 
> Dan - Can you try the attached patch - on top current -mm1? (It's pretty
> no brainer that it will fix both cases but two testing heads are better
> than one.. :)
> 

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Parag Warudkar]

On Friday 18 February 2005 10:32 am, Dan Dennedy wrote:
> I have tested the patches (including for allocation), and it is working
> great, but should I only commit for now the deallocation patch? Hmm..
> which is worse the debug or the 200K waste?
Thanks for following it up.

IMHO, we should commit both patches for now since we don't have an alternative 
solution yet. 

Jody - Is the 200K waste for sure or do you want me to verify it by some 
means? ( Reason I am asking is firstly, Dave Brownell was quite sure it 
wasn't that costly and secondly, I am hoping it isn't.. ;)

Parag

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Jody McIntyre]

On Fri, Feb 18, 2005 at 10:42:46AM -0500, Parag Warudkar wrote:
> On Friday 18 February 2005 10:32 am, Dan Dennedy wrote:
> > I have tested the patches (including for allocation), and it is working
> > great, but should I only commit for now the deallocation patch? Hmm..
> > which is worse the debug or the 200K waste?
> Thanks for following it up.
> 
> IMHO, we should commit both patches for now since we don't have an alternative 
> solution yet. 

I disagree because the impact of this bug is small.  How often do you start
an ISO receive?  If you think it needs to be fixed urgently, please
explain why - maybe I'm just missing somethnig.

> Jody - Is the 200K waste for sure or do you want me to verify it by some 
> means? ( Reason I am asking is firstly, Dave Brownell was quite sure it 
> wasn't that costly and secondly, I am hoping it isn't.. ;)

I'm not sure, but I looked through the code and it seems to allocate:
 - 16 buffers of 2x PAGE_SIZE (= 131072 on i386)
 - 16 buffers of PAGE_SIZE (= 65536 on i386)
 - various other smaller structures.

I'm not sure how to actually _measure_ how much memory this is using.
slabinfo isn't useful, at least on my system, because the 1394
allocations get lost in the noise of other activity.

If you really need this fixed quickly, I'll find some time this weekend
to examine the locks.  In particular, I'm not sure what host_info_lock
protects or why it needs to be held in so many places with irqs disabled.

Jody

> 
> Parag

-- 

Re: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()

[Parag Warudkar]

On Saturday 19 February 2005 01:36 am, Jody McIntyre wrote:
> I disagree because the impact of this bug is small.  How often do you start
> an ISO receive?  If you think it needs to be fixed urgently, please
> explain why - maybe I'm just missing somethnig.
>

I have to agree that the impact is small even for the people using ISO recv - 
I happen to use it quite frequently and it hasn't locked up on me yet. So I 
certainly don't need it fixed atm. It's just the "dmesg annoyance" if you 
will, to deal with :) !

> I'm not sure, but I looked through the code and it seems to allocate:
>  - 16 buffers of 2x PAGE_SIZE (= 131072 on i386)
>  - 16 buffers of PAGE_SIZE (= 65536 on i386)
>  - various other smaller structures.

OTOH, if it allocates so much of memory while irqs disabled and holding locks, 
isn't there a  good chance for the allocator to sleep and things to go wrong?

Parag