[lkml]   [2006]   [Apr]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] sbp2: fix spinlock recursion
    Andrew Morton wrote:
    > Stefan Richter <> wrote:
    >>@@ -2540,6 +2537,7 @@ static int sbp2scsi_abort(struct scsi_cm
    >> command->Current_done(command->Current_SCpnt);
    >> }
    >> }
    >> + spin_unlock_irqrestore(&scsi_id->sbp2_command_orb_lock, flags);
    > This changes the call environment for all implementations of
    > ->Current_done(). Are they all safe to call under this lock?

    Short answer: Yes, trust me. ;-) Long answer:

    The done() callbacks are passed on to sbp2 from the SCSI stack along
    with each SCSI command via the queuecommand hook. The done() callback is
    safe to call in atomic context. So does
    Documentation/scsi/scsi_mid_low_api.txt say, and many if not all SCSI
    low-level handlers rely on this fact. So whatever this callback does, it
    is "self-contained" and it won't conflict with sbp2's internal ORB list
    handling. In particular, it won't race with the sbp2_command_orb_lock.

    Moreover, sbp2 already calls the done() handler with
    sbp2_command_orb_lock taken in sbp2scsi_complete_all_commands(). I admit
    this is ultimately no proof of correctness, especially since this
    portion of code introduced the spinlock recursion in the first place and
    we didn't realize it since this code's submission before 2.6.15 until
    now. (I have learned a lesson from this.)

    I stress-tested my patch on x86 uniprocessor with a preemptible SMP
    kernel (alas I have no SMP machine yet) and made sure that all code
    paths which involve the sbp2_command_orb_lock were gone through multiple
    times. Which is of course also no proof.
    Stefan Richter
    -=====-=-==- -=-- ---=-
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2006-04-03 14:37    [W:0.028 / U:2.428 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site