lkml.org 
[lkml]   [2005]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] ohci1394: dma_pool_destroy while in_atomic() && irqs_disabled()
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,
>
>

--- 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 */
\
 
 \ /
  Last update: 2005-03-22 14:10    [W:0.143 / U:0.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site