add ata scsi driver

On Mon, May 26 2003, Linus Torvalds wrote:
> > What does the block layer need, that it doesn't have now? >
> Exactly. I'd _love_ for people to really think about this.

In discussion with Jeff, it seems most of what he wants is already there. He just doesn't know it yet :-)

Maybe that's my problem as well, maybe the code / comments / doc / whatever is not clear enough.

My wishlist for this would be:

1. Unified SG segment allocation. The SCSI layer currently has a mempool implementation to cope with this, is there a reason it can't become block generic?

2. Device locality awareness. Quite a bit of the esoteric SCSI queueing code occurs because we have two type of queue events:
a. device can't accept another command---stop queue and restart when the device sends a completion back
b. the host adapter is out of resources for *all* its devices. Block all device queues until we free some resources (again, usually a returning command).

3. Perhaps some type of unified command handling. At the moment, we all seem to allocate DMA'able regions for our commands/taskfiles/whatever and attach them to reqest->special. Then we need to release them again before completing the request.

4. Same thing goes for sense buffers.

5. There needs to be some amalgam of the SCSI code for dynamic tag command queue depth handling.

OK, I'll stop now.

James

Of course that is doable, when I killed scsi_dma.c it was just a direct replacement. Given that IDE had no such dynamic sg list allocation requirements, it stayed in SCSI. Overdesign is never good :)

This should be doable.

This is harder, because it involves more than one specific queue.

Completely agree.

Again, block layer queueing was designed for what I needed (ide tcq) and no overdesign was attempted. If you describe what you need, I'd be very happy to oblige and add those bits. Some decent depth change handling, I presume?

Jens Axboe

I agree with the sentiment. I just don't think variable size SG tables will remain the exclusive province of SCSI forever.

Yes, this is our nastycase, especially for locking and ref counting...you didn't say I only had to hand off the easy problems, though...

Hotpluggin has to have some awareness of this locality too. Even for IDE, hot unplug a card and you can lose two devices per cable.

Pretty much yes, now. We lost all of our memory allocation nightmare problems when we moved away from fixed command queues per device to lazy command allocation using slabs.

James

Agree, until that becomes a problem I don't see a reason to rip it out of SCSI.

:)

I really think this should be handled in the SCSI layer. You are dealing with devices hanging off a hardware unit of some sort.

It's also possible to go too far with this whole abstraction deal. The resulting code ends up being contorted, instead of having to separate 'straight forward' cases in SCSI and XYZ (for instance).

Hmmm yes. Now we are moving into general device management, though.

Alright, so what do you need? Start out with X tags, shrink to Y (based on repeated queue full conditions)? Anything else?

Jens Axboe

Actually, it's easier than that: just an API to alter the number of tags in the block layer (really only the size of your internal hash table). The actual heuristics of when to alter the queue depth is the province of the individual drivers (although Doug Ledford was going to come up with a generic implementation).

James

That's actually what I meant, that the SCSI layer would call down into the block layer to set the size. I don't/want to know about queue full conditions.

The internal memory requirements for the queue table is small (a bit per tag), so I think we can basically get away with just decrementing ->max_depth.

Jens Axboe

James, something like this would be enough then (untested, compiles)?

===== drivers/block/ll_rw_blk.c 1.170 vs edited =====

@@ -413,11 +413,12 @@
 {
 	struct blk_queue_tag *bqt = q->queue_tags;
 
-	if (unlikely(bqt == NULL || bqt->max_depth < tag))
+	if (unlikely(bqt == NULL))
 		return NULL;
 
 	return bqt->tag_index[tag];
 }
+
 /**
  * blk_queue_free_tags - release tag maintenance info
  * @q:  the request queue for the device

@@ -448,38 +449,26 @@
 	q->queue_flags &= ~(1 << QUEUE_FLAG_QUEUED);
 }
 
-/**
- * blk_queue_init_tags - initialize the queue tag info
- * @q:  the request queue for the device
- * @depth:  the maximum queue depth supported
- **/
-int blk_queue_init_tags(request_queue_t *q, int depth)
+static int init_tag_map(struct blk_queue_tag *tags, int depth)
 {
-	struct blk_queue_tag *tags;
 	int bits, i;
 
 	if (depth > (queue_nr_requests*2)) {
 		depth = (queue_nr_requests*2);
-		printk("blk_queue_init_tags: adjusted depth to %d\n", depth);
+		printk(KERN_ERR "%s: adjusted depth to %d\n", __FUNCTION__, depth);
 	}
 
-	tags = kmalloc(sizeof(struct blk_queue_tag),GFP_ATOMIC);
-	if (!tags)
-		goto fail;
-
 	tags->tag_index = kmalloc(depth * sizeof(struct request *), GFP_ATOMIC);
 	if (!tags->tag_index)
-		goto fail_index;
+		goto fail;
 
 	bits = (depth / BLK_TAGS_PER_LONG) + 1;
 	tags->tag_map = kmalloc(bits * sizeof(unsigned long), GFP_ATOMIC);
 	if (!tags->tag_map)
-		goto fail_map;
+		goto fail;
 
 	memset(tags->tag_index, 0, depth * sizeof(struct request *));
 	memset(tags->tag_map, 0, bits * sizeof(unsigned long));
-	INIT_LIST_HEAD(&tags->busy_list);
-	tags->busy = 0;
 	tags->max_depth = depth;
 
 	/*

@@ -488,22 +477,89 @@
 	for (i = depth; i < bits * BLK_TAGS_PER_LONG; i++)
 		__set_bit(i, tags->tag_map);
 
+	return 0;
+fail:
+	kfree(tags->tag_index);
+	return -ENOMEM;
+}
+
+
+/**
+ * blk_queue_init_tags - initialize the queue tag info
+ * @q:  the request queue for the device
+ * @depth:  the maximum queue depth supported
+ **/
+int blk_queue_init_tags(request_queue_t *q, int depth)
+{
+	struct blk_queue_tag *tags;
+
+	tags = kmalloc(sizeof(struct blk_queue_tag),GFP_ATOMIC);
+	if (!tags)
+		goto fail;
+
+	if (init_tag_map(tags, depth))
+		goto fail;
+
+	INIT_LIST_HEAD(&tags->busy_list);
+	tags->busy = 0;
+
 	/*
 	 * assign it, all done
 	 */
 	q->queue_tags = tags;
 	q->queue_flags |= (1 << QUEUE_FLAG_QUEUED);
 	return 0;
-
-fail_map:
-	kfree(tags->tag_index);
-fail_index:
-	kfree(tags);
 fail:
+	kfree(tags);
 	return -ENOMEM;
 }
 
 /**
+ * blk_queue_resize_tags - change the queueing depth
+ * @q:  the request queue for the device
+ * @new_depth: the new max command queueing depth
+ *
+ *  Notes:
+ *    Must be called with the queue lock held.
+ **/
+int blk_queue_resize_tags(request_queue_t *q, int new_depth)
+{
+	struct blk_queue_tag *bqt = q->queue_tags;
+	struct request **tag_index;
+	unsigned long *tag_map;
+	int bits, max_depth;
+
+	if (!bqt)
+		return -ENXIO;
+
+	/*
+	 * don't bother sizing down
+	 */
+	if (new_depth <= bqt->max_depth) {
+		bqt->max_depth = new_depth;
+		return 0;
+	}
+
+	/*
+	 * save the old state info, so we can copy it back
+	 */
+	tag_index = bqt->tag_index;
+	tag_map = bqt->tag_map;
+	max_depth = bqt->max_depth;
+
+	if (init_tag_map(bqt, new_depth))
+		return -ENOMEM;
+
+	memcpy(bqt->tag_index, tag_index, max_depth * sizeof(struct request *));
+	bits = (max_depth / BLK_TAGS_PER_LONG) + 1;
+	memcpy(bqt->tag_map, bqt->tag_map, bits * sizeof(unsigned long));
+
+	kfree(tag_index);
+	kfree(tag_map);
+	return 0;
+}
+
+/**
  * blk_queue_end_tag - end tag operations for a request
  * @q:  the request queue for the device
  * @tag:  the tag that has completed

@@ -523,9 +579,6 @@
 	int tag = rq->tag;
 
 	BUG_ON(tag == -1);
-
-	if (unlikely(tag >= bqt->max_depth))
-		return;
 
 	if (unlikely(!__test_and_clear_bit(tag, bqt->tag_map))) {
 		printk("attempt to clear non-busy tag (%d)\n", tag);

===== include/linux/blkdev.h 1.105 vs edited =====

@@ -452,6 +452,7 @@
 extern void blk_queue_end_tag(request_queue_t *, struct request *);
 extern int blk_queue_init_tags(request_queue_t *, int);
 extern void blk_queue_free_tags(request_queue_t *);
+extern int blk_queue_resize_tags(request_queue_t *, int);
 extern void blk_queue_invalidate_tags(request_queue_t *);
 extern void blk_congestion_wait(int rw, long timeout);
 

Jens Axboe

Yes...the only concern I would have is that if you downsize the tag map, you don't seem to keep any memory of what the high water mark actually is. Thus, I can drop the depth by 8 and then increase it again by 4 and you won't see that the increase can be accommodated by the already allocated space.

James

If you increase it again, the maps are resized. Is that a problem? Seems ok to me.

Jens Axboe

What I mean is that you allocate memory whenever the depth increases. Even if you have an array large enough to accommodate the increase (because you don't release when you decrease the tag depth).

On further examination, there's also an invalid tag race: If a device is throttling, it might want to do a big decrease followed fairly quickly by a small increase. When it does the increase, you potentially still have outstanding tags above the new depth, which will now run off the end of your newly allocated tag array.

James

Yes I know what you mean, but my question is if it's worth it to keep track of? No memory is really lost, but we are doing a copy of tag map and bitmap of course in addition to the extra allocation. Given that you're not going to change depths 100 times per seconds, I don't think it's worth it to actually do anything about it. Especially since you'll quickly settle at the desired depth and stay there. But hey, I'm an accomodating guy (pfft), so here's the change just for you :)

Oh yes you are right. How does the attached look? With real_max_depth, that should work as well since we'll only ever alloc a bigger area.

===== drivers/block/ll_rw_blk.c 1.170 vs edited =====

@@ -413,11 +413,12 @@
 {
 	struct blk_queue_tag *bqt = q->queue_tags;
 
-	if (unlikely(bqt == NULL || bqt->max_depth < tag))
+	if (unlikely(bqt == NULL || tag >= bqt->real_max_depth))
 		return NULL;
 
 	return bqt->tag_index[tag];
 }
+
 /**
  * blk_queue_free_tags - release tag maintenance info
  * @q:  the request queue for the device

@@ -448,39 +449,28 @@
 	q->queue_flags &= ~(1 << QUEUE_FLAG_QUEUED);
 }
 
-/**
- * blk_queue_init_tags - initialize the queue tag info
- * @q:  the request queue for the device
- * @depth:  the maximum queue depth supported
- **/
-int blk_queue_init_tags(request_queue_t *q, int depth)
+static int init_tag_map(struct blk_queue_tag *tags, int depth)
 {
-	struct blk_queue_tag *tags;
 	int bits, i;
 
 	if (depth > (queue_nr_requests*2)) {
 		depth = (queue_nr_requests*2);
-		printk("blk_queue_init_tags: adjusted depth to %d\n", depth);
+		printk(KERN_ERR "%s: adjusted depth to %d\n", __FUNCTION__, depth);
 	}
 
-	tags = kmalloc(sizeof(struct blk_queue_tag),GFP_ATOMIC);
-	if (!tags)
-		goto fail;
-
 	tags->tag_index = kmalloc(depth * sizeof(struct request *), GFP_ATOMIC);
 	if (!tags->tag_index)
-		goto fail_index;
+		goto fail;
 
 	bits = (depth / BLK_TAGS_PER_LONG) + 1;
 	tags->tag_map = kmalloc(bits * sizeof(unsigned long), GFP_ATOMIC);
 	if (!tags->tag_map)
-		goto fail_map;
+		goto fail;
 
 	memset(tags->tag_index, 0, depth * sizeof(struct request *));
 	memset(tags->tag_map, 0, bits * sizeof(unsigned long));
-	INIT_LIST_HEAD(&tags->busy_list);
-	tags->busy = 0;
 	tags->max_depth = depth;
+	tags->real_max_depth = bits * BITS_PER_LONG;
 
 	/*
 	 * set the upper bits if the depth isn't a multiple of the word size

@@ -488,22 +478,89 @@
 	for (i = depth; i < bits * BLK_TAGS_PER_LONG; i++)
 		__set_bit(i, tags->tag_map);
 
+	return 0;
+fail:
+	kfree(tags->tag_index);
+	return -ENOMEM;
+}
+
+
+/**
+ * blk_queue_init_tags - initialize the queue tag info
+ * @q:  the request queue for the device
+ * @depth:  the maximum queue depth supported
+ **/
+int blk_queue_init_tags(request_queue_t *q, int depth)
+{
+	struct blk_queue_tag *tags;
+
+	tags = kmalloc(sizeof(struct blk_queue_tag),GFP_ATOMIC);
+	if (!tags)
+		goto fail;
+
+	if (init_tag_map(tags, depth))
+		goto fail;
+
+	INIT_LIST_HEAD(&tags->busy_list);
+	tags->busy = 0;
+
 	/*
 	 * assign it, all done
 	 */
 	q->queue_tags = tags;
 	q->queue_flags |= (1 << QUEUE_FLAG_QUEUED);
 	return 0;
-
-fail_map:
-	kfree(tags->tag_index);
-fail_index:
-	kfree(tags);
 fail:
+	kfree(tags);
 	return -ENOMEM;
 }
 
 /**
+ * blk_queue_resize_tags - change the queueing depth
+ * @q:  the request queue for the device
+ * @new_depth: the new max command queueing depth
+ *
+ *  Notes:
+ *    Must be called with the queue lock held.
+ **/
+int blk_queue_resize_tags(request_queue_t *q, int new_depth)
+{
+	struct blk_queue_tag *bqt = q->queue_tags;
+	struct request **tag_index;
+	unsigned long *tag_map;
+	int bits, max_depth;
+
+	if (!bqt)
+		return -ENXIO;
+
+	/*
+	 * don't bother sizing down
+	 */
+	if (new_depth <= bqt->real_max_depth) {
+		bqt->max_depth = new_depth;
+		return 0;
+	}
+
+	/*
+	 * save the old state info, so we can copy it back
+	 */
+	tag_index = bqt->tag_index;
+	tag_map = bqt->tag_map;
+	max_depth = bqt->real_max_depth;
+
+	if (init_tag_map(bqt, new_depth))
+		return -ENOMEM;
+
+	memcpy(bqt->tag_index, tag_index, max_depth * sizeof(struct request *));
+	bits = max_depth / BLK_TAGS_PER_LONG;
+	memcpy(bqt->tag_map, bqt->tag_map, bits * sizeof(unsigned long));
+
+	kfree(tag_index);
+	kfree(tag_map);
+	return 0;
+}
+
+/**
  * blk_queue_end_tag - end tag operations for a request
  * @q:  the request queue for the device
  * @tag:  the tag that has completed

@@ -524,7 +581,7 @@
 
 	BUG_ON(tag == -1);
 
-	if (unlikely(tag >= bqt->max_depth))
+	if (unlikely(tag >= bqt->real_max_depth))
 		return;
 
 	if (unlikely(!__test_and_clear_bit(tag, bqt->tag_map))) {

===== include/linux/blkdev.h 1.105 vs edited =====

@@ -179,7 +179,8 @@
 	unsigned long *tag_map;		/* bit map of free/busy tags */
 	struct list_head busy_list;	/* fifo list of busy tags */
 	int busy;			/* current depth */
-	int max_depth;
+	int max_depth;			/* what we will send to device */
+	int real_max_depth;		/* what the array can hold */
 };
 
 struct request_queue

@@ -452,6 +453,7 @@
 extern void blk_queue_end_tag(request_queue_t *, struct request *);
 extern int blk_queue_init_tags(request_queue_t *, int);
 extern void blk_queue_free_tags(request_queue_t *);
+extern int blk_queue_resize_tags(request_queue_t *, int);
 extern void blk_queue_invalidate_tags(request_queue_t *);
 extern void blk_congestion_wait(int rw, long timeout);
 

Jens Axboe

That looks perfect. You submit the block layer changes, I'll plumb it into SCSI and we can try it out....just as soon as I convert the one SCSI driver that uses the block queue tags to do dynamic queue adjustment...

James

Talking about tagged queueing - does the SCSI layer still remove the request from the request list when it starts executing it?

At least historically that's a major mistake, and generates a crappy elevator, because it removes information from the block layer about where the disk is (or is going to be).

I know Andrew thinks that SCSI tagged queuing is a bunch of crap, and he has the latency numbers to prove it. He blames the SCSI disks themselves, but I think it might be the fact that SCSI makes it impossible to make a fair queuing algorithm for higher levels by hiding information.

Has anybody looked at just removing the request at command _completion_ time instead? That's what IDE does, and it's the _right_ thing to do.

I'd hate for SATA to pick up these kinds of mistakes from the SCSI layer.

Linus

That's a block layer requirement (blk_queue_start_tag does the dequeue). But yes, for untagged requests, we still dequeue them manually.

Well...I could do it, but since it only applies to untagged devices (which is really tapes and some CD-ROMs nowadays), I'm not sure it would be worth the effort.

The elevator is based on linear head movements. I'm not sure its optimal to figure out a way to leave all the executing tagged requests in the queue, is it? They can be spread out all over the disc with no idea which one the disc is currently executing.

James

Historically, yes.

But we've been moving more and more towards a latency-based elevator, or at least one that takes latency into account. Which is exactly why you'd like to leave unfinished requests on the queue, because otherwise your queue doesn't really show what is really going on.

In particular, while the higher layers don't actually _do_ this yet, from a latency standpoint the right thing to do when some request seems to get starved is to refuse to feed more tagged requeusts to the device until the starved request has finished.

As I mentioned, Andrew actually had some really bad latency numbers to prove that this is a real issue. SCSI with more than 4 tags or so results in potentially _major_ starvation, because the disks themselves are not even trying to avoid it.

Also, even aside from the starvation issue with unfair disks, just from a "linear head movement" standpoint, you really want to sort the queue according to what is going on _now_ in the disk. If the disk eats the requests immediately (but doesn't execute them immediately), the sorting has nothing to go on, and you get tons of back-and-forth movements.

Basically, if you're trying to do an elevator, you need to know what the outstanding commands are. Think it through on paper, and you'll see what I mean.

Linus

The newer io schedulers divide the queue into a front and backend, so it's not exactly trivial to just 'leave it on the queue'. Which queue?

You know which ones are pending, they are on the busy queue. You can look there.

Agree

Basically everyone agrees that this shouldn't happen in newer disks, I'm inclined to think we are seeing internal queueing bugs in the SCSI drivers or SCSI layer itself.

Who said we are using an elevator? News flash, we haven't used an elevator design for a long time.

Jens Axboe

OK, number seven on the list was going to be thinking about tracking timeouts at the block layer.

Tag starvation handling is a property of each individual SCSI HBA (and I know it shouldn't be). Some do a good job, some don't. However, it's implicitly also handled in the SCSI error handler because on timeout we quiesce the device (which finally causes the starved tag to execute). I have thought about doing it properly in the error handler, but it's a radical divergence---you basically need to begin to throttle the queue, but wait to see if your starved tag gets serviced and then increase the feed again (as opposed to our current quiesce then handle model).

Tag starvation isn't really a problem for the majority of modern SCSI drives (big iron vendors yelled and kicked about this a while ago). You can still find some of the older drives that do have these issues (I keep several for the purposes of exciting the error handling).

Most scsi discs have a non linear geometry anyway, so "head scheduling" is pretty useless to them. All SCSI really wants is I/O consolidation. i.e. we'd like to use fewer larger sized requests.

James

Yes

Not necessarily, the io schedulers keep track of where we are going. You don't need an active front request for that.

I know this is a pet peeve of yours (must be, I remember you bringing it up at least 3 time before :), but I don't think that's necessarily true. It shouldn't matter _one_ bit whether you leave the request there or not, it's unmergeable. As long as the io scheduler keeps track of this (and it does!) we are golden.

Jens Axboe

It's not the merging that I worry about. It's latency and starvation.

Think of it this way: if you keep feeding a disk requests, and the disk always tries to do the closest one (which is a likely algorithm), you can easily have a situation where the disk _never_ actually schedules a request that is at one "end" of the platter.

Think of all the fairness issues we've had in the elevator code, and realize that the low-level disk probably implements _none_ of those fairness algorithms.

Hmm.. Where does it keep track of request latency for requests that have been removed from the queue?

Linus

Then you have a bad disk, period. If the disks always tries to approximate SPTF internally, then it's a bad design. Apparently that Other OS times read/write requests out after 3 seconds, we we at least know we are getting service in that time frame. Not saying that is good enough, just a data point.

But the situation you describe above can easily be fixed, you described the solution yourself in the previous mail. The silly tag depth is a problem in itself, it should not be done. Keeping a sane number of tags just to keep the disk busy, and we can use the "don't queue more requests before X finishes, because X has been waiting for Y ms" tactic.

In fact, considering folks want to make error handling for command timeouts a block property (that I agree with, we are already going there with the SG_IO stuff), we can soft timeout a command if need be and handle the case from there. What do you think?

I think it does, to some extent at least.

Well, it doesn't...

Jens Axboe

I doubt they do a very good job of it. I know of bad cases, even with "high-end" hardware. Sure, we can hope that it's getting better, but do we want to bet on it.

Yeah. Which means that right now _really_ long starvation will show up as timeouts, while other cases will just show up as bad latency.

Which will _work_, of course (assuming the timeout handling is correct, which is a big if in itself), but it still sucks from a usability standpoint.

Even if we drop our timeouts from 30 seconds (or whatever they are now) down to just a few seconds, that's a _loooong_ time, and we should be a lot more proactive about things. Audio/video stuff tends to want things with latencies in the tenth-of-a-second range, even when they buffer things up internally to hide the worst cases.

Linus

There is an elevator notifier which is called on request completion in Andrew's tree (needed for AS io scheduler). This can be used to do what you want.

Well, yeah, sure, you can use it to keep track of outstanding requests. But wouldn't it be nicer to see them in the first place?

The question being: how do you sanely avoid adding more requests to the queue if you start seeing starvation? Or if adding more requests, at least using an ordered tag or a barrier or whatever to make sure that you tell the disk that the new request must not be done before the old one has finally been satisfied?

I think you'd want to see the old requests in order to be able to make that decision reasonably well.

This is clearly not a 2.6.x issue, btw.

Linus

Yeah. Basically the driver will call:
elv_next_request, elv_remove_request, elv_completed_request

elv_remove_request can easily just move the request to another list, which is removed by elv_completed_request.

Don't let the names bother you, the elevator (in Andrew's tree) gets all the information it needs.

Many IDE drives will take several seconds to give up on a failing I/O because they do all the recovery themselves. IDE CD/DVD can be especially bad for this.

No I agree, it's always nice to handle these cases in software so we don't have to rely on these things getting fixed.

Here's something ridicolously simple, that just wont start a new tag if the oldest tag is older than 100ms. Clearly nothing for submission, but it gets the point across.

Now only look at reads, and we've got something a little useful at least.

James, speaking of queue localities and tcq... Doug mentioned some time ago that aic7xxx dishes out tags numbers from a hba pool which makes it impossible to support with out current block layer queueing code. Maybe it we associate the blk_queue_tag structure with a bunch of queues instead of having a 1:1 mapping it could work.

===== drivers/block/ll_rw_blk.c 1.170 vs edited =====

@@ -574,6 +574,13 @@
 		BUG();
 	}
 
+	if (!list_empty(&bqt->busy_list)) {
+		struct request *__rq = list_entry_rq(bqt->busy_list.prev);
+
+		if (time_after(rq->timeout, jiffies))
+			return 1;
+	}
+
 	for (map = bqt->tag_map; *map == -1UL; map++) {
 		tag += BLK_TAGS_PER_LONG;
 

@@ -589,6 +596,7 @@
 	bqt->tag_index[tag] = rq;
 	blkdev_dequeue_request(rq);
 	list_add(&rq->queuelist, &bqt->busy_list);
+	rq->timeout = jiffies + HZ / 10;
 	bqt->busy++;
 	return 0;
 }

Jens Axboe

Yes, A large number of devices (not just the aic7xxx) have a single issue queue for all outgoing requests (this is the reason for the can_queue limit in the host template).

If the issue queue is < 256 slots, it makes a lot of sense to use global tag numbers instead of device local ones, since then you can simply map the returning tag number to an index in the issue queue and not bother having to keep a hash table of <pun, lun, tag> to look it up in.

Even drivers which have larger or variable size issue queues (here the qlogic ones spring immediately to mind) often have a virtual index number attached to their commands (which, this time is 16 bits) which they'd like to use as a unique offset into the issue queue. Obviously, they face exactly the same challenges as tags and they'd like a similar solution.

A global structure for a bunch of queues would probably be the most useful way forwards.

James

Yes, I think something like this should work very well.

In fact, it might fit in very well indeed with AS - and in general it might be a good idea to have some nice interface for the IO scheduler to give this kind of ordering hints down to the hardware.

Linus

On Tue, 2003-05-27 at 10:36, Linus Torvalds wrote:
Btw, in case you wonder why I care about names and organization, it's because with the names and organization comes assumptions and expectations.

One prime example of this is cdrecord, and the incredible braindamage that the name "SCSI" foisted upon it. Why? Because everybody (ie schily) _knows_ that SCSI is addressed by bus/id/lun, and thinks that anything else is wrong. So you have total idiocies like the "cdrecord -scanbus" crap for finding your device, and totally useless naming that makes no sense in any sane environment.

Calling something SCSI when it isn't brings on these kinds of bad things:
people make assuptions that aren't sensible or desireable.

Names have power. There's baggage and assumptions in a name. In the case of SCSI, there is a _lot_ of baggage.

I took this one on board a long time ago. Even in the SCSI world, FC devices don't think in terms of PUN, they think in terms of WWN.

If you look at the mid layer (and I don't promise this to be complete yet) we're moving away from referring to things by host/channel/id/lun. Now we just have host/device list in most places. About the only place we convert back to the numbers is to print messages.

We're certainly not there yet, since we need to support legacy interfaces like /proc/scsi/scsi. But eventually you'll probably see us using the sysfs name instead of the id (FC devices will probably stuff WWNs in here, other things may use numbers) and lun (not sure how we'll represent SCSI-3 LUN hierarchies yet). Hopefully, it will be possible to make the mid layer entirely unaware of any id/lun distinction so it could be configured for a flatter host/device space instead.

James

ATA defines WWN too... I'm curious what the format is? uuid-ish?

Jeff

Heh, you'll regret asking that one. There are currently seven possible identifier formats for the WWN (which is basically the SCSI Device Identification VPD page).

The extremely gory details are in the SPC-3 spec (section 7.6.4 of r12).

However, really, as far as Linux is concerned we don't need to care. It just needs to be reducible to an ASCII representation and dumped into the sysfs name or embedded into bus_id. The reduction to ASCII can be subsystem (or even device driver) specific.

However, note that I'm not thinking of forcing the WWN here. I'm thinking of using whatever makes most sense to the device, so SPI devices will continue to use simple target/lun numbers here. FC devices will probably want to use their variant of WWN/PortID. The rationale is to get rid of the unnecessary internal mappings some drivers use to get to the physical address they send on the wire.

Thus, if you never address ATA devices by the WWN, you probably never want to make it part of the addressing scheme.

Exporting a unique ID for userspace to use is a different (and probably orthogonal) issue.

James

Oh, no question. My main interest is having a persistent id for a device's media. Then Linux can use that to allow mapping in case device names or majors change at each boot (and similar situations).

Jeff

Well, OK, that's not an in-kernel issue.

My current thought for this is that deriving the unique name can be awfully device specific (even in SCSI there are several fallback methods plus the usual black/white list of things that don't quite return entirely unique objects). Thus, I think the derivation probably belongs in userspace as part of hotplug. Once the unique ID is determined, it could be written to a well known place in the sysfs tree for all the rest of the OS (things like udev for persistent device naming) to use.

James

It's definitely an in-kernel issue, because the mapping is in-kernel now:

	<major,minor> -> queue

Agreed. And kernel assistance is likely necessary in many cases to obtain the unique id, even if it's only userspace that's reading it.

Agreed, with the above proviso.

Jeff

The mapping in the kernel is gendisk -> queue. On the syscall boundary we have an addition dev_t -> gendisk mapping. The scsi midlayer e.g. doesn't care about dev_t or even major / minor at all.

FC uses a 64-bit identifier for a WWN.

you end up with things like "50:00:0e:10:00:00:01:1c". ironically, FC switching doesn't actually 'use' the WWN but a 24-bit FC_ID.

for things like mappings -- who knows -- perhaps WWN is good -- perhaps something like a serial # is also good -- perhaps both are good?

        mel-stglab-mds9509-1# show scsi-target lun vsan 1

        - DF350F from HITACHI (Rev 0000)
          FCID is 0x7f0002 in VSAN 1, PWWN is 50:00:0e:10:00:00:01:1c
          ------------------------------------------------------------------------------
          LUN    Capacity  Status  Serial Number    Device-Id
                 (MB)
          ------------------------------------------------------------------------------
          0x0    16651     Online                   C:2 A:0 T:1 HITACHI 
D35067950000
          0x1    16651     Online                   C:2 A:0 T:1 HITACHI 

D35067950001

        - ST318452FC from HP (Rev HP03)
          FCID is 0x7f01da in VSAN 1, PWWN is 22:00:00:04:cf:8c:1f:95
          ------------------------------------------------------------------------------
          LUN    Capacity  Status  Serial Number    Device-Id
                 (MB)
          ------------------------------------------------------------------------------
          0x0    17920     Online  3EV0JVQJ         C:1 A:0 T:3 
20:00:00:04:cf:8c:1f:95
        [..]

cheers,

lincoln.

Agree, it should take the edge of crappy hardware at least.

And deadline, they share the same request expire mechanism. But I read your hint, I'll add the hint and fix this for real. Was waiting for the other tcq patch to be comitted as they overlap, but I see that is in so...

Jens Axboe