From: Ming Lei <ming.lei@redhat.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: Damien Le Moal <damien.lemoal@wdc.com>,
linux-block@vger.kernel.org,
Adam Manzanares <adam.manzanares@wdc.com>,
Alexander Viro <viro@zeniv.linux.org.uk>,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 2/7] block: Remove bio->bi_ioc
Date: Wed, 21 Nov 2018 09:21:58 +0800 [thread overview]
Message-ID: <20181121012157.GC31748@ming.t460p> (raw)
In-Reply-To: <c132f449-b45f-aab8-1b57-1a45e43d22f4@kernel.dk>
On Tue, Nov 20, 2018 at 10:31:19AM -0700, Jens Axboe wrote:
> On 11/20/18 10:21 AM, Ming Lei wrote:
> > On Mon, Nov 19, 2018 at 12:51:26PM +0900, Damien Le Moal wrote:
> >> bio->bi_ioc is never set so always NULL. Remove references to it in
> >> bio_disassociate_task() and in rq_ioc() and delete this field from
> >> struct bio. With this change, rq_ioc() always returns
> >> current->io_context without the need for a bio argument. Further
> >> simplify the code and make it more readable by also removing this
> >> helper, which also allows to simplify blk_mq_sched_assign_ioc() by
> >> removing its bio argument.
> >>
> >> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> >> ---
> >> block/bio.c | 4 ----
> >> block/blk-core.c | 2 +-
> >> block/blk-mq-sched.c | 4 ++--
> >> block/blk-mq-sched.h | 2 +-
> >> block/blk-mq.c | 4 ++--
> >> block/blk.h | 16 ----------------
> >> include/linux/blk_types.h | 3 +--
> >> 7 files changed, 7 insertions(+), 28 deletions(-)
> >>
> >> diff --git a/block/bio.c b/block/bio.c
> >> index 4f4d9884443b..03895cc0d74a 100644
> >> --- a/block/bio.c
> >> +++ b/block/bio.c
> >> @@ -2027,10 +2027,6 @@ int bio_associate_blkg(struct bio *bio, struct blkcg_gq *blkg)
> >> */
> >> void bio_disassociate_task(struct bio *bio)
> >> {
> >> - if (bio->bi_ioc) {
> >> - put_io_context(bio->bi_ioc);
> >> - bio->bi_ioc = NULL;
> >> - }
> >> if (bio->bi_css) {
> >> css_put(bio->bi_css);
> >> bio->bi_css = NULL;
> >> diff --git a/block/blk-core.c b/block/blk-core.c
> >> index d6e8ab9ca99d..492648c96992 100644
> >> --- a/block/blk-core.c
> >> +++ b/block/blk-core.c
> >> @@ -813,7 +813,7 @@ unsigned int blk_plug_queued_count(struct request_queue *q)
> >>
> >> void blk_init_request_from_bio(struct request *req, struct bio *bio)
> >> {
> >> - struct io_context *ioc = rq_ioc(bio);
> >> + struct io_context *ioc = current->io_context;
> >>
> >> if (bio->bi_opf & REQ_RAHEAD)
> >> req->cmd_flags |= REQ_FAILFAST_MASK;
> >> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> >> index d084f731d104..13b8dc332541 100644
> >> --- a/block/blk-mq-sched.c
> >> +++ b/block/blk-mq-sched.c
> >> @@ -31,10 +31,10 @@ void blk_mq_sched_free_hctx_data(struct request_queue *q,
> >> }
> >> EXPORT_SYMBOL_GPL(blk_mq_sched_free_hctx_data);
> >>
> >> -void blk_mq_sched_assign_ioc(struct request *rq, struct bio *bio)
> >> +void blk_mq_sched_assign_ioc(struct request *rq)
> >> {
> >> struct request_queue *q = rq->q;
> >> - struct io_context *ioc = rq_ioc(bio);
> >> + struct io_context *ioc = current->io_context;
> >> struct io_cq *icq;
> >>
> >> spin_lock_irq(&q->queue_lock);
> >> diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h
> >> index 7ff5671bf128..0f719c8532ae 100644
> >> --- a/block/blk-mq-sched.h
> >> +++ b/block/blk-mq-sched.h
> >> @@ -8,7 +8,7 @@
> >> void blk_mq_sched_free_hctx_data(struct request_queue *q,
> >> void (*exit)(struct blk_mq_hw_ctx *));
> >>
> >> -void blk_mq_sched_assign_ioc(struct request *rq, struct bio *bio);
> >> +void blk_mq_sched_assign_ioc(struct request *rq);
> >>
> >> void blk_mq_sched_request_inserted(struct request *rq);
> >> bool blk_mq_sched_try_merge(struct request_queue *q, struct bio *bio,
> >> diff --git a/block/blk-mq.c b/block/blk-mq.c
> >> index 32b246ed44c0..636f80b96fa6 100644
> >> --- a/block/blk-mq.c
> >> +++ b/block/blk-mq.c
> >> @@ -389,8 +389,8 @@ static struct request *blk_mq_get_request(struct request_queue *q,
> >> if (!op_is_flush(data->cmd_flags)) {
> >> rq->elv.icq = NULL;
> >> if (e && e->type->ops.prepare_request) {
> >> - if (e->type->icq_cache && rq_ioc(bio))
> >> - blk_mq_sched_assign_ioc(rq, bio);
> >> + if (e->type->icq_cache)
> >> + blk_mq_sched_assign_ioc(rq);
> >>
> >> e->type->ops.prepare_request(rq, bio);
> >> rq->rq_flags |= RQF_ELVPRIV;
> >> diff --git a/block/blk.h b/block/blk.h
> >> index 816a9abb87cd..610948157a5b 100644
> >> --- a/block/blk.h
> >> +++ b/block/blk.h
> >> @@ -254,22 +254,6 @@ void ioc_clear_queue(struct request_queue *q);
> >>
> >> int create_task_io_context(struct task_struct *task, gfp_t gfp_mask, int node);
> >>
> >> -/**
> >> - * rq_ioc - determine io_context for request allocation
> >> - * @bio: request being allocated is for this bio (can be %NULL)
> >> - *
> >> - * Determine io_context to use for request allocation for @bio. May return
> >> - * %NULL if %current->io_context doesn't exist.
> >> - */
> >> -static inline struct io_context *rq_ioc(struct bio *bio)
> >> -{
> >> -#ifdef CONFIG_BLK_CGROUP
> >> - if (bio && bio->bi_ioc)
> >> - return bio->bi_ioc;
> >> -#endif
> >> - return current->io_context;
> >> -}
> >> -
> >> /**
> >> * create_io_context - try to create task->io_context
> >> * @gfp_mask: allocation mask
> >> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> >> index dbdbfbd6a987..c0ba1a038ff3 100644
> >> --- a/include/linux/blk_types.h
> >> +++ b/include/linux/blk_types.h
> >> @@ -174,10 +174,9 @@ struct bio {
> >> void *bi_private;
> >> #ifdef CONFIG_BLK_CGROUP
> >> /*
> >> - * Optional ioc and css associated with this bio. Put on bio
> >> + * Optional css associated with this bio. Put on bio
> >> * release. Read comment on top of bio_associate_current().
> >> */
> >> - struct io_context *bi_ioc;
> >> struct cgroup_subsys_state *bi_css;
> >> struct blkcg_gq *bi_blkg;
> >> struct bio_issue bi_issue;
> >
> > Hi,
> >
> > Just found the following kernel oops, seems it is likely related with this
> > patch.
> >
> > [ 391.981012] BUG: unable to handle kernel NULL pointer dereference at 0000000000000038
> > [ 391.982506] PGD 0 P4D 0
> > [ 391.982975] Oops: 0000 [#1] PREEMPT SMP PTI
> > [ 391.983769] CPU: 1 PID: 1790 Comm: scsi_id Not tainted 4.20.0-rc3_72abead3bf43_for-4.21-block-mp-bvec-V11+ #1
> > [ 391.985563] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.10.2-2.fc27 04/01/2014
> > [ 391.987107] RIP: 0010:ioc_lookup_icq+0x13/0x54
> > [ 391.987936] Code: f6 48 8b 3d 1c 78 5b 01 5b 5d 41 5c 41 5d 41 5e 41 5f e9 68 bd eb ff 0f 1f 44 00 00 55 53 48 89 fb 51 48 89 f5 e8 e3 82 da ff <48> 8b 43 38 48 85 c0 74 05 48 39 28 74 22 48 63 b5 8c 00 00 00 48
> > [ 391.991318] RSP: 0018:ffffc90001467bb0 EFLAGS: 00010002
> > [ 391.992292] RAX: ffff888266c85ac0 RBX: 0000000000000000 RCX: 0000000000000100
> > [ 391.993615] RDX: 0000000000000001 RSI: ffff88826601f230 RDI: 0000000000000000
> > [ 391.994917] RBP: ffff88826601f230 R08: 00000000f461df07 R09: 0000000000000006
> > [ 391.996242] R10: ffffc90001467b10 R11: 0000000000000020 R12: ffff888269df4000
> > [ 391.997572] R13: 0000000000000000 R14: ffff88826601f2c4 R15: 0000000000000001
> > [ 391.998905] FS: 00007fa6923b7940(0000) GS:ffff888277a80000(0000) knlGS:0000000000000000
> > [ 392.000389] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 392.001468] CR2: 0000000000000038 CR3: 0000000106dd8005 CR4: 0000000000760ee0
> > [ 392.002783] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [ 392.004108] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > [ 392.005394] PKRU: 55555554
> > [ 392.005897] Call Trace:
> > [ 392.006384] blk_mq_sched_assign_ioc+0x3d/0x7f
> > [ 392.007216] blk_mq_get_request+0x321/0x354
> > [ 392.008008] blk_mq_alloc_request+0x4e/0xbf
> > [ 392.008802] blk_get_request+0x24/0x4c
> > [ 392.009518] sg_io+0x93/0x371
> > [ 392.010074] ? bd_acquire+0xa6/0xa6
> > [ 392.010707] ? dput+0x29/0xfd
> > [ 392.011232] ? mntput_no_expire+0x11/0x185
> > [ 392.011987] scsi_cmd_ioctl+0x1d3/0x386
> > [ 392.012707] sd_ioctl+0xbb/0xde [sd_mod]
> > [ 392.013449] blkdev_ioctl+0x893/0x8bf
> > [ 392.014132] block_ioctl+0x3c/0x3f
> > [ 392.014781] vfs_ioctl+0x1e/0x2b
> > [ 392.015378] do_vfs_ioctl+0x531/0x559
> > [ 392.016059] ksys_ioctl+0x3e/0x5d
> > [ 392.016681] __x64_sys_ioctl+0x16/0x19
> > [ 392.017361] do_syscall_64+0x84/0x13f
> > [ 392.018060] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > [ 392.018995] RIP: 0033:0x7fa691edf267
> > [ 392.019663] Code: b3 66 90 48 8b 05 19 3c 2c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d e9 3b 2c 00 f7 d8 64 89 01 48
> > [ 392.023072] RSP: 002b:00007ffe463e8de8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > [ 392.024457] RAX: ffffffffffffffda RBX: 00007ffe463e8e20 RCX: 00007fa691edf267
> > [ 392.025767] RDX: 00007ffe463e8e20 RSI: 0000000000002285 RDI: 0000000000000004
> > [ 392.027068] RBP: 00007ffe463e9470 R08: 0000000000002006 R09: 00000000fffffe00
> > [ 392.028413] R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffe463e9920
> > [ 392.029722] R13: 00007ffe463e8e20 R14: 00007ffe463e8e2a R15: 00007ffe463e9920
> > [ 392.031031] Modules linked in: scsi_debug null_blk isofs iTCO_wdt iTCO_vendor_support i2c_i801 i2c_core lpc_ich mfd_core ip_tables sr_mod cdrom usb_storage sd_mod ahci libahci libata crc32c_intel virtio_scsi qemu_fw_cfg dm_mirror dm_region_hash dm_log dm_mod [last unloaded: null_blk]
> > [ 392.035573] Dumping ftrace buffer:
> > [ 392.036203] (ftrace buffer empty)
> > [ 392.036871] CR2: 0000000000000038
> > [ 392.037503] ---[ end trace fa20a1088b068790 ]---
>
> I think the below should fix it, we haven't necessarily setup an
> ioc if we're just doing as passthrough request.
>
>
> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> index 13b8dc332541..f096d8989773 100644
> --- a/block/blk-mq-sched.c
> +++ b/block/blk-mq-sched.c
> @@ -34,9 +34,16 @@ EXPORT_SYMBOL_GPL(blk_mq_sched_free_hctx_data);
> void blk_mq_sched_assign_ioc(struct request *rq)
> {
> struct request_queue *q = rq->q;
> - struct io_context *ioc = current->io_context;
> + struct io_context *ioc;
> struct io_cq *icq;
>
> + /*
> + * May not have an IO context if it's a passthrough request
> + */
> + ioc = current->io_context;
> + if (!ioc)
> + return;
> +
> spin_lock_irq(&q->queue_lock);
> icq = ioc_lookup_icq(ioc, q);
> spin_unlock_irq(&q->queue_lock);
Looks this patch fixes the kernel panic in elevator stress switch test.
Tested-by: Ming Lei <ming.lei@redhat.com>
Thanks,
Ming
next prev parent reply other threads:[~2018-11-21 1:22 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-19 3:51 [PATCH 0/7] Improve I/O priority handling Damien Le Moal
2018-11-19 3:51 ` [PATCH 1/7] aio: Comment use of IOCB_FLAG_IOPRIO aio flag Damien Le Moal
2018-11-19 8:12 ` Christoph Hellwig
2018-11-19 8:15 ` Johannes Thumshirn
2018-11-19 3:51 ` [PATCH 2/7] block: Remove bio->bi_ioc Damien Le Moal
2018-11-19 8:13 ` Christoph Hellwig
2018-11-19 8:16 ` Johannes Thumshirn
2018-11-19 19:07 ` Adam Manzanares
2018-11-20 17:21 ` Ming Lei
2018-11-20 17:31 ` Jens Axboe
2018-11-20 23:58 ` Damien Le Moal
2018-11-21 1:24 ` Ming Lei
2018-11-21 1:31 ` Damien Le Moal
2018-11-21 2:10 ` Jens Axboe
2018-11-21 2:14 ` Damien Le Moal
2018-11-21 2:45 ` Damien Le Moal
2018-11-21 2:48 ` Jens Axboe
2018-11-21 2:50 ` Damien Le Moal
2018-11-21 1:21 ` Ming Lei [this message]
2018-11-19 3:51 ` [PATCH 3/7] block: Fix get_task_ioprio() default return value Damien Le Moal
2018-11-19 8:16 ` Christoph Hellwig
2018-11-20 1:47 ` Damien Le Moal
2018-11-19 3:51 ` [PATCH 4/7] block: Introduce get_current_ioprio() Damien Le Moal
2018-11-19 8:17 ` Christoph Hellwig
2018-11-19 8:26 ` Johannes Thumshirn
2018-11-19 18:17 ` Adam Manzanares
2018-11-19 23:46 ` Damien Le Moal
2018-11-19 3:51 ` [PATCH 5/7] aio: Fix fallback I/O priority value Damien Le Moal
2018-11-19 8:18 ` Christoph Hellwig
2018-11-19 8:27 ` Johannes Thumshirn
2018-11-19 19:08 ` Adam Manzanares
2018-11-19 3:51 ` [PATCH 6/7] block: prevent merging of requests with different priorities Damien Le Moal
2018-11-19 8:19 ` Christoph Hellwig
2018-11-19 8:31 ` Johannes Thumshirn
2018-11-19 3:51 ` [PATCH 7/7] block: Initialize BIO I/O priority early Damien Le Moal
2018-11-19 8:19 ` Christoph Hellwig
2018-11-19 19:11 ` Adam Manzanares
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=20181121012157.GC31748@ming.t460p \
--to=ming.lei@redhat.com \
--cc=adam.manzanares@wdc.com \
--cc=axboe@kernel.dk \
--cc=damien.lemoal@wdc.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=viro@zeniv.linux.org.uk \
/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.