Re: [PATCH linux-2.6-block:post-2.6.15 09/10] blk: add FUA supportto IDE

From: Tejun Heo
Date: Thu Nov 24 2005 - 07:21:37 EST



Oops, I was delusional again.

Tejun Heo wrote:

Well, this one is quite a pain in the ass.

I'm not very fond of ->rq_select_barrier() approach for the following reasons.

* That removes possibility of correct synchronization. With blk_queue_ordered() approach, we can later add blk_queue_[un]lock_ordered() to achieve correct synchronization if that becomes necessary, but with ->rq_select_barrier() approach, the low-level driver ends up having less control over what's gonna happen when.

Of course, we can do the same lock/unlock dance with ->rq_select_barrier() approach. I wasn't thinking straight. Forget this rationale.


* Changing ordered mode is not supposed to be a frequent operation and the blk_queue_ordered() interface makes that explicit.

So, I added ide_driver_t->protocol_changed() callback which gets called whenever dma/multimode changes occur. Unfortunately, dma/multmode changes can be committed with or without context, and with or without queue lock. As blk_queue_ordered uses the queue lock for synchronization, this becomes issue.

I tried to distinguish places where the changes occur while queue lock is held from the other. Not only was it highly error-prone, it couldn't be done without modifying/auditing all low-level drivers as some drivers (cs5520) use the same function which touches dma setting (cs5520_tune_chipset) from both ->speedproc (called with queuelock) and ->ide_dma_check (called without queuelock).

One alternative I'm thinking of is using a workqueue to call blk_queue_ordered, such that we don't have to guess whether or not we're called with queuelock held. Unfortunately, this will give us a small window where wrong barrier requests can hit the drive.

One thing I wanna add here is that using ->rq_select_barrier() would have similar race window. The race windows is just hidden there in the request queue.


Bartlomiej, any ideas?

Jens, as this one seems to need some time to settle, I'm gonna post updated patchset for post-2.6.15 without ide-fua patch, so that the other stuff can be pushed into -mm. I think we can live without ide-fua for a while. :-)

Thanks.



--
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/