Re: [PATCH V2 0/2] block: remove unnecessary RESTART

2017-10-31 Thread Ming Lei 
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

2017-10-31 Thread Bart Van Assche 
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

2017-10-31 Thread Ming Lei 
On Tue, Oct 31, 2017 at 07:53:03PM -0600, Jens Axboe wrote:
> 
> > On Oct 31, 2017, at 7:46 PM, Ming Lei  wrote:
> > 
> >> 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

2017-10-31 Thread Jens Axboe 

> On Oct 31, 2017, at 7:46 PM, Ming Lei  wrote:
> 
>> 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

2017-10-31 Thread Ming Lei 
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

2017-10-31 Thread Hongxu Jia 

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.

2017-10-31 Thread Jens Axboe 
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.

2017-10-31 Thread Scott Bauer 
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

2017-10-31 Thread Jens Axboe 
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

2017-10-31 Thread Bart Van Assche 
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

2017-10-31 Thread weiping zhang 
device_add_disk need do more safety error handle, so this patch just
add WARN_ON.

Reviewed-by: Jan Kara 
Signed-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

2017-10-31 Thread weiping zhang 
Convert bdi_debug_register to int and then do error handle for it.

Reviewed-by: Jan Kara 
Signed-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

2017-10-31 Thread weiping zhang 
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

2017-10-31 Thread weiping zhang 
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

2017-10-31 Thread weiping zhang 
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

2017-10-31 Thread weiping zhang 
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

2017-10-31 Thread weiping zhang 
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

2017-10-31 Thread tang . junhui 
From: Tang Junhui 

Currently, 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

2017-10-31 Thread Hongxu Jia 
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