lkml.org 
[lkml]   [2002]   [Jul]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] 2.5.28 small REQ_SPECIAL abstraction
    On Sun, Jul 28 2002, Linus Torvalds wrote:
    >
    >
    > On Mon, 29 Jul 2002, Jens Axboe wrote:
    > > >
    > > > I think you are missing the point. The stuff should not be in the
    > > > _generic_ blk_insert_request(). As I posted in my first reply to Martin,
    > > > SCSI needs to clear the tag before calling blk_insert_request() if it
    > > > needs to.
    > >
    > > Here's the patch to fix it, btw. Linus, please apply.
    >
    > I can't apply this while I think it looks horribly broken.
    >
    > Your patch makes scsi_lib call blk_queue_end_tag() without holding the
    > queue spinlock.
    >
    > Which looks horribly broken, since it _will_ corrupt the queues.
    >
    > In fact, it looks about a million times more broken than the code that
    > Martin submitted.
    >
    > Please explain to me why I am wrong.

    Duh yes, the locking is broken of course :/

    Ok I'd rather just make a __blk_insert_request as well, and have SCSI
    grab the lock itself.

    > Linus
    >
    > PS. I actually do tend to look as the patches I apply. Right now it looks
    > like you're wrong, and Martin is right.

    I think Martin's was wrong in concept, mine was wrong in implementation.

    # This is a BitKeeper generated patch for the following project:
    # Project Name: Linux kernel tree
    # This patch format is intended for GNU patch command version 2.5 or higher.
    # This patch includes the following deltas:
    # ChangeSet 1.509 -> 1.510
    # drivers/block/ll_rw_blk.c 1.96 -> 1.97
    # drivers/scsi/scsi_lib.c 1.29 -> 1.30
    #
    # The following is the BitKeeper ChangeSet Log
    # --------------------------------------------
    # 02/07/29 axboe@burns.home.kernel.dk 1.510
    # blk_insert_request + REQ_QUEUED fixed
    # --------------------------------------------
    #
    diff -Nru a/drivers/block/ll_rw_blk.c b/drivers/block/ll_rw_blk.c
    --- a/drivers/block/ll_rw_blk.c Mon Jul 29 08:33:16 2002
    +++ b/drivers/block/ll_rw_blk.c Mon Jul 29 08:33:16 2002
    @@ -1233,6 +1233,23 @@
    return rq;
    }

    +void __blk_insert_request(request_queue_t *q, struct request *rq,
    + int at_head, void *data)
    +{
    + /*
    + * tell I/O scheduler that this isn't a regular read/write (ie it
    + * must not attempt merges on this) and that it acts as a soft
    + * barrier
    + */
    + rq->flags &= REQ_QUEUED;
    + rq->flags |= REQ_SPECIAL | REQ_BARRIER;
    +
    + rq->special = data;
    +
    + _elv_add_request(q, rq, !at_head, 0);
    + q->request_fn(q);
    +}
    +
    /**
    * blk_insert_request - insert a special request in to a request queue
    * @q: request queue where request should be inserted
    @@ -1253,24 +1270,11 @@
    * host that is unable to accept a particular command.
    */
    void blk_insert_request(request_queue_t *q, struct request *rq,
    - int at_head, void *data)
    + int at_head, void *data)
    {
    unsigned long flags;

    - /*
    - * tell I/O scheduler that this isn't a regular read/write (ie it
    - * must not attempt merges on this) and that it acts as a soft
    - * barrier
    - */
    - rq->flags &= REQ_QUEUED;
    - rq->flags |= REQ_SPECIAL | REQ_BARRIER;
    -
    - rq->special = data;
    -
    spin_lock_irqsave(q->queue_lock, flags);
    - /* If command is tagged, release the tag */
    - if(blk_rq_tagged(rq))
    - blk_queue_end_tag(q, rq);
    _elv_add_request(q, rq, !at_head, 0);
    q->request_fn(q);
    spin_unlock_irqrestore(q->queue_lock, flags);
    @@ -2125,6 +2129,7 @@
    EXPORT_SYMBOL(blk_get_request);
    EXPORT_SYMBOL(__blk_get_request);
    EXPORT_SYMBOL(blk_put_request);
    +EXPORT_SYMBOL(__blk_insert_request);
    EXPORT_SYMBOL(blk_insert_request);

    EXPORT_SYMBOL(blk_queue_prep_rq);
    diff -Nru a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
    --- a/drivers/scsi/scsi_lib.c Mon Jul 29 08:33:16 2002
    +++ b/drivers/scsi/scsi_lib.c Mon Jul 29 08:33:16 2002
    @@ -73,8 +73,15 @@
    int scsi_insert_special_cmd(Scsi_Cmnd * SCpnt, int at_head)
    {
    request_queue_t *q = &SCpnt->device->request_queue;
    + unsigned long flags;

    - blk_insert_request(q, SCpnt->request, at_head, SCpnt);
    + spin_lock_irqsave(q->queue_lock, flags);
    +
    + if (blk_rq_tagged(SCpnt->request))
    + blk_queue_end_tag(q, SCpnt->request);
    +
    + __blk_insert_request(q, SCpnt->request, at_head, SCpnt);
    + spin_unlock_irqrestore(q->queue_lock, flags);
    return 0;
    }

    @@ -101,8 +108,15 @@
    int scsi_insert_special_req(Scsi_Request * SRpnt, int at_head)
    {
    request_queue_t *q = &SRpnt->sr_device->request_queue;
    + unsigned long flags;
    +
    + spin_lock_irqsave(q->queue_lock, flags);
    +
    + if (blk_rq_tagged(SRpnt->sr_request))
    + blk_queue_end_tag(q, SRpnt->sr_request);

    - blk_insert_request(q, SRpnt->sr_request, at_head, SRpnt);
    + __blk_insert_request(q, SRpnt->sr_request, at_head, SRpnt);
    + spin_unlock_irqrestore(q->queue_lock, flags);
    return 0;
    }

    --
    Jens Axboe

    -
    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-03-22 13:27    [W:0.046 / U:0.072 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site