lkml.org 
[lkml]   [2005]   [Nov]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] blk: fix dangling pointer access in __elv_add_request
    Jens Axboe wrote:
    > On Tue, Nov 01 2005, Tejun Heo wrote:
    >
    >>cfq's add_req_fn callback may invoke q->request_fn directly and
    >>depending on low-level driver used and timing, a queued request may be
    >>finished & deallocated before add_req_fn callback returns. So,
    >>__elv_add_request must not access rq after it's passed to add_req_fn
    >>callback.
    >
    >
    > It's a generel problem, you may get the queue run at any time regardless
    > of what the io scheduler is doing. CFQ does run the queue manully
    > sometimes, but SCSI may do the very same thing for you as well. Given
    > that SCSI also shortly reenables interrupts in the ->request_fn()
    > handling, it's quite possible for the request to be completed.
    >
    > So, as we don't hold a reference to the request, I'd say your patch
    > looks correct and should be applied right away.
    >
    >
    >>Jens, does generalizing queue kicking functions and disallowing
    >>ioscheds from directly calling q->request_fn sound like a good idea?
    >
    >
    > Yes certainly.
    >

    The thing is that we are holding queue_lock before calling add_req_fn
    callback and also after it finishes giving it an appearance of
    atomicity. I think q->request_fn semantics is peculiar and a bit prone
    to bug, so it might be better to make ioscheds always use generic queue
    kicking function which always uses work queue to run q->request_fn so
    that we don't have queue_lock releasing and regrabbing inbetween. Do
    you think there can be any noticieable performance issues?

    Hmmmm... One more thing about q->request_fn's locking behavior is that,
    as I noted while posting the ordered patchset, for SCSI, the behavior
    can reorder issued requests making it impossible to use ordered tags for
    flushing. I'm thinking of submitting a patch to make scsi request_fn
    atomic w.r.t. queue_lock, but there might be some performance issues I'm
    not aware of. Functions which release and regrab locks underneath the
    caller are just... hard. :-p

    Thanks.

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

    \
     
     \ /
      Last update: 2005-11-01 11:15    [W:0.025 / U:33.280 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site