All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Kent Overstreet <koverstreet@google.com>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-aio@kvack.org, akpm@linux-foundation.org,
	Zach Brown <zab@redhat.com>, Joel Becker <jlbec@evilplan.org>,
	Jeff Moyer <jmoyer@redhat.com>, Al Viro <viro@zeniv.linux.org.uk>,
	Benjamin LaHaise <bcrl@kvack.org>
Subject: Re: [PATCH 21/21] block: Bio cancellation
Date: Wed, 15 May 2013 19:52:43 +0200	[thread overview]
Message-ID: <20130515175243.GL4728@kernel.dk> (raw)
In-Reply-To: <1368494338-7069-22-git-send-email-koverstreet@google.com>

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.

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.

> @@ -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.

So nack on this one.

-- 
Jens Axboe

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

WARNING: multiple messages have this Message-ID (diff)
From: Jens Axboe <axboe@kernel.dk>
To: Kent Overstreet <koverstreet@google.com>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-aio@kvack.org, akpm@linux-foundation.org,
	Zach Brown <zab@redhat.com>, Joel Becker <jlbec@evilplan.org>,
	Jeff Moyer <jmoyer@redhat.com>, Al Viro <viro@zeniv.linux.org.uk>,
	Benjamin LaHaise <bcrl@kvack.org>
Subject: Re: [PATCH 21/21] block: Bio cancellation
Date: Wed, 15 May 2013 19:52:43 +0200	[thread overview]
Message-ID: <20130515175243.GL4728@kernel.dk> (raw)
In-Reply-To: <1368494338-7069-22-git-send-email-koverstreet@google.com>

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.

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.

> @@ -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.

So nack on this one.

-- 
Jens Axboe


  reply	other threads:[~2013-05-15 17:52 UTC|newest]

Thread overview: 104+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-14  1:18 AIO refactoring/performance improvements/cancellation Kent Overstreet
2013-05-14  1:18 ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 01/21] aio: fix kioctx not being freed after cancellation at exit time Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 02/21] aio: reqs_active -> reqs_available Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 03/21] aio: percpu reqs_available Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 04/21] Generic percpu refcounting Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14 13:51   ` Oleg Nesterov
2013-05-14 13:51     ` Oleg Nesterov
2013-05-15  8:21     ` Kent Overstreet
2013-05-15  8:21       ` Kent Overstreet
2013-05-14 14:59   ` Tejun Heo
2013-05-14 14:59     ` Tejun Heo
2013-05-14 15:28     ` Oleg Nesterov
2013-05-14 15:28       ` Oleg Nesterov
2013-05-15  9:00       ` Kent Overstreet
2013-05-15  9:00         ` Kent Overstreet
2013-05-15  8:58     ` Kent Overstreet
2013-05-15  8:58       ` Kent Overstreet
2013-05-15 17:37       ` Tejun Heo
2013-05-15 17:37         ` Tejun Heo
2013-05-28 23:47         ` Kent Overstreet
2013-05-28 23:47           ` Kent Overstreet
2013-05-29  1:11           ` Tejun Heo
2013-05-29  1:11             ` Tejun Heo
2013-05-29  4:59           ` Rusty Russell
2013-05-29  4:59             ` Rusty Russell
2013-05-31 20:12             ` Kent Overstreet
2013-05-31 20:12               ` Kent Overstreet
2013-05-14 21:59   ` Tejun Heo
2013-05-14 21:59     ` Tejun Heo
2013-05-14 22:15     ` Tejun Heo
2013-05-14 22:15       ` Tejun Heo
2013-05-15  9:07     ` Kent Overstreet
2013-05-15  9:07       ` Kent Overstreet
2013-05-15 17:56       ` Tejun Heo
2013-05-15 17:56         ` Tejun Heo
2013-05-16  0:26   ` Rusty Russell
2013-05-16  0:26     ` Rusty Russell
2013-05-14  1:18 ` [PATCH 05/21] aio: percpu ioctx refcount Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 06/21] aio: io_cancel() no longer returns the io_event Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 07/21] aio: Don't use ctx->tail unnecessarily Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 08/21] aio: Kill aio_rw_vect_retry() Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 09/21] aio: Kill unneeded kiocb members Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 10/21] aio: Kill ki_users Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 11/21] aio: Kill ki_dtor Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 12/21] aio: convert the ioctx list to radix tree Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 13/21] block: prep work for batch completion Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 14/21] block, aio: batch completion for bios/kiocbs Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 15/21] virtio-blk: convert to batch completion Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 16/21] mtip32xx: " Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 17/21] Percpu tag allocator Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14 13:48   ` Oleg Nesterov
2013-05-14 13:48     ` Oleg Nesterov
2013-05-14 14:24     ` Oleg Nesterov
2013-05-14 14:24       ` Oleg Nesterov
2013-05-15  9:34       ` Kent Overstreet
2013-05-15  9:34         ` Kent Overstreet
2013-05-15  9:25     ` Kent Overstreet
2013-05-15  9:25       ` Kent Overstreet
2013-05-15 15:41       ` Oleg Nesterov
2013-05-15 15:41         ` Oleg Nesterov
2013-05-15 16:10         ` Oleg Nesterov
2013-05-15 16:10           ` Oleg Nesterov
2013-06-10 23:20         ` Kent Overstreet
2013-06-10 23:20           ` Kent Overstreet
2013-06-11 17:42           ` Oleg Nesterov
2013-06-11 17:42             ` Oleg Nesterov
2013-05-14 15:03   ` Tejun Heo
2013-05-14 15:03     ` Tejun Heo
2013-05-15 20:19   ` Andi Kleen
2013-05-15 20:19     ` Andi Kleen
2013-05-14  1:18 ` [PATCH 18/21] aio: Allow cancellation without a cancel callback, new kiocb lookup Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 19/21] aio/usb: Update cancellation for new synchonization Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 20/21] direct-io: Set dio->io_error directly Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-14  1:18 ` [PATCH 21/21] block: Bio cancellation Kent Overstreet
2013-05-14  1:18   ` Kent Overstreet
2013-05-15 17:52   ` Jens Axboe [this message]
2013-05-15 17:52     ` Jens Axboe
2013-05-15 19:29     ` Kent Overstreet
2013-05-15 19:29       ` Kent Overstreet
2013-05-15 20:01       ` Jens Axboe
2013-05-15 20:01         ` Jens Axboe
2013-05-31 22:52         ` Kent Overstreet
2013-05-31 22:52           ` Kent Overstreet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130515175243.GL4728@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=akpm@linux-foundation.org \
    --cc=bcrl@kvack.org \
    --cc=jlbec@evilplan.org \
    --cc=jmoyer@redhat.com \
    --cc=koverstreet@google.com \
    --cc=linux-aio@kvack.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=zab@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.