| title: | Re PATCH 4 5 dm implement REQ FLUSH FUA s |
|
Hello,
On 08/30/2010 03:28 PM, Mike Snitzer wrote:
+ clone- cmd = rq- cmd;
+ clone- cmd_len = rq- cmd_len;
+ clone- sense = rq- sense;
+ clone- buffer = rq- buffer;
clone- end_io = end_clone_request;
clone- end_io_data = tio;
blk_rq_prep_clone() of a REQ_FLUSH request will result in a
rq_data_dir(clone) of read.
Hmmm... why? blk_rq_prep_clone() copies all REQ_* flags in
REQ_CLONE_MASK and REQ_WRITE is definitely there. Ill check.
I still had the following:
if (rq- cmd_flags & REQ_FLUSH) {
blk_rq_init(NULL, clone);
clone- cmd_type = REQ_TYPE_FS;
/* without this the clone has a rq_data_dir of 0 */
clone- cmd_flags |= WRITE_FLUSH;
} else {
r = blk_rq_prep_clone(clone, rq, tio- md- bs, GFP_ATOMIC,
dm_rq_bio_constructor, tio);
...
Request-based DMs REQ_FLUSH still works without this special casing but
I figured Id raise this to ask: what is the proper rq_data_dir() is for
a REQ_FLUSH?
Technically block layer doesnt care one way or the other but WRITE
definitely. Maybe it would be a good idea to enforce that from block
layer.
@@ -1709,15 +1621,12 @@ static void dm_request_fn(struct request_queue *q)
if (!rq)
goto plug_and_out;
- if (unlikely(dm_rq_is_flush_request(rq))) {
- BUG_ON(md- flush_request);
- md- flush_request = rq;
- blk_start_request(rq);
- queue_work(md- wq, &md- barrier_work);
- goto out;
- }
+ /* always use block 0 to find the target for flushes for now */
+ pos = 0;
+ if (!(rq- cmd_flags & REQ_FLUSH))
+ pos = blk_rq_pos(rq);
- ti = dm_table_find_target(map, blk_rq_pos(rq));
+ ti = dm_table_find_target(map, pos);
I added the following here: BUG_ON(!dm_target_is_valid(ti));
Ill add it.
if (ti- type- busy && ti- type- busy(ti))
goto plug_and_out;
I also needed to avoid the - busy call for REQ_FLUSH:
if (!(rq- cmd_flags & REQ_FLUSH)) {
ti = dm_table_find_target(map, blk_rq_pos(rq));
BUG_ON(!dm_target_is_valid(ti));
if (ti- type- busy && ti- type- busy(ti))
goto plug_and_out;
} else {
/* rq-based only ever has one target! leverage this for FLUSH */
ti = dm_table_get_target(map, 0);
}
If I allowed - busy to be called for REQ_FLUSH it would result in a
deadlock. I havent identified where/why yet.
Ah... thats probably from "if (!elv_queue_empty(q))" check below,
flushes are on a separate queue but I forgot to update
elv_queue_empty() to check the flush queue. elv_queue_empty() can
return %true spuriously in which case the queue wont be plugged and
restarted later leading to queue hang. Ill fix elv_queue_empty().
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at rel="nofollow" vger.kernel.org/majordomo-info.html vger.kernel.org/majordomo-info.html
|
