[lkml]   [2010]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 2/2] scsi: don't use execute_in_process_context()
    On Wed, 2010-12-15 at 20:19 +0100, Tejun Heo wrote:
    > Hello,
    > On 12/15/2010 08:10 PM, James Bottomley wrote:
    > >> Yes, it would do, but we were already too far with the existing
    > >> implementation and I don't agree we need more when replacing it with
    > >> usual workqueue usage would remove the issue. So, when we actually
    > >> need them, let's consider that or any other way to do it, please.
    > >> A core API with only a few users which can be easily replaced isn't
    > >> really worth keeping around. Wouldn't you agree?
    > >
    > > Not really ... since the fix is small and obvious.
    > IMHO, it's a bit too subtle to be a good API. The callee is called
    > under different (locking) context depending on the callsite and I've
    > been already bitten enough times from implicit THIS_MODULEs. Both
    > properties increase possbility of introducing problems which can be
    > quite difficult to detect and reproduce.

    Both have subtleties ... see below.

    > > Plus now it can't be moved into SCSI because I need the unremovable
    > > call chain.
    > Yes, with the proposed change, it cannot be moved to SCSI.
    > > Show me how you propose to fix it differently first, since we both agree
    > > the initial attempt doesn't work, and we can take the discussion from
    > > there.
    > Given that the structures containing the work items are dynamically
    > allocated, I would introduce a scsi_wq, unconditionally schedule
    > release works on them and flush them before unloading. Please note
    > that workqueues no longer require dedicated threads, so it's quite
    > cheap.

    A single flush won't quite work. The target is a parent of the device,
    both of which release methods have execute_in_process_context()
    requirements. What can happen here is that the last put of the device
    will release the target (from the function). If both are moved to
    workqueues, a single flush could cause the execution of the device work,
    which then queues up target work (and makes it still pending). A double
    flush will solve this (because I think our nesting level doesn't go
    beyond 2) but it's a bit ugly ...

    execute_in_process_context() doesn't have this problem because the first
    call automatically executes the second inline (because it now has


     \ /
      Last update: 2010-12-15 20:35    [W:0.039 / U:0.512 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site