| Message ID | 0d1ba678-69c0-16ce-6c3e-475cd8da203c@grimberg.me |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 03/08/2017 01:03 AM, Sagi Grimberg wrote: > >> - if (likely(blk_queue_enter(q, false) == 0)) { >> + if (likely(blk_queue_enter(q, bio_flagged(bio, BIO_NOWAIT)) >> == 0)) { >> ret = q->make_request_fn(q, bio); > > I think that for ->make_request to not block we'd need to set > BLK_MQ_REQ_NOWAIT in blk_mq_alloc_data to avoid blocking on a tag > allocation. > > Something like the untested addition below: I did that in the first series, but there are too many reasons to block in blk-mq [1]. I dropped blk-mq work in v2. [1] https://patchwork.kernel.org/patch/9571051/
On Wed 08-03-17 09:00:09, Goldwyn Rodrigues wrote: > > > On 03/08/2017 01:03 AM, Sagi Grimberg wrote: > > > >> - if (likely(blk_queue_enter(q, false) == 0)) { > >> + if (likely(blk_queue_enter(q, bio_flagged(bio, BIO_NOWAIT)) > >> == 0)) { > >> ret = q->make_request_fn(q, bio); > > > > I think that for ->make_request to not block we'd need to set > > BLK_MQ_REQ_NOWAIT in blk_mq_alloc_data to avoid blocking on a tag > > allocation. > > > > Something like the untested addition below: > > I did that in the first series, but there are too many reasons to block > in blk-mq [1]. I dropped blk-mq work in v2. > > [1] https://patchwork.kernel.org/patch/9571051/ Well, that's not really good. If we cannot support this for both blk-mq and legacy block layer the feature will not be usable. So please work on blk-mq support as well. Honza
On Wed, Mar 08, 2017 at 04:28:06PM +0100, Jan Kara wrote: > Well, that's not really good. If we cannot support this for both blk-mq and > legacy block layer the feature will not be usable. So please work on blk-mq > support as well. Exactly. In addition to that anything implementing a feature for the legacy request request code but not blk-mq is grounds for an instant NAK.
On 03/08/2017 08:00 AM, Goldwyn Rodrigues wrote: > > > On 03/08/2017 01:03 AM, Sagi Grimberg wrote: >> >>> - if (likely(blk_queue_enter(q, false) == 0)) { >>> + if (likely(blk_queue_enter(q, bio_flagged(bio, BIO_NOWAIT)) >>> == 0)) { >>> ret = q->make_request_fn(q, bio); >> >> I think that for ->make_request to not block we'd need to set >> BLK_MQ_REQ_NOWAIT in blk_mq_alloc_data to avoid blocking on a tag >> allocation. >> >> Something like the untested addition below: > > I did that in the first series, but there are too many reasons to block > in blk-mq [1]. I dropped blk-mq work in v2. That's complete nonsense, there are no more places in blk-mq that will block that in the legacy path. Most of the examples from your URL: > [1] https://patchwork.kernel.org/patch/9571051/ are not blk-mq, but writeback throttling, and drivers that explicitly hook into ->make_request_fn. As others have mentioned, it's a total non-starter to focus on the deprecated IO path and just ignore the new one. Back to the drawing board.
On 03/08/2017 10:17 AM, Jens Axboe wrote: > On 03/08/2017 08:00 AM, Goldwyn Rodrigues wrote: >> >> >> On 03/08/2017 01:03 AM, Sagi Grimberg wrote: >>> >>>> - if (likely(blk_queue_enter(q, false) == 0)) { >>>> + if (likely(blk_queue_enter(q, bio_flagged(bio, BIO_NOWAIT)) >>>> == 0)) { >>>> ret = q->make_request_fn(q, bio); >>> >>> I think that for ->make_request to not block we'd need to set >>> BLK_MQ_REQ_NOWAIT in blk_mq_alloc_data to avoid blocking on a tag >>> allocation. >>> >>> Something like the untested addition below: >> >> I did that in the first series, but there are too many reasons to block >> in blk-mq [1]. I dropped blk-mq work in v2. > > That's complete nonsense, there are no more places in blk-mq that will > block that in the legacy path. Most of the examples from your URL: > >> [1] https://patchwork.kernel.org/patch/9571051/ > > are not blk-mq, but writeback throttling, and drivers that explicitly > hook into ->make_request_fn. > > As others have mentioned, it's a total non-starter to focus on the > deprecated IO path and just ignore the new one. Back to the drawing > board. > Thanks, I understood what I was doing wrong. I will include blk-mq in the patch.
diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c index 09af8ff18719..40e78b57fb44 100644 --- a/block/blk-mq-sched.c +++ b/block/blk-mq-sched.c @@ -119,6 +119,9 @@ struct request *blk_mq_sched_get_request(struct request_queue *q, if (likely(!data->hctx)) data->hctx = blk_mq_map_queue(q, data->ctx->cpu); + if (likely(bio) && bio_flagged(bio, BIO_NOWAIT)) + data->flags |= BLK_MQ_REQ_NOWAIT; + if (e) { data->flags |= BLK_MQ_REQ_INTERNAL;