From: Parag Warudkar 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 - 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. 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.) DESC ohci1394-dma_pool_destroy-while-in_atomic-irqs_disabled-tidy EDESC DESC ohci1394-dma_pool_destroy-while-in_atomic-irqs_disabled-simplification EDESC Signed-off-by: Andrew Morton --- 25-akpm/drivers/ieee1394/ohci1394.c | 62 +++++++++++++++++++++++++++--------- 25-akpm/drivers/ieee1394/ohci1394.h | 2 + 2 files changed, 50 insertions(+), 14 deletions(-) diff -puN drivers/ieee1394/ohci1394.c~ohci1394-dma_pool_destroy-while-in_atomic-irqs_disabled drivers/ieee1394/ohci1394.c --- 25/drivers/ieee1394/ohci1394.c~ohci1394-dma_pool_destroy-while-in_atomic-irqs_disabled 2005-02-04 12:51:53.000000000 -0800 +++ 25-akpm/drivers/ieee1394/ohci1394.c 2005-02-04 12:56:18.703200888 -0800 @@ -99,6 +99,7 @@ #include #include #include +#include #include #include @@ -161,6 +162,10 @@ printk(level "%s: " fmt "\n" , OHCI1394_ #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: 1250 $ Ben Collins "; @@ -176,7 +181,8 @@ static int alloc_dma_rcv_ctx(struct ti_o 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 int alloc_dma_trm_ctx(struct ti_o 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[] = { @@ -1115,7 +1123,9 @@ static int ohci_devctl(struct hpsb_host if (ohci->ir_legacy_channels == 0) { stop_dma_rcv_ctx(&ohci->ir_legacy_context); - free_dma_rcv_ctx(&ohci->ir_legacy_context); + free_dma_rcv_ctx(&ohci->ir_legacy_context, + OHCI_FREE_DMA_CTX_DELAYED, + &ohci->irlc_free_dma); DBGMSG("ISO receive legacy context deactivated"); } break; @@ -2878,7 +2888,8 @@ static void stop_dma_rcv_ctx(struct dma_ } -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; @@ -2905,8 +2916,15 @@ static void free_dma_rcv_ctx(struct dma_ 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); } @@ -2940,7 +2958,7 @@ alloc_dma_rcv_ctx(struct ti_ohci *ohci, 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*)); @@ -2952,7 +2970,7 @@ alloc_dma_rcv_ctx(struct ti_ohci *ohci, 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*)); @@ -2962,7 +2980,7 @@ alloc_dma_rcv_ctx(struct ti_ohci *ohci, 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; } @@ -2981,7 +2999,7 @@ alloc_dma_rcv_ctx(struct ti_ohci *ohci, } 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; } @@ -2993,7 +3011,7 @@ alloc_dma_rcv_ctx(struct ti_ohci *ohci, } 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; } } @@ -3007,7 +3025,7 @@ alloc_dma_rcv_ctx(struct ti_ohci *ohci, 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; } @@ -3357,6 +3375,7 @@ static int __devinit ohci1394_pci_probe( /* 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; @@ -3379,17 +3398,32 @@ static int __devinit ohci1394_pci_probe( if (hpsb_add_host(host)) FAIL(-ENOMEM, "Failed to register host with highlevel"); + INIT_WORK(&ohci->irlc_free_dma, ohci_free_irlc_pci_pool, + ohci->ir_legacy_context.prg_pool); + 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) return; @@ -3434,15 +3468,15 @@ static void ohci1394_pci_remove(struct p /* 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); diff -puN drivers/ieee1394/ohci1394.h~ohci1394-dma_pool_destroy-while-in_atomic-irqs_disabled drivers/ieee1394/ohci1394.h --- 25/drivers/ieee1394/ohci1394.h~ohci1394-dma_pool_destroy-while-in_atomic-irqs_disabled 2005-02-04 12:51:53.000000000 -0800 +++ 25-akpm/drivers/ieee1394/ohci1394.h 2005-02-04 12:56:18.704200736 -0800 @@ -197,6 +197,8 @@ struct ti_ohci { 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 */ _