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

From: Parag Warudkar (kernel-stuff_at_comcast.net)
Date: 01/30/05

  • Next message: Sam Ravnborg: "Re: kbuild: shorthand ym2y, ym2m etc" 
    Date:	Sun, 30 Jan 2005 17:49:28 -0500
To: Andrew Morton <akpm@osdl.org>

    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
    -
    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/

  • Next message: Sam Ravnborg: "Re: kbuild: shorthand ym2y, ym2m etc"

    Relevant Pages