On Mon, Apr 08 2002, Martin Dalecki wrote:
> Jens Axboe wrote:
> >Hi,
> >
> >I've implemented tagged command queueing for ATA disk drives, and it's
> >now ready for people to give it a test spin. As it has had only limited
> >testing so far, please be very careful with it. It has been tested on
> >two drives so far, a GXP75-30gb and a GXP120-40gb, and with a PIIX4
> >controller:
>
> OK after a cursory look I see that the patch contains quite
> a lot of ideas for the generic code itself. Do you think that it would
> be worth wile to extract them first or should the patch be just included
> in mainline. (I don't intent to interferre too much with your efforts to
> do something similar in 2.4.xx.)....
Good question, I've asked myself that too... Yeah I see some of my ideas
as being nice to have in mainline even without TCQ. The big one being
ata_request_t of course, there are some parts to this:
- Separate scatterlist and dma table out from hwgroup. This is not
really needed for TCQ, but saves doing a blk_rq_map_sg on a request
more than once. If future ATA hardware would support more than one
pending DMA operation per hwgroup, this would be useful even without
TCQ.
- Use ata_request_t as the main request command. This is where I really
want to go. I'm not saying that we need a complete IDE mid layer, but
a private request type is a nice way to unify the passing of a general
command around. So the taskfile stuff would remain very low level,
ata_request would add the higher level parts. I could expand lots more
on this, but I'm quite sure you know where I'm going :-)
Note that the ata_request_t usage is a bit messy in the current patch,
that's merely because I was more focused on getting TCQ stable than
designing this out right now. So I think we should let it mature in the
TCQ patch for just a while before making any final commitments. Agreed?
Of course this will leave me with the pain of merging with your IDE
stuff every time a new -pre comes out (updating this patch from
2.5.1-pre where I last used it was _not_ funny! :-), but I can handle
that.
In addition, there are small buglet fixes in the patch that should go to
general. I will extract these, I already send you one of these earlier
today.
BTW, I just found an SMP race in the current patch. I'll send out a new
version later, for now it's here:
--- ../../linux-2.5.8-pre2/drivers/ide/ide.c Mon Apr 8 14:53:06 2002
+++ drivers/ide/ide.c Mon Apr 8 14:40:33 2002
@@ -1373,8 +1373,17 @@
break;
}
- if (blk_queue_plugged(&drive->queue))
- BUG();
+ /*
+ * there's a small window between where the queue could be
+ * replugged while we are in here when using tcq (in which
+ * case the queue is probably empty anyways...), so check
+ * and leave if appropriate. When not using tqc, this is
+ * still a severe BUG!
+ */
+ if (blk_queue_plugged(&drive->queue)) {
+ BUG_ON(!drive->using_tcq);
+ break;
+ }
/*
* just continuing an interrupted request maybe
-- 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/
This archive was generated by hypermail 2b29 : Mon Apr 15 2002 - 22:00:10 EST