On Wed, May 15, 2013 at 07:52:43PM +0200, Jens Axboe wrote: > On Mon, May 13 2013, Kent Overstreet wrote: > > If a bio is associated with a kiocb, allow it to be cancelled. > > > > This is accomplished by adding a pointer to a kiocb in struct bio, and > > when we go to dequeue a request we check if its bio has been cancelled - > > if so, we end the request with -ECANCELED. > > > > We don't currently try to cancel bios if IO has already been started - > > that'd require a per bio callback function, and a way to find all the > > outstanding bios for a given kiocb. Such a mechanism may or may not be > > added in the future but this patch tries to start simple. > > > > Currently this can only be triggered with aio and io_cancel(), but the > > mechanism can be used for sync io too. > > > > It can also be used for bios created by stacking drivers, and bio clones > > in general - when cloning a bio, if the bi_iocb pointer is copied as > > well the clone will then be cancellable. bio_clone() could be modified > > to do this, but hasn't in this patch because all the bio_clone() users > > would need to be auditied to make sure that it's safe. We can't blindly > > make e.g. raid5 writes cancellable without the knowledge of the md code. > > This is a pretty ugly hack, to be honest. It only works for aio. And it > grows struct bio just for that.
It's only implemented for aio in this patch but it's actually completely trivial to extend to sync kiocbs too - we can make killing a process cancel outstanding sync DIOs, I just haven't gotten around to writing the code. With sync kiocbs anything can use it. I do hate to grow struct bio, but the aio attribute stuff I'm also working on is going to need the same damn thing. > I do like the staged approach, where we just check whether a bio is > canceled when we come across it in the various parts of bio allocate to > completion. Yeah, that's the only sane way to do it imo. If we had to do it with the ki_cancel callback, since bios -> kiocbs isn't 1:1 we'd have to keep all the outstanding bios on a list protected by a lock so we could chase down all the bios we need to cancel, and I don't even want to think about stacking devices... This is also trivial to plumb through stacking devices, for ones that want to support it - md for example probably wouldn't want to support cancellation for writes (raid consistency) but for reads all it has to do is copy the kiocb pointer to the new bios it creates. (I keep having people tell me we're (Google) going to need cancel for outstanding NCQ/TCQ requests, to which my response has been "LALALALA GO AWAY I CAN'T HEAR YOU"). > > @@ -2124,6 +2130,12 @@ struct request *blk_peek_request(struct > > request_queue *q) > > trace_block_rq_issue(q, rq); > > } > > > > + if (rq->bio && !rq->bio->bi_next && bio_cancelled(rq->bio)) { > > + blk_start_request(rq); > > + __blk_end_request_all(rq, -ECANCELED); > > + continue; > > + } > > Pretty hacky too, given that it only works for the generic case of a > non-merged bio. More incomplete than hacky, imo - since with spinning disks you wouldn't save much by cancelling one bio out of a merged request. It would make sense to cancel the request if all the bios have been cancelled, but wanted to start out simple and get something useful with a minimal amount of code. Anyways, this patch is still more at the RFC stage but there is serious demand for cancellation (I've seen what people are using it for, it's not all crazy and the lack of it is something people are working around today, painfully). -- 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/