Re: [PATCH V2 0/2] block: remove unnecessary RESTART
On Wed, Nov 01, 2017 at 03:54:09AM +, Bart Van Assche wrote: > On Tue, 2017-10-31 at 09:47 +0800, Ming Lei wrote: > > On Mon, Oct 30, 2017 at 08:24:57PM +, Bart Van Assche wrote: > > > On Fri, 2017-10-27 at 13:38 +0800, Ming Lei wrote: > > > > On Fri, Oct 27, 2017 at 04:53:18AM +, Bart Van Assche wrote: > > > > > On Fri, 2017-10-27 at 12:43 +0800, Ming Lei wrote: > > > > > > The 1st patch removes the RESTART for TAG-SHARED because SCSI > > > > > > handles it > > > > > > by itself, and not necessary to waste CPU to do the expensive > > > > > > RESTART. > > > > > > And Roman Pen reported that this RESTART cuts half of IOPS in his > > > > > > case. > > > > > > > > > > > > The 2nd patch removes the RESTART when .get_budget returns > > > > > > BLK_STS_RESOURCE, > > > > > > and this RESTART is handled by SCSI's RESTART(scsi_end_request()) > > > > > > too. > > > > > > > > > > There are more block drivers than the SCSI core that share tags. If > > > > > the > > > > > > > > Could you share us what the other in-tree driver which share tags is? > > > > > > I think the following in-tree drivers support shared tags (in alphabetical > > > order): > > > * null_blk. See also the shared_tags kernel module parameter. > > > * nvme. See also nvme_alloc_ns(). > > > * scsi-mq. > > > > For both null_blk and nvme, we don't need to deal with cross-queue RESTART, > > because BLK_MQ_S_TAG_WAITING has handled it already. > > blk_mq_dispatch_wait_add() returns immediately if BLK_MQ_S_TAG_WAITING is > already set. Are you really sure that removing the restart mechanism doesn't If this flag is set, that means blk_mq_dispatch_wake() will be run when driver tag is available, so the hw queue will be run at that time. > break the NVMe driver if there are multiple namespaces? Did your commit 6d8c6c0f97ad ("blk-mq: Restart a single queue if tag sets are shared") suppose to fix NVMe? Please take a look at the cover letter: https://marc.info/?t=149158911500010=1=2 -- Ming
Re: [PATCH V2 0/2] block: remove unnecessary RESTART
On Tue, 2017-10-31 at 09:47 +0800, Ming Lei wrote: > On Mon, Oct 30, 2017 at 08:24:57PM +, Bart Van Assche wrote: > > On Fri, 2017-10-27 at 13:38 +0800, Ming Lei wrote: > > > On Fri, Oct 27, 2017 at 04:53:18AM +, Bart Van Assche wrote: > > > > On Fri, 2017-10-27 at 12:43 +0800, Ming Lei wrote: > > > > > The 1st patch removes the RESTART for TAG-SHARED because SCSI handles > > > > > it > > > > > by itself, and not necessary to waste CPU to do the expensive RESTART. > > > > > And Roman Pen reported that this RESTART cuts half of IOPS in his > > > > > case. > > > > > > > > > > The 2nd patch removes the RESTART when .get_budget returns > > > > > BLK_STS_RESOURCE, > > > > > and this RESTART is handled by SCSI's RESTART(scsi_end_request()) too. > > > > > > > > There are more block drivers than the SCSI core that share tags. If the > > > > > > Could you share us what the other in-tree driver which share tags is? > > > > I think the following in-tree drivers support shared tags (in alphabetical > > order): > > * null_blk. See also the shared_tags kernel module parameter. > > * nvme. See also nvme_alloc_ns(). > > * scsi-mq. > > For both null_blk and nvme, we don't need to deal with cross-queue RESTART, > because BLK_MQ_S_TAG_WAITING has handled it already. blk_mq_dispatch_wait_add() returns immediately if BLK_MQ_S_TAG_WAITING is already set. Are you really sure that removing the restart mechanism doesn't break the NVMe driver if there are multiple namespaces? Bart.
Re: [PATCH V2 0/2] block: remove unnecessary RESTART
On Tue, Oct 31, 2017 at 07:53:03PM -0600, Jens Axboe wrote: > > > On Oct 31, 2017, at 7:46 PM, Ming Leiwrote: > > > >> On Tue, Oct 31, 2017 at 02:29:32PM -0600, Jens Axboe wrote: > >>> On 10/26/2017 10:43 PM, Ming Lei wrote: > >>> Hi Jens, > >>> > >>> The 1st patch removes the RESTART for TAG-SHARED because SCSI handles it > >>> by itself, and not necessary to waste CPU to do the expensive RESTART. > >>> And Roman Pen reported that this RESTART cuts half of IOPS in his case. > >>> > >>> The 2nd patch removes the RESTART when .get_budget returns > >>> BLK_STS_RESOURCE, > >>> and this RESTART is handled by SCSI's RESTART(scsi_end_request()) too. > >> > >> What base is this against? > > > > The for-next branch of your block tree: > > From when? Doesn’t apply at all today. I just tried today's for-next(top commit is 'MAINTAINERS: Remove Rafael from Opal maintainers.'), and the two patches can be applied cleanly. I guess you may try to apply the two patches against for-4.15/block, which doesn't include the patchset of '[PATCH V10 0/8] blk-mq-sched: improve sequential I/O'[1]: https://marc.info/?t=15079731662=1=2 -- Ming
Re: [PATCH V2 0/2] block: remove unnecessary RESTART
> On Oct 31, 2017, at 7:46 PM, Ming Leiwrote: > >> On Tue, Oct 31, 2017 at 02:29:32PM -0600, Jens Axboe wrote: >>> On 10/26/2017 10:43 PM, Ming Lei wrote: >>> Hi Jens, >>> >>> The 1st patch removes the RESTART for TAG-SHARED because SCSI handles it >>> by itself, and not necessary to waste CPU to do the expensive RESTART. >>> And Roman Pen reported that this RESTART cuts half of IOPS in his case. >>> >>> The 2nd patch removes the RESTART when .get_budget returns BLK_STS_RESOURCE, >>> and this RESTART is handled by SCSI's RESTART(scsi_end_request()) too. >> >> What base is this against? > > The for-next branch of your block tree: From when? Doesn’t apply at all today.
Re: [PATCH V2 0/2] block: remove unnecessary RESTART
On Tue, Oct 31, 2017 at 02:29:32PM -0600, Jens Axboe wrote: > On 10/26/2017 10:43 PM, Ming Lei wrote: > > Hi Jens, > > > > The 1st patch removes the RESTART for TAG-SHARED because SCSI handles it > > by itself, and not necessary to waste CPU to do the expensive RESTART. > > And Roman Pen reported that this RESTART cuts half of IOPS in his case. > > > > The 2nd patch removes the RESTART when .get_budget returns BLK_STS_RESOURCE, > > and this RESTART is handled by SCSI's RESTART(scsi_end_request()) too. > > What base is this against? The for-next branch of your block tree: -- Ming
Re: [PATCH] ide:ide-cd: fix kernel panic resulting from missing scsi_req_init
On 2017年10月31日 23:23, Bart Van Assche wrote: On Tue, 2017-10-31 at 15:39 +0800, Hongxu Jia wrote: Since we split the scsi_request out of struct request, while the standard prep_rq_fn builds 10 byte cmds, it missed to invoke scsi_req_init() to initialize certain fields of a scsi_request structure (.__cmd[], .cmd, .cmd_len and .sense_len but no other members of struct scsi_request). An example panic on virtual machines (qemu/virtualbox) to boot from IDE cdrom: ... [8.754381] Call Trace: [8.755419] blk_peek_request+0x182/0x2e0 [8.755863] blk_fetch_request+0x1c/0x40 [8.756148] ? ktime_get+0x40/0xa0 [8.756385] do_ide_request+0x37d/0x660 [8.756704] ? cfq_group_service_tree_add+0x98/0xc0 [8.757011] ? cfq_service_tree_add+0x1e5/0x2c0 [8.757313] ? ktime_get+0x40/0xa0 [8.757544] __blk_run_queue+0x3d/0x60 [8.757837] queue_unplugged+0x2f/0xc0 [8.758088] blk_flush_plug_list+0x1f4/0x240 [8.758362] blk_finish_plug+0x2c/0x40 ... [8.770906] RIP: ide_cdrom_prep_fn+0x63/0x180 RSP: 92aec018bae8 [8.772329] ---[ end trace 6408481e551a85c9 ]--- ... With which kernel version did you encounter this kernel panic? IDE CD-ROM access works fine here from inside qemu with kernel v4.14.0-rc6. I also compiled with kernel 4.14.0-rc6, and it failed. Ubuntu 17.10, Fedora 27 do not have the same issue, because they disable ide and use ata piix to instead. Ubuntu 17.10, kernel 4.13.0-16 vim /boot/config-4.13.0-16-generic ... # CONFIG_IDE is not set CONFIG_ATA_PIIX=y ... Fedora 27, kernel 4.13.5 vi /boot/config-4.13.5-300.fc27.x86_64 ... # CONFIG_IDE is not set CONFIG_ATA_PIIX=y ... With above config, the boot log has ... [ 4.352505] ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100 [ 4.354875] ata2.00: configured for MWDMA2 [ 4.385692] scsi 1:0:0:0: CD-ROM QEMU QEMU DVD-ROM 2.5+ PQ: 0 ANSI: 5 [ 4.398879] sr 1:0:0:0: [sr0] scsi3-mmc drive: 4x/4x cd/rw xa/form2 tray [ 4.399875] cdrom: Uniform CD-ROM driver Revision: 3.20 ... If apply this fix and enable ide, the boot log has ... [ 5.337407] hdc: QEMU DVD-ROM, ATAPI CD/DVD-ROM drive [ 5.967310] hdc: MWDMA2 mode selected [ 5.970682] ide0 at 0x1f0-0x1f7,0x3f6 on irq 14 [ 5.972311] ide1 at 0x170-0x177,0x376 on irq 15 [ 5.979317] ide-gd driver 1.18 [ 5.980579] ide-cd driver 5.00 [ 5.996508] ide-cd: hdc: ATAPI 4X DVD-ROM drive, 512kB Cache [ 5.997926] cdrom: Uniform CD-ROM driver Revision: 3.20 ... What about your kernel config and boot log? Without this fix and enable ide, the boot log failed: ... Loading /vmlinuz... ok Loading /initrd...ok [ 0.00] Linux version 4.14.0-rc6-yocto-standard (oe-user@oe-host) (gcc version 7.2.0 (GCC)) #112 SMP PREEMPT Wed Nov 1 09:14:09 CST 2017 [ 0.00] Command line: BOOT_IMAGE=/vmlinuz initrd=/initrd LABEL=boot root=/dev/ram0 console=ttyS0,115200 debugshell [ 0.00] x86/fpu: x87 FPU will use FXSAVE ... [ 8.714028] BUG: unable to handle kernel NULL pointer dereference at (null) [ 8.715178] IP: ide_cdrom_prep_fn+0x63/0x180 [ 8.715557] PGD 0 P4D 0 [ 8.715935] Oops: 0002 [#1] PREEMPT SMP [ 8.716488] Modules linked in: [ 8.717043] CPU: 0 PID: 95 Comm: udevd Not tainted 4.14.0-rc6-yocto-standard #112 [ 8.717693] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.10.2-0-g5f4c7b1-prebuilt.qemu-project.org 04/01/2014 [ 8.718807] task: 8b9a7ae42340 task.stack: 95bfc0104000 [ 8.719313] RIP: 0010:ide_cdrom_prep_fn+0x63/0x180 [ 8.719661] RSP: 0018:95bfc0107ae8 EFLAGS: 0002 [ 8.720203] RAX: 0002 RBX: 8b9a7b2eca88 RCX: [ 8.720780] RDX: RSI: 8b9a7ae6ea00 RDI: 0013b3fc [ 8.721335] RBP: 95bfc0107ae8 R08: R09: 0020 [ 8.721846] R10: 0001 R11: 0c022540 R12: 8b9a7ae6ea00 [ 8.722373] R13: 8b9a7b2eca88 R14: 8b9a7f8d7000 R15: [ 8.722946] FS: 7f00f5068300() GS:8b9a7f20() knlGS: [ 8.723550] CS: 0010 DS: ES: CR0: 80050033 [ 8.723967] CR2: CR3: 3fa88000 CR4: 06f0 [ 8.724735] Call Trace: [ 8.725769] blk_peek_request+0x182/0x2e0 [ 8.726199] blk_fetch_request+0x1c/0x40 [ 8.726506] ? _raw_spin_unlock_irq+0x23/0x30 [ 8.726932] do_ide_request+0x37d/0x660 [ 8.727254] ? cfq_group_service_tree_add+0x98/0xc0 [ 8.727651] ? cfq_service_tree_add+0x1e5/0x2c0 [ 8.728067] ? ktime_get+0x40/0xa0 [ 8.728361] __blk_run_queue+0x3d/0x60 [ 8.728675] queue_unplugged+0x2f/0xc0 [ 8.728994] blk_flush_plug_list+0x1f4/0x240 [ 8.729341] blk_finish_plug+0x2c/0x40 [ 8.729670] __do_page_cache_readahead+0x1bb/0x260 [ 8.730084] force_page_cache_readahead+0xb5/0x110 [ 8.730452] ? force_page_cache_readahead+0xb5/0x110 [ 8.730801] page_cache_sync_readahead+0x3f/0x50 [
Re: [PATCH] MAINTAINERS: Remove Rafael from Opal maintainers.
On 10/31/2017 02:15 PM, Scott Bauer wrote: > He is no longer working on storage. Applied, thanks. -- Jens Axboe
[PATCH] MAINTAINERS: Remove Rafael from Opal maintainers.
He is no longer working on storage. Signed-off-by: Scott Bauer--- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index af0cb69f6a3e..5c0864d7d7ad 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12052,7 +12052,6 @@ F: drivers/mmc/host/sdhci-spear.c SECURE ENCRYPTING DEVICE (SED) OPAL DRIVER M: Scott Bauer M: Jonathan Derrick -M: Rafael Antognolli L: linux-block@vger.kernel.org S: Supported F: block/sed* -- 2.11.0
Re: [PATCH V2 0/2] block: remove unnecessary RESTART
On 10/26/2017 10:43 PM, Ming Lei wrote: > Hi Jens, > > The 1st patch removes the RESTART for TAG-SHARED because SCSI handles it > by itself, and not necessary to waste CPU to do the expensive RESTART. > And Roman Pen reported that this RESTART cuts half of IOPS in his case. > > The 2nd patch removes the RESTART when .get_budget returns BLK_STS_RESOURCE, > and this RESTART is handled by SCSI's RESTART(scsi_end_request()) too. What base is this against? -- Jens Axboe
Re: [PATCH] ide:ide-cd: fix kernel panic resulting from missing scsi_req_init
On Tue, 2017-10-31 at 15:39 +0800, Hongxu Jia wrote: > Since we split the scsi_request out of struct request, while the > standard prep_rq_fn builds 10 byte cmds, it missed to invoke > scsi_req_init() to initialize certain fields of a scsi_request > structure (.__cmd[], .cmd, .cmd_len and .sense_len but no other > members of struct scsi_request). > > An example panic on virtual machines (qemu/virtualbox) to boot > from IDE cdrom: > ... > [8.754381] Call Trace: > [8.755419] blk_peek_request+0x182/0x2e0 > [8.755863] blk_fetch_request+0x1c/0x40 > [8.756148] ? ktime_get+0x40/0xa0 > [8.756385] do_ide_request+0x37d/0x660 > [8.756704] ? cfq_group_service_tree_add+0x98/0xc0 > [8.757011] ? cfq_service_tree_add+0x1e5/0x2c0 > [8.757313] ? ktime_get+0x40/0xa0 > [8.757544] __blk_run_queue+0x3d/0x60 > [8.757837] queue_unplugged+0x2f/0xc0 > [8.758088] blk_flush_plug_list+0x1f4/0x240 > [8.758362] blk_finish_plug+0x2c/0x40 > ... > [8.770906] RIP: ide_cdrom_prep_fn+0x63/0x180 RSP: 92aec018bae8 > [8.772329] ---[ end trace 6408481e551a85c9 ]--- > ... With which kernel version did you encounter this kernel panic? IDE CD-ROM access works fine here from inside qemu with kernel v4.14.0-rc6. Bart.
[PATCH v2 3/3] block: add WARN_ON if bdi register fail
device_add_disk need do more safety error handle, so this patch just add WARN_ON. Reviewed-by: Jan KaraSigned-off-by: weiping zhang --- block/genhd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/genhd.c b/block/genhd.c index dd305c65ffb0..52834433878c 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -660,7 +660,7 @@ void device_add_disk(struct device *parent, struct gendisk *disk) /* Register BDI before referencing it from bdev */ bdi = disk->queue->backing_dev_info; - bdi_register_owner(bdi, disk_to_dev(disk)); + WARN_ON(bdi_register_owner(bdi, disk_to_dev(disk))); blk_register_region(disk_devt(disk), disk->minors, NULL, exact_match, exact_lock, disk); -- 2.14.2
[PATCH v2 1/3] bdi: convert bdi_debug_register to int
Convert bdi_debug_register to int and then do error handle for it. Reviewed-by: Jan KaraSigned-off-by: weiping zhang --- mm/backing-dev.c | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/mm/backing-dev.c b/mm/backing-dev.c index 74b52dfd5852..b5f940ce0143 100644 --- a/mm/backing-dev.c +++ b/mm/backing-dev.c @@ -113,11 +113,23 @@ static const struct file_operations bdi_debug_stats_fops = { .release= single_release, }; -static void bdi_debug_register(struct backing_dev_info *bdi, const char *name) +static int bdi_debug_register(struct backing_dev_info *bdi, const char *name) { + if (!bdi_debug_root) + return -ENOMEM; + bdi->debug_dir = debugfs_create_dir(name, bdi_debug_root); + if (!bdi->debug_dir) + return -ENOMEM; + bdi->debug_stats = debugfs_create_file("stats", 0444, bdi->debug_dir, bdi, _debug_stats_fops); + if (!bdi->debug_stats) { + debugfs_remove(bdi->debug_dir); + return -ENOMEM; + } + + return 0; } static void bdi_debug_unregister(struct backing_dev_info *bdi) @@ -129,9 +141,10 @@ static void bdi_debug_unregister(struct backing_dev_info *bdi) static inline void bdi_debug_init(void) { } -static inline void bdi_debug_register(struct backing_dev_info *bdi, +static inline int bdi_debug_register(struct backing_dev_info *bdi, const char *name) { + return 0; } static inline void bdi_debug_unregister(struct backing_dev_info *bdi) { -- 2.14.2
[PATCH v2 2/3] bdi: add error handle for bdi_debug_register
In order to make error handle more cleaner we call bdi_debug_register before set state to WB_registered, that we can avoid call bdi_unregister in release_bdi(). Signed-off-by: weiping zhang--- mm/backing-dev.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mm/backing-dev.c b/mm/backing-dev.c index b5f940ce0143..84b2dc76f140 100644 --- a/mm/backing-dev.c +++ b/mm/backing-dev.c @@ -882,10 +882,13 @@ int bdi_register_va(struct backing_dev_info *bdi, const char *fmt, va_list args) if (IS_ERR(dev)) return PTR_ERR(dev); + if (bdi_debug_register(bdi, dev_name(dev))) { + device_destroy(bdi_class, dev->devt); + return -ENOMEM; + } cgwb_bdi_register(bdi); bdi->dev = dev; - bdi_debug_register(bdi, dev_name(dev)); set_bit(WB_registered, >wb.state); spin_lock_bh(_lock); -- 2.14.2
[PATCH v2 0/3] add error handle for bdi debugfs register
Hello, Change since V1: * remove the patch for bdi_debug_init(), because patch1 add a check for bdi_debug_root * remove bdi_put in bdi_register, this functions has two callers: caller1: mtd_bdi_init->bdi_register, bdi_put if register fail caller2: device_add_disk->bdi_register_owner->bdi_register, this call stack need more safety cleanup, so patch3 add an WARN_ON. weiping zhang (3): bdi: convert bdi_debug_register to int bdi: add error handle for bdi_debug_register block: add WARN_ON if bdi register fail block/genhd.c| 2 +- mm/backing-dev.c | 22 +++--- 2 files changed, 20 insertions(+), 4 deletions(-) -- 2.14.2
Re: [PATCH 4/4] block: add WARN_ON if bdi register fail
On Mon, Oct 30, 2017 at 02:14:30PM +0100, Jan Kara wrote: > On Fri 27-10-17 01:36:42, weiping zhang wrote: > > device_add_disk need do more safety error handle, so this patch just > > add WARN_ON. > > > > Signed-off-by: weiping zhang> > --- > > block/genhd.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/block/genhd.c b/block/genhd.c > > index dd305c65ffb0..cb55eea821eb 100644 > > --- a/block/genhd.c > > +++ b/block/genhd.c > > @@ -660,7 +660,9 @@ void device_add_disk(struct device *parent, struct > > gendisk *disk) > > > > /* Register BDI before referencing it from bdev */ > > bdi = disk->queue->backing_dev_info; > > - bdi_register_owner(bdi, disk_to_dev(disk)); > > + retval = bdi_register_owner(bdi, disk_to_dev(disk)); > > + if (retval) > > + WARN_ON(1); > > Just a nit: You can do > > WARN_ON(retval); > > Otherwise you can add: > > Reviewed-by: Jan Kara > more claner, I'll apply at V2, Thanks -- weiping
Re: [PATCH 3/4] bdi: add error handle for bdi_debug_register
On Mon, Oct 30, 2017 at 02:10:16PM +0100, Jan Kara wrote: > On Fri 27-10-17 01:36:14, weiping zhang wrote: > > In order to make error handle more cleaner we call bdi_debug_register > > before set state to WB_registered, that we can avoid call bdi_unregister > > in release_bdi(). > > > > Signed-off-by: weiping zhang> > --- > > mm/backing-dev.c | 7 ++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/mm/backing-dev.c b/mm/backing-dev.c > > index e9d6a1ede12b..54396d53f471 100644 > > --- a/mm/backing-dev.c > > +++ b/mm/backing-dev.c > > @@ -893,10 +893,13 @@ int bdi_register_va(struct backing_dev_info *bdi, > > const char *fmt, va_list args) > > if (IS_ERR(dev)) > > return PTR_ERR(dev); > > > > + if (bdi_debug_register(bdi, dev_name(dev))) { > > + device_destroy(bdi_class, dev->devt); > > + return -ENOMEM; > > + } > > cgwb_bdi_register(bdi); > > bdi->dev = dev; > > > > - bdi_debug_register(bdi, dev_name(dev)); > > set_bit(WB_registered, >wb.state); > > > > spin_lock_bh(_lock); > > @@ -916,6 +919,8 @@ int bdi_register(struct backing_dev_info *bdi, const > > char *fmt, ...) > > va_start(args, fmt); > > ret = bdi_register_va(bdi, fmt, args); > > va_end(args); > > + if (ret) > > + bdi_put(bdi); > > Why do you drop bdi reference here in case of error? We didn't do it > previously if bdi_register_va() failed for other reasons... > At first I want add cleanup, because device_add_disk->bdi_register_owner->bdi_register doen't do clanup. But I notice that mtd_bdi_init also call bdi_register and do cleanup, so this bdi_put() is wrong. I'll remove it at V2. Thanks a lot. -- weiping
Re: [PATCH 1/4] bdi: add check for bdi_debug_root
On Mon, Oct 30, 2017 at 02:00:28PM +0100, Jan Kara wrote: > On Fri 27-10-17 01:35:36, weiping zhang wrote: > > this patch add a check for bdi_debug_root and do error handle for it. > > we should make sure it was created success, otherwise when add new > > block device's bdi folder(eg, 8:0) will be create a debugfs root directory. > > > > Signed-off-by: weiping zhang> > --- > > mm/backing-dev.c | 17 ++--- > > 1 file changed, 14 insertions(+), 3 deletions(-) > > These functions get called only on system boot - ENOMEM in those cases is > generally considered fatal and oopsing is acceptable result. So I don't > think this patch is needed. > OK, I drop this patch. Thanks a ton.
[PATCH] bcache: stop writeback thread after detaching
From: Tang JunhuiCurrently, when a cached device detaching from cache, writeback thread is not stopped, and writeback_rate_update work is not canceled. For example, after bellow command: echo 1 >/sys/block/sdb/bcache/detach you can still see the writeback thread. Then you attach the device to the cache again, bcache will create another writeback thread, for example, after bellow command: echo ba0fb5cd-658a-4533-9806-6ce166d883b9 > /sys/block/sdb/bcache/attach then you will see 2 writeback threads. This patch stops writeback thread and cancels writeback_rate_update work when cached device detaching from cache. Signed-off-by: Tang Junhui --- drivers/md/bcache/super.c | 6 ++ 1 file changed, 6 insertions(+) mode change 100644 => 100755 drivers/md/bcache/super.c diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c old mode 100644 new mode 100755 index 8352fad..bf79892 --- a/drivers/md/bcache/super.c +++ b/drivers/md/bcache/super.c @@ -891,6 +891,12 @@ static void cached_dev_detach_finish(struct work_struct *w) BUG_ON(!test_bit(BCACHE_DEV_DETACHING, >disk.flags)); BUG_ON(atomic_read(>count)); + cancel_delayed_work_sync(>writeback_rate_update); + if (!IS_ERR_OR_NULL(dc->writeback_thread)) { + kthread_stop(dc->writeback_thread); + dc->writeback_thread = NULL; + } + mutex_lock(_register_lock); memset(>sb.set_uuid, 0, 16); -- 1.8.3.1
[PATCH] ide:ide-cd: fix kernel panic resulting from missing scsi_req_init
Since we split the scsi_request out of struct request, while the standard prep_rq_fn builds 10 byte cmds, it missed to invoke scsi_req_init() to initialize certain fields of a scsi_request structure (.__cmd[], .cmd, .cmd_len and .sense_len but no other members of struct scsi_request). An example panic on virtual machines (qemu/virtualbox) to boot from IDE cdrom: ... [8.754381] Call Trace: [8.755419] blk_peek_request+0x182/0x2e0 [8.755863] blk_fetch_request+0x1c/0x40 [8.756148] ? ktime_get+0x40/0xa0 [8.756385] do_ide_request+0x37d/0x660 [8.756704] ? cfq_group_service_tree_add+0x98/0xc0 [8.757011] ? cfq_service_tree_add+0x1e5/0x2c0 [8.757313] ? ktime_get+0x40/0xa0 [8.757544] __blk_run_queue+0x3d/0x60 [8.757837] queue_unplugged+0x2f/0xc0 [8.758088] blk_flush_plug_list+0x1f4/0x240 [8.758362] blk_finish_plug+0x2c/0x40 ... [8.770906] RIP: ide_cdrom_prep_fn+0x63/0x180 RSP: 92aec018bae8 [8.772329] ---[ end trace 6408481e551a85c9 ]--- ... Fixes: 82ed4db499b8 ("block: split scsi_request out of struct request") Signed-off-by: Hongxu Jia--- drivers/ide/ide-cd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c index 81e18f96..a7355ab 100644 --- a/drivers/ide/ide-cd.c +++ b/drivers/ide/ide-cd.c @@ -1328,6 +1328,7 @@ static int ide_cdrom_prep_fs(struct request_queue *q, struct request *rq) unsigned long blocks = blk_rq_sectors(rq) / (hard_sect >> 9); struct scsi_request *req = scsi_req(rq); + scsi_req_init(req); memset(req->cmd, 0, BLK_MAX_CDB); if (rq_data_dir(rq) == READ) -- 2.7.4