On Tue, Jan 14, 2014 at 03:17:32PM -0500, Martin K. Petersen wrote: > >>>>> "Kent" == Kent Overstreet <k...@daterainc.com> writes: > > >> IOW, DISCARD, WRITE SAME and the impending COPY requests do not have > >> a 1:1 mapping between the block range worked on and the size of any > >> bvecs attached. Your recent changes must have changed the way we > >> handled that in the past. > > Kent> Yeah - but with WRITE_SAME bios, wouldn't we at least have to > Kent> check that they're writing the same data to merge them? > > We do. Check blk_write_same_mergeable().
Aha, I missed that. Side note, one of the things on my todo list/wishlist is make a separate enum for bio type - bare flush, normal read/write, scsi command, discard and write same. In particular with bi_size meaning different things depending on the type of the command, I think having an enum we can switch off of where appropriate instead of a bunch of random flags will make things a hell of a lot saner. Looking more at this code, I'm not sure it did what we wanted before for REQ_DISCARD and REQ_WRITE_SAME bios/requests - I suspect for REQ_WRITE_SAME requests that had been merged it overcounted. There's also that horrible hack in the scsi (?) code Christoph added for discards - where the driver adds a page to the bio - if the driver is counting the number of segments _after_ that god only knows what is supposed to happen. Does the below patch look like what we want? I'm assuming that if multiple WRITE_SAME bios are merged, since they're all writing the same data we can consider the entire request to be a single segment. commit 1755e7ffc5745591d37b8956ce2512f4052a104a Author: Kent Overstreet <k...@daterainc.com> Date: Tue Jan 14 14:22:01 2014 -0800 block: Explicitly handle discard/write same when counting segments diff --git a/block/blk-merge.c b/block/blk-merge.c index 8f8adaa..7d977f8 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -21,6 +21,12 @@ static unsigned int __blk_recalc_rq_segments(struct request_queue *q, if (!bio) return 0; + if (bio->bi_rw & REQ_DISCARD) + return 0; + + if (bio->bi_rw & REQ_WRITE_SAME) + return 1; + fbio = bio; cluster = blk_queue_cluster(q); seg_size = 0; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/