* Re: [PATCH v3] scsi, block: fix duplicate bdi name registration crashes
From: Bart Van Assche @ 2017-02-01 22:23 UTC (permalink / raw)
To: dan.j.williams@intel.com, martin.petersen@oracle.com
Cc: linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
hch@lst.de, axboe@kernel.dk, linux-scsi@vger.kernel.org,
osandov@osandov.com, James.Bottomley@hansenpartnership.com,
osandov@fb.com, jack@suse.cz
In-Reply-To: <148598661794.11527.10024529085859239448.stgit@dwillia2-desk3.amr.corp.intel.com>
On Wed, 2017-02-01 at 14:05 -0800, Dan Williams wrote:
> Warnings of the following form occur because scsi reuses a devt number
> while the block layer still has it referenced as the name of the bdi
Thanks!
Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>=
^ permalink raw reply
* [PATCH v3] scsi, block: fix duplicate bdi name registration crashes
From: Dan Williams @ 2017-02-01 22:05 UTC (permalink / raw)
To: martin.petersen
Cc: Jens Axboe, linux-block, Jan Kara, linux-scsi, linux-kernel,
Christoph Hellwig, James Bottomley, Bart Van Assche,
Omar Sandoval, Omar Sandoval
Warnings of the following form occur because scsi reuses a devt number
while the block layer still has it referenced as the name of the bdi
[1]:
WARNING: CPU: 1 PID: 93 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x62/0x80
sysfs: cannot create duplicate filename '/devices/virtual/bdi/8:192'
[..]
Call Trace:
dump_stack+0x86/0xc3
__warn+0xcb/0xf0
warn_slowpath_fmt+0x5f/0x80
? kernfs_path_from_node+0x4f/0x60
sysfs_warn_dup+0x62/0x80
sysfs_create_dir_ns+0x77/0x90
kobject_add_internal+0xb2/0x350
kobject_add+0x75/0xd0
device_add+0x15a/0x650
device_create_groups_vargs+0xe0/0xf0
device_create_vargs+0x1c/0x20
bdi_register+0x90/0x240
? lockdep_init_map+0x57/0x200
bdi_register_owner+0x36/0x60
device_add_disk+0x1bb/0x4e0
? __pm_runtime_use_autosuspend+0x5c/0x70
sd_probe_async+0x10d/0x1c0
async_run_entry_fn+0x39/0x170
This is a brute-force fix to pass the devt release information from
sd_probe() to the locations where we register the bdi,
device_add_disk(), and unregister the bdi, blk_cleanup_queue().
Thanks to Omar for the quick reproducer script [2]. This patch survives
where an unmodified kernel fails in a few seconds.
[1]: https://marc.info/?l=linux-scsi&m=147116857810716&w=4
[2]: http://marc.info/?l=linux-block&m=148554717109098&w=2
Cc: James Bottomley <James.Bottomley@hansenpartnership.com>
Cc: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Jan Kara <jack@suse.cz>
Reported-by: Omar Sandoval <osandov@osandov.com>
Tested-by: Omar Sandoval <osandov@fb.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Changes in v3:
* un-inline {get,put}_disk_devt() (Bart)
* added Omar's tested-by from v2
Changes in v2:
* rebased on top of Jan's bdi lifetime series
* replace kref_{get,put}() with atomic_{inc,dec_and_test} (Christoph)
block/blk-core.c | 1 +
block/genhd.c | 21 +++++++++++++++++++++
drivers/scsi/sd.c | 41 +++++++++++++++++++++++++++++++++--------
include/linux/blkdev.h | 1 +
include/linux/genhd.h | 8 ++++++++
5 files changed, 64 insertions(+), 8 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 84fabb51714a..0cd6b3c4b41c 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -595,6 +595,7 @@ void blk_cleanup_queue(struct request_queue *q)
spin_unlock_irq(lock);
bdi_unregister(q->backing_dev_info);
+ put_disk_devt(q->disk_devt);
/* @q is and will stay empty, shutdown and put */
blk_put_queue(q);
diff --git a/block/genhd.c b/block/genhd.c
index d9ccd42f3675..3631cd480295 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -572,6 +572,20 @@ static void register_disk(struct device *parent, struct gendisk *disk)
disk_part_iter_exit(&piter);
}
+void put_disk_devt(struct disk_devt *disk_devt)
+{
+ if (disk_devt && atomic_dec_and_test(&disk_devt->count))
+ disk_devt->release(disk_devt);
+}
+EXPORT_SYMBOL(put_disk_devt);
+
+void get_disk_devt(struct disk_devt *disk_devt)
+{
+ if (disk_devt)
+ atomic_inc(&disk_devt->count);
+}
+EXPORT_SYMBOL(get_disk_devt);
+
/**
* device_add_disk - add partitioning information to kernel list
* @parent: parent device for the disk
@@ -612,6 +626,13 @@ void device_add_disk(struct device *parent, struct gendisk *disk)
disk_alloc_events(disk);
+ /*
+ * Take a reference on the devt and assign it to queue since it
+ * must not be reallocated while the bdi is registered
+ */
+ disk->queue->disk_devt = disk->disk_devt;
+ get_disk_devt(disk->disk_devt);
+
/* Register BDI before referencing it from bdev */
bdi = disk->queue->backing_dev_info;
bdi_register_owner(bdi, disk_to_dev(disk));
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 0b09638fa39b..102111e730ce 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3067,6 +3067,23 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
put_device(&sdkp->dev);
}
+struct sd_devt {
+ int idx;
+ struct disk_devt disk_devt;
+};
+
+void sd_devt_release(struct disk_devt *disk_devt)
+{
+ struct sd_devt *sd_devt = container_of(disk_devt, struct sd_devt,
+ disk_devt);
+
+ spin_lock(&sd_index_lock);
+ ida_remove(&sd_index_ida, sd_devt->idx);
+ spin_unlock(&sd_index_lock);
+
+ kfree(sd_devt);
+}
+
/**
* sd_probe - called during driver initialization and whenever a
* new scsi device is attached to the system. It is called once
@@ -3088,6 +3105,7 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
static int sd_probe(struct device *dev)
{
struct scsi_device *sdp = to_scsi_device(dev);
+ struct sd_devt *sd_devt;
struct scsi_disk *sdkp;
struct gendisk *gd;
int index;
@@ -3113,9 +3131,13 @@ static int sd_probe(struct device *dev)
if (!sdkp)
goto out;
+ sd_devt = kzalloc(sizeof(*sd_devt), GFP_KERNEL);
+ if (!sd_devt)
+ goto out_free;
+
gd = alloc_disk(SD_MINORS);
if (!gd)
- goto out_free;
+ goto out_free_devt;
do {
if (!ida_pre_get(&sd_index_ida, GFP_KERNEL))
@@ -3131,6 +3153,11 @@ static int sd_probe(struct device *dev)
goto out_put;
}
+ atomic_set(&sd_devt->disk_devt.count, 1);
+ sd_devt->disk_devt.release = sd_devt_release;
+ sd_devt->idx = index;
+ gd->disk_devt = &sd_devt->disk_devt;
+
error = sd_format_disk_name("sd", index, gd->disk_name, DISK_NAME_LEN);
if (error) {
sdev_printk(KERN_WARNING, sdp, "SCSI disk (sd) name length exceeded.\n");
@@ -3170,13 +3197,14 @@ static int sd_probe(struct device *dev)
return 0;
out_free_index:
- spin_lock(&sd_index_lock);
- ida_remove(&sd_index_ida, index);
- spin_unlock(&sd_index_lock);
+ put_disk_devt(&sd_devt->disk_devt);
+ sd_devt = NULL;
out_put:
put_disk(gd);
out_free:
kfree(sdkp);
+ out_free_devt:
+ kfree(sd_devt);
out:
scsi_autopm_put_device(sdp);
return error;
@@ -3235,10 +3263,7 @@ static void scsi_disk_release(struct device *dev)
struct scsi_disk *sdkp = to_scsi_disk(dev);
struct gendisk *disk = sdkp->disk;
- spin_lock(&sd_index_lock);
- ida_remove(&sd_index_ida, sdkp->index);
- spin_unlock(&sd_index_lock);
-
+ put_disk_devt(disk->disk_devt);
disk->private_data = NULL;
put_disk(disk);
put_device(&sdkp->device->sdev_gendev);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 3c0ff78b1219..53195a4d597a 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -433,6 +433,7 @@ struct request_queue {
struct delayed_work delay_work;
struct backing_dev_info *backing_dev_info;
+ struct disk_devt *disk_devt;
/*
* The queue owner gets to use this for whatever they like.
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 76f39754e7b0..a999d281a2f1 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -167,6 +167,13 @@ struct blk_integrity {
};
#endif /* CONFIG_BLK_DEV_INTEGRITY */
+struct disk_devt {
+ atomic_t count;
+ void (*release)(struct disk_devt *disk_devt);
+};
+
+void put_disk_devt(struct disk_devt *disk_devt);
+void get_disk_devt(struct disk_devt *disk_devt);
struct gendisk {
/* major, first_minor and minors are input parameters only,
@@ -176,6 +183,7 @@ struct gendisk {
int first_minor;
int minors; /* maximum number of minors, =1 for
* disks that can't be partitioned. */
+ struct disk_devt *disk_devt;
char disk_name[DISK_NAME_LEN]; /* name of major driver */
char *(*devnode)(struct gendisk *gd, umode_t *mode);
^ permalink raw reply related
* Re: [dm-devel] split scsi passthrough fields out of struct request V2
From: Bart Van Assche @ 2017-02-01 22:01 UTC (permalink / raw)
To: axboe@kernel.dk; +Cc: hch@lst.de, linux-block@vger.kernel.org
In-Reply-To: <7e963480-edf9-5687-25f3-83890373a26f@kernel.dk>
On Wed, 2017-02-01 at 09:13 -0800, Jens Axboe wrote:
> So that's changing the elevator - did this happen while heavy IO was
> going to the drive, or was it idle?
Hello Jens,
I think I figured out what was going on:
* Test 02-mq created scsi-mq SRP paths and multipathd created dm-mq device
nodes on top of these SRP paths.
* Test 02-sq started with removing the SRP paths and with switching the
SCSI and DM cores from mq to sq but did not remove the dm-mq device
nodes. When that test script next performed an SRP log in multipathd
failed to add the single queue SCSI devices to an existing dm-mq
device node. In other words, the dm-mq devices had queue_if_no_path
set but did not have any paths. Hence the lockup for requests sent
to these dm devices.
I have modified my tests scripts such that the dm device nodes from a
previous test are removed before a new test starts. Since I made that
change I haven't seen any I/O lockup. However, a new issue shows up
sporadically, an issue that I had not yet seen during any test with
a kernel tree from Linus:
[ 227.613440] general protection fault: 0000 [#1] SMP
[ 227.613495] Modules linked in: dm_service_time ib_srp scsi_transport_srp=
target_core_user uio target_core_pscsi target_core_file ib_srpt target_cor=
e_iblock target_core_mod brd netconsole xt_CHECKSUM iptable_mangle ipt_MASQ=
UERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat libcrc32c nf_c=
onntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject=
_ipv4 xt_tcpudp tun bridge stp llc ebtable_filter ebtables ip6table_filter =
ip6_tables iptable_filter ip_tables x_tables af_packet ib_ipoib msr rdma_uc=
m ib_ucm ib_uverbs ib_umad rdma_cm configfs ib_cm iw_cm mlx4_ib ib_core sb_=
edac edac_core x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel ipm=
i_ssif kvm irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmul=
ni_intel pcbc tg3 aesni_intel iTCO_wdt mlx4_core ptp iTCO_vendor_support dc=
dbas aes_x86_64 crypto_simd glue_helper pps_core cryptd pcspkr devlink ipmi=
_si libphy ipmi_devintf fjes ipmi_msghandler tpm_tis tpm_tis_core lpc_ich m=
ei_me mfd_core mei shpchp wmi tpm button hid_generic usbhid mgag200 i2c_alg=
o_bit drm_kms_helper syscopyarea sysfillrect sr_mod sysimgblt fb_sys_fops c=
drom ttm drm ehci_pci ehci_hcd usbcore usb_common sg dm_multipath dm_mod sc=
si_dh_rdac scsi_dh_emc scsi_dh_alua autofs4
[ 227.613774] CPU: 3 PID: 28 Comm: ksoftirqd/3 Not tainted 4.10.0-rc5-dbg+=
#1
[ 227.613840] Hardware name: Dell Inc. PowerEdge R430/03XKDV, BIOS 1.0.2 1=
1/17/2014
[ 227.613893] task: ffff880172a249c0 task.stack: ffffc90001aa8000
[ 227.613932] RIP: 0010:rq_completed+0x12/0x90 [dm_mod]
[ 227.613965] RSP: 0018:ffffc90001aabda8 EFLAGS: 00010246
[ 227.614006] RAX: 0000000000000000 RBX: 6b6b6b6b6b6b6b6b RCX: 00000000000=
00000
[ 227.614043] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 6b6b6b6b6b6=
b6b6b
[ 227.614074] RBP: ffffc90001aabdc0 R08: ffff8803825f4c38 R09: 00000000000=
00000
[ 227.614105] R10: 0000000000000000 R11: 0000000000000000 R12: 00000000000=
00000
[ 227.614137] R13: 0000000000000000 R14: ffffffff81c05120 R15: 00000000000=
00004
[ 227.614170] FS: 0000000000000000(0000) GS:ffff88046f2c0000(0000) knlGS:=
0000000000000000
[ 227.614209] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 227.614239] CR2: 0000557e28bc20d0 CR3: 000000038594e000 CR4: 00000000001=
406e0
[ 227.614268] Call Trace:
[ 227.614301] dm_softirq_done+0xe6/0x1e0 [dm_mod]
[ 227.614337] blk_done_softirq+0x88/0xa0
[ 227.614369] __do_softirq+0xba/0x4c0
[ 227.614470] run_ksoftirqd+0x1a/0x50
[ 227.614499] smpboot_thread_fn+0x123/0x1e0
[ 227.614529] kthread+0x107/0x140
[ 227.614624] ret_from_fork+0x2e/0x40
[ 227.614648] Code: ff ff 31 f6 48 89 c7 e8 cd 0e 2f e1 5d c3 90 66 2e 0f =
1f 84 00 00 00 00 00 55 48 63 f6 48 89 e5 41 55 41 89 d5 41 54 53 48 89 fb =
<4c> 8b a7 88 02 00 00 f0 ff 8c b7 50 03 00 00 e8 ba 43 ff ff 85=20
[ 227.614738] RIP: rq_completed+0x12/0x90 [dm_mod] RSP: ffffc90001aabda8
(gdb) list *(rq_completed+0x12)
0xdd12 is in rq_completed (drivers/md/dm-rq.c:187).
182 * the md may be freed in dm_put() at the end of this function.
183 * Or do dm_get() before calling this function and dm_put() later.
184 */
185 static void rq_completed(struct mapped_device *md, int rw, bool run=
_queue)
186 {
187 struct request_queue *q =3D md->queue;
188 unsigned long flags;
189
190 atomic_dec(&md->pending[rw]);
191
(gdb) disas rq_completed
Dump of assembler code for function rq_completed:
0x000000000000dd00 <+0>: push %rbp
0x000000000000dd01 <+1>: movslq %esi,%rsi
0x000000000000dd04 <+4>: mov %rsp,%rbp
0x000000000000dd07 <+7>: push %r13
0x000000000000dd09 <+9>: mov %edx,%r13d
0x000000000000dd0c <+12>: push %r12
0x000000000000dd0e <+14>: push %rbx
0x000000000000dd0f <+15>: mov %rdi,%rbx
0x000000000000dd12 <+18>: mov 0x288(%rdi),%r12
0x000000000000dd19 <+25>: lock decl 0x350(%rdi,%rsi,4)
So this was caused by an attempt to dereference %rdi =3D 0x6b6b6b6b6b6b6b6b=
.
Hence this is probably a use-after-free of struct mapped_device.
Bart.=
^ permalink raw reply
* [PATCH] nbd: use an idr to keep track of nbd devices
From: Josef Bacik @ 2017-02-01 21:11 UTC (permalink / raw)
To: axboe, linux-block, kernel-team
To prepare for dynamically adding new nbd devices to the system switch
from using an array for the nbd devices and instead use an idr. This
copies what loop does for keeping track of its devices.
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
drivers/block/nbd.c | 213 ++++++++++++++++++++++++++++------------------------
1 file changed, 115 insertions(+), 98 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 9fe9763..96f7681 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -41,6 +41,9 @@
#include <linux/nbd.h>
+static DEFINE_IDR(nbd_index_idr);
+static DEFINE_MUTEX(nbd_index_mutex);
+
struct nbd_sock {
struct socket *sock;
struct mutex tx_lock;
@@ -89,9 +92,9 @@ static struct dentry *nbd_dbg_dir;
#define NBD_MAGIC 0x68797548
static unsigned int nbds_max = 16;
-static struct nbd_device *nbd_dev;
static int max_part;
static struct workqueue_struct *recv_workqueue;
+static int part_shift;
static inline struct device *nbd_to_dev(struct nbd_device *nbd)
{
@@ -997,6 +1000,103 @@ static struct blk_mq_ops nbd_mq_ops = {
.timeout = nbd_xmit_timeout,
};
+static void nbd_dev_remove(struct nbd_device *nbd)
+{
+ struct gendisk *disk = nbd->disk;
+ nbd->magic = 0;
+ if (disk) {
+ del_gendisk(disk);
+ blk_cleanup_queue(disk->queue);
+ blk_mq_free_tag_set(&nbd->tag_set);
+ put_disk(disk);
+ }
+ kfree(nbd);
+}
+
+static int nbd_dev_add(int index)
+{
+ struct nbd_device *nbd;
+ struct gendisk *disk;
+ struct request_queue *q;
+ int err = -ENOMEM;
+
+ nbd = kzalloc(sizeof(struct nbd_device), GFP_KERNEL);
+ if (!nbd)
+ goto out;
+
+ disk = alloc_disk(1 << part_shift);
+ if (!disk)
+ goto out_free_nbd;
+
+ if (index >= 0) {
+ err = idr_alloc(&nbd_index_idr, nbd, index, index + 1,
+ GFP_KERNEL);
+ if (err == -ENOSPC)
+ err = -EEXIST;
+ } else {
+ err = idr_alloc(&nbd_index_idr, nbd, 0, 0, GFP_KERNEL);
+ if (err >= 0)
+ index = err;
+ }
+ if (err < 0)
+ goto out_free_disk;
+
+ nbd->disk = disk;
+ nbd->tag_set.ops = &nbd_mq_ops;
+ nbd->tag_set.nr_hw_queues = 1;
+ nbd->tag_set.queue_depth = 128;
+ nbd->tag_set.numa_node = NUMA_NO_NODE;
+ nbd->tag_set.cmd_size = sizeof(struct nbd_cmd);
+ nbd->tag_set.flags = BLK_MQ_F_SHOULD_MERGE |
+ BLK_MQ_F_SG_MERGE | BLK_MQ_F_BLOCKING;
+ nbd->tag_set.driver_data = nbd;
+
+ err = blk_mq_alloc_tag_set(&nbd->tag_set);
+ if (err)
+ goto out_free_idr;
+
+ q = blk_mq_init_queue(&nbd->tag_set);
+ if (IS_ERR(q)) {
+ err = PTR_ERR(q);
+ goto out_free_tags;
+ }
+ disk->queue = q;
+
+ /*
+ * Tell the block layer that we are not a rotational device
+ */
+ queue_flag_set_unlocked(QUEUE_FLAG_NONROT, disk->queue);
+ queue_flag_clear_unlocked(QUEUE_FLAG_ADD_RANDOM, disk->queue);
+ disk->queue->limits.discard_granularity = 512;
+ blk_queue_max_discard_sectors(disk->queue, UINT_MAX);
+ disk->queue->limits.discard_zeroes_data = 0;
+ blk_queue_max_hw_sectors(disk->queue, 65536);
+ disk->queue->limits.max_sectors = 256;
+
+ nbd->magic = NBD_MAGIC;
+ mutex_init(&nbd->config_lock);
+ disk->major = NBD_MAJOR;
+ disk->first_minor = index << part_shift;
+ disk->fops = &nbd_fops;
+ disk->private_data = nbd;
+ sprintf(disk->disk_name, "nbd%d", index);
+ init_waitqueue_head(&nbd->recv_wq);
+ nbd_reset(nbd);
+ add_disk(disk);
+ return index;
+
+out_free_tags:
+ blk_mq_free_tag_set(&nbd->tag_set);
+out_free_idr:
+ idr_remove(&nbd_index_idr, index);
+out_free_disk:
+ put_disk(disk);
+out_free_nbd:
+ kfree(nbd);
+out:
+ return err;
+}
+
/*
* And here should be modules and kernel interface
* (Just smiley confuses emacs :-)
@@ -1004,9 +1104,7 @@ static struct blk_mq_ops nbd_mq_ops = {
static int __init nbd_init(void)
{
- int err = -ENOMEM;
int i;
- int part_shift;
BUILD_BUG_ON(sizeof(struct nbd_request) != 28);
@@ -1040,114 +1138,33 @@ static int __init nbd_init(void)
if (!recv_workqueue)
return -ENOMEM;
- nbd_dev = kcalloc(nbds_max, sizeof(*nbd_dev), GFP_KERNEL);
- if (!nbd_dev) {
- destroy_workqueue(recv_workqueue);
- return -ENOMEM;
- }
-
- for (i = 0; i < nbds_max; i++) {
- struct request_queue *q;
- struct gendisk *disk = alloc_disk(1 << part_shift);
- if (!disk)
- goto out;
- nbd_dev[i].disk = disk;
-
- nbd_dev[i].tag_set.ops = &nbd_mq_ops;
- nbd_dev[i].tag_set.nr_hw_queues = 1;
- nbd_dev[i].tag_set.queue_depth = 128;
- nbd_dev[i].tag_set.numa_node = NUMA_NO_NODE;
- nbd_dev[i].tag_set.cmd_size = sizeof(struct nbd_cmd);
- nbd_dev[i].tag_set.flags = BLK_MQ_F_SHOULD_MERGE |
- BLK_MQ_F_SG_MERGE | BLK_MQ_F_BLOCKING;
- nbd_dev[i].tag_set.driver_data = &nbd_dev[i];
-
- err = blk_mq_alloc_tag_set(&nbd_dev[i].tag_set);
- if (err) {
- put_disk(disk);
- goto out;
- }
-
- /*
- * The new linux 2.5 block layer implementation requires
- * every gendisk to have its very own request_queue struct.
- * These structs are big so we dynamically allocate them.
- */
- q = blk_mq_init_queue(&nbd_dev[i].tag_set);
- if (IS_ERR(q)) {
- blk_mq_free_tag_set(&nbd_dev[i].tag_set);
- put_disk(disk);
- goto out;
- }
- disk->queue = q;
-
- /*
- * Tell the block layer that we are not a rotational device
- */
- queue_flag_set_unlocked(QUEUE_FLAG_NONROT, disk->queue);
- queue_flag_clear_unlocked(QUEUE_FLAG_ADD_RANDOM, disk->queue);
- disk->queue->limits.discard_granularity = 512;
- blk_queue_max_discard_sectors(disk->queue, UINT_MAX);
- disk->queue->limits.discard_zeroes_data = 0;
- blk_queue_max_hw_sectors(disk->queue, 65536);
- disk->queue->limits.max_sectors = 256;
- }
-
- if (register_blkdev(NBD_MAJOR, "nbd")) {
- err = -EIO;
- goto out;
- }
-
- printk(KERN_INFO "nbd: registered device at major %d\n", NBD_MAJOR);
+ if (register_blkdev(NBD_MAJOR, "nbd"))
+ return -EIO;
nbd_dbg_init();
- for (i = 0; i < nbds_max; i++) {
- struct gendisk *disk = nbd_dev[i].disk;
- nbd_dev[i].magic = NBD_MAGIC;
- mutex_init(&nbd_dev[i].config_lock);
- disk->major = NBD_MAJOR;
- disk->first_minor = i << part_shift;
- disk->fops = &nbd_fops;
- disk->private_data = &nbd_dev[i];
- sprintf(disk->disk_name, "nbd%d", i);
- init_waitqueue_head(&nbd_dev[i].recv_wq);
- nbd_reset(&nbd_dev[i]);
- add_disk(disk);
- }
+ mutex_lock(&nbd_index_mutex);
+ for (i = 0; i < nbds_max; i++)
+ nbd_dev_add(i);
+ mutex_unlock(&nbd_index_mutex);
+ return 0;
+}
+static int nbd_exit_cb(int id, void *ptr, void *data)
+{
+ struct nbd_device *nbd = ptr;
+ nbd_dev_remove(nbd);
return 0;
-out:
- while (i--) {
- blk_mq_free_tag_set(&nbd_dev[i].tag_set);
- blk_cleanup_queue(nbd_dev[i].disk->queue);
- put_disk(nbd_dev[i].disk);
- }
- kfree(nbd_dev);
- destroy_workqueue(recv_workqueue);
- return err;
}
static void __exit nbd_cleanup(void)
{
- int i;
-
nbd_dbg_close();
- for (i = 0; i < nbds_max; i++) {
- struct gendisk *disk = nbd_dev[i].disk;
- nbd_dev[i].magic = 0;
- if (disk) {
- del_gendisk(disk);
- blk_cleanup_queue(disk->queue);
- blk_mq_free_tag_set(&nbd_dev[i].tag_set);
- put_disk(disk);
- }
- }
+ idr_for_each(&nbd_index_idr, &nbd_exit_cb, NULL);
+ idr_destroy(&nbd_index_idr);
destroy_workqueue(recv_workqueue);
unregister_blkdev(NBD_MAJOR, "nbd");
- kfree(nbd_dev);
- printk(KERN_INFO "nbd: unregistered device at major %d\n", NBD_MAJOR);
}
module_init(nbd_init);
--
2.7.4
^ permalink raw reply related
* [PATCH] nbd: use our own workqueue for recv threads
From: Josef Bacik @ 2017-02-01 21:11 UTC (permalink / raw)
To: axboe, linux-block, kernel-team
Since we are in the memory reclaim path we need our recv work to be on a
workqueue that has WQ_MEM_RECLAIM set so we can avoid deadlocks. Also
set WQ_HIGHPRI since we are in the completion path for IO.
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
drivers/block/nbd.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 9fd06ee..9fe9763 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -91,6 +91,7 @@ static struct dentry *nbd_dbg_dir;
static unsigned int nbds_max = 16;
static struct nbd_device *nbd_dev;
static int max_part;
+static struct workqueue_struct *recv_workqueue;
static inline struct device *nbd_to_dev(struct nbd_device *nbd)
{
@@ -785,7 +786,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
INIT_WORK(&args[i].work, recv_work);
args[i].nbd = nbd;
args[i].index = i;
- queue_work(system_long_wq, &args[i].work);
+ queue_work(recv_workqueue, &args[i].work);
}
wait_event_interruptible(nbd->recv_wq,
atomic_read(&nbd->recv_threads) == 0);
@@ -1034,10 +1035,16 @@ static int __init nbd_init(void)
if (nbds_max > 1UL << (MINORBITS - part_shift))
return -EINVAL;
+ recv_workqueue = alloc_workqueue("knbd-recv",
+ WQ_MEM_RECLAIM | WQ_HIGHPRI, 0);
+ if (!recv_workqueue)
+ return -ENOMEM;
nbd_dev = kcalloc(nbds_max, sizeof(*nbd_dev), GFP_KERNEL);
- if (!nbd_dev)
+ if (!nbd_dev) {
+ destroy_workqueue(recv_workqueue);
return -ENOMEM;
+ }
for (i = 0; i < nbds_max; i++) {
struct request_queue *q;
@@ -1117,6 +1124,7 @@ static int __init nbd_init(void)
put_disk(nbd_dev[i].disk);
}
kfree(nbd_dev);
+ destroy_workqueue(recv_workqueue);
return err;
}
@@ -1136,6 +1144,7 @@ static void __exit nbd_cleanup(void)
put_disk(disk);
}
}
+ destroy_workqueue(recv_workqueue);
unregister_blkdev(NBD_MAJOR, "nbd");
kfree(nbd_dev);
printk(KERN_INFO "nbd: unregistered device at major %d\n", NBD_MAJOR);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2] scsi, block: fix duplicate bdi name registration crashes
From: Bart Van Assche @ 2017-02-01 20:35 UTC (permalink / raw)
To: dan.j.williams@intel.com, martin.petersen@oracle.com
Cc: linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
hch@lst.de, axboe@kernel.dk, linux-scsi@vger.kernel.org,
osandov@osandov.com, James.Bottomley@hansenpartnership.com,
jack@suse.cz
In-Reply-To: <148597749824.11392.9598040130844203664.stgit@dwillia2-desk3.amr.corp.intel.com>
On Wed, 2017-02-01 at 11:32 -0800, Dan Williams wrote:
> diff --git a/include/linux/genhd.h b/include/linux/genhd.h
> index 76f39754e7b0..27e7e12c0c2d 100644
> --- a/include/linux/genhd.h
> +++ b/include/linux/genhd.h
> @@ -167,6 +167,22 @@ struct blk_integrity {
> };
> =20
> #endif /* CONFIG_BLK_DEV_INTEGRITY */
> +struct disk_devt {
> + atomic_t count;
> + void (*release)(struct disk_devt *disk_devt);
> +};
> +
> +static inline void put_disk_devt(struct disk_devt *disk_devt)
> +{
> + if (disk_devt && atomic_dec_and_test(&disk_devt->count))
> + disk_devt->release(disk_devt);
> +}
> +
> +static inline void get_disk_devt(struct disk_devt *disk_devt)
> +{
> + if (disk_devt)
> + atomic_inc(&disk_devt->count);
> +}
The <linux/genhd.h> header file is included directly or indirectly in a
huge number of source files. Since neither get_disk_devt() nor
put_disk_devt() are called from the I/O path, please move these functions
from=A0<linux/genhd.h> into block/genhd.c to keep the compilation time of
the kernel as short as possible.
Thanks,
Bart.=
^ permalink raw reply
* Re: [PATCH v2] scsi, block: fix duplicate bdi name registration crashes
From: Omar Sandoval @ 2017-02-01 20:26 UTC (permalink / raw)
To: Dan Williams
Cc: martin.petersen, Jens Axboe, linux-block, Jan Kara, linux-scsi,
linux-kernel, James Bottomley, Bart Van Assche, Christoph Hellwig
In-Reply-To: <148597749824.11392.9598040130844203664.stgit@dwillia2-desk3.amr.corp.intel.com>
On Wed, Feb 01, 2017 at 11:32:12AM -0800, Dan Williams wrote:
> Warnings of the following form occur because scsi reuses a devt number
> while the block layer still has it referenced as the name of the bdi
> [1]:
>
> WARNING: CPU: 1 PID: 93 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x62/0x80
> sysfs: cannot create duplicate filename '/devices/virtual/bdi/8:192'
> [..]
> Call Trace:
> dump_stack+0x86/0xc3
> __warn+0xcb/0xf0
> warn_slowpath_fmt+0x5f/0x80
> ? kernfs_path_from_node+0x4f/0x60
> sysfs_warn_dup+0x62/0x80
> sysfs_create_dir_ns+0x77/0x90
> kobject_add_internal+0xb2/0x350
> kobject_add+0x75/0xd0
> device_add+0x15a/0x650
> device_create_groups_vargs+0xe0/0xf0
> device_create_vargs+0x1c/0x20
> bdi_register+0x90/0x240
> ? lockdep_init_map+0x57/0x200
> bdi_register_owner+0x36/0x60
> device_add_disk+0x1bb/0x4e0
> ? __pm_runtime_use_autosuspend+0x5c/0x70
> sd_probe_async+0x10d/0x1c0
> async_run_entry_fn+0x39/0x170
>
> This is a brute-force fix to pass the devt release information from
> sd_probe() to the locations where we register the bdi,
> device_add_disk(), and unregister the bdi, blk_cleanup_queue().
>
> Thanks to Omar for the quick reproducer script [2]. This patch survives
> where an unmodified kernel fails in a few seconds.
>
> [1]: https://marc.info/?l=linux-scsi&m=147116857810716&w=4
> [2]: http://marc.info/?l=linux-block&m=148554717109098&w=2
Thanks, Dan, this fixes it for me, too.
Tested-by: Omar Sandoval <osandov@fb.com>
> Cc: James Bottomley <James.Bottomley@hansenpartnership.com>
> Cc: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Jan Kara <jack@suse.cz>
> Reported-by: Omar Sandoval <osandov@osandov.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> Changes in v2:
> * rebased on top of Jan's bdi lifetime series
> * replace kref_{get,put}() with atomic_{inc,dec_and_test} (Christoph)
>
> block/blk-core.c | 1 +
> block/genhd.c | 7 +++++++
> drivers/scsi/sd.c | 41 +++++++++++++++++++++++++++++++++--------
> include/linux/blkdev.h | 1 +
> include/linux/genhd.h | 17 +++++++++++++++++
> 5 files changed, 59 insertions(+), 8 deletions(-)
^ permalink raw reply
* Re: [PATCH] block: Update comments that refer to __bio_map_user() and bio_map_user()
From: Jens Axboe @ 2017-02-01 19:33 UTC (permalink / raw)
To: Bart Van Assche; +Cc: linux-block, Christoph Hellwig, Ming Lei, Jens Axboe
In-Reply-To: <20170201162008.4628-1-bart.vanassche@sandisk.com>
On 02/01/2017 08:20 AM, Bart Van Assche wrote:
> Since __bio_map_user() and bio_map_user() have been removed, update
> the comments that still refer to these functions.
Applied, thanks Bart.
--
Jens Axboe
^ permalink raw reply
* [PATCH v2] scsi, block: fix duplicate bdi name registration crashes
From: Dan Williams @ 2017-02-01 19:32 UTC (permalink / raw)
To: martin.petersen
Cc: Jens Axboe, linux-block, Jan Kara, linux-scsi, linux-kernel,
James Bottomley, Bart Van Assche, Omar Sandoval,
Christoph Hellwig
Warnings of the following form occur because scsi reuses a devt number
while the block layer still has it referenced as the name of the bdi
[1]:
WARNING: CPU: 1 PID: 93 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x62/0x80
sysfs: cannot create duplicate filename '/devices/virtual/bdi/8:192'
[..]
Call Trace:
dump_stack+0x86/0xc3
__warn+0xcb/0xf0
warn_slowpath_fmt+0x5f/0x80
? kernfs_path_from_node+0x4f/0x60
sysfs_warn_dup+0x62/0x80
sysfs_create_dir_ns+0x77/0x90
kobject_add_internal+0xb2/0x350
kobject_add+0x75/0xd0
device_add+0x15a/0x650
device_create_groups_vargs+0xe0/0xf0
device_create_vargs+0x1c/0x20
bdi_register+0x90/0x240
? lockdep_init_map+0x57/0x200
bdi_register_owner+0x36/0x60
device_add_disk+0x1bb/0x4e0
? __pm_runtime_use_autosuspend+0x5c/0x70
sd_probe_async+0x10d/0x1c0
async_run_entry_fn+0x39/0x170
This is a brute-force fix to pass the devt release information from
sd_probe() to the locations where we register the bdi,
device_add_disk(), and unregister the bdi, blk_cleanup_queue().
Thanks to Omar for the quick reproducer script [2]. This patch survives
where an unmodified kernel fails in a few seconds.
[1]: https://marc.info/?l=linux-scsi&m=147116857810716&w=4
[2]: http://marc.info/?l=linux-block&m=148554717109098&w=2
Cc: James Bottomley <James.Bottomley@hansenpartnership.com>
Cc: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Jan Kara <jack@suse.cz>
Reported-by: Omar Sandoval <osandov@osandov.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Changes in v2:
* rebased on top of Jan's bdi lifetime series
* replace kref_{get,put}() with atomic_{inc,dec_and_test} (Christoph)
block/blk-core.c | 1 +
block/genhd.c | 7 +++++++
drivers/scsi/sd.c | 41 +++++++++++++++++++++++++++++++++--------
include/linux/blkdev.h | 1 +
include/linux/genhd.h | 17 +++++++++++++++++
5 files changed, 59 insertions(+), 8 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 84fabb51714a..0cd6b3c4b41c 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -595,6 +595,7 @@ void blk_cleanup_queue(struct request_queue *q)
spin_unlock_irq(lock);
bdi_unregister(q->backing_dev_info);
+ put_disk_devt(q->disk_devt);
/* @q is and will stay empty, shutdown and put */
blk_put_queue(q);
diff --git a/block/genhd.c b/block/genhd.c
index d9ccd42f3675..124499db04d6 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -612,6 +612,13 @@ void device_add_disk(struct device *parent, struct gendisk *disk)
disk_alloc_events(disk);
+ /*
+ * Take a reference on the devt and assign it to queue since it
+ * must not be reallocated while the bdi is registered
+ */
+ disk->queue->disk_devt = disk->disk_devt;
+ get_disk_devt(disk->disk_devt);
+
/* Register BDI before referencing it from bdev */
bdi = disk->queue->backing_dev_info;
bdi_register_owner(bdi, disk_to_dev(disk));
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 0b09638fa39b..102111e730ce 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3067,6 +3067,23 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
put_device(&sdkp->dev);
}
+struct sd_devt {
+ int idx;
+ struct disk_devt disk_devt;
+};
+
+void sd_devt_release(struct disk_devt *disk_devt)
+{
+ struct sd_devt *sd_devt = container_of(disk_devt, struct sd_devt,
+ disk_devt);
+
+ spin_lock(&sd_index_lock);
+ ida_remove(&sd_index_ida, sd_devt->idx);
+ spin_unlock(&sd_index_lock);
+
+ kfree(sd_devt);
+}
+
/**
* sd_probe - called during driver initialization and whenever a
* new scsi device is attached to the system. It is called once
@@ -3088,6 +3105,7 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
static int sd_probe(struct device *dev)
{
struct scsi_device *sdp = to_scsi_device(dev);
+ struct sd_devt *sd_devt;
struct scsi_disk *sdkp;
struct gendisk *gd;
int index;
@@ -3113,9 +3131,13 @@ static int sd_probe(struct device *dev)
if (!sdkp)
goto out;
+ sd_devt = kzalloc(sizeof(*sd_devt), GFP_KERNEL);
+ if (!sd_devt)
+ goto out_free;
+
gd = alloc_disk(SD_MINORS);
if (!gd)
- goto out_free;
+ goto out_free_devt;
do {
if (!ida_pre_get(&sd_index_ida, GFP_KERNEL))
@@ -3131,6 +3153,11 @@ static int sd_probe(struct device *dev)
goto out_put;
}
+ atomic_set(&sd_devt->disk_devt.count, 1);
+ sd_devt->disk_devt.release = sd_devt_release;
+ sd_devt->idx = index;
+ gd->disk_devt = &sd_devt->disk_devt;
+
error = sd_format_disk_name("sd", index, gd->disk_name, DISK_NAME_LEN);
if (error) {
sdev_printk(KERN_WARNING, sdp, "SCSI disk (sd) name length exceeded.\n");
@@ -3170,13 +3197,14 @@ static int sd_probe(struct device *dev)
return 0;
out_free_index:
- spin_lock(&sd_index_lock);
- ida_remove(&sd_index_ida, index);
- spin_unlock(&sd_index_lock);
+ put_disk_devt(&sd_devt->disk_devt);
+ sd_devt = NULL;
out_put:
put_disk(gd);
out_free:
kfree(sdkp);
+ out_free_devt:
+ kfree(sd_devt);
out:
scsi_autopm_put_device(sdp);
return error;
@@ -3235,10 +3263,7 @@ static void scsi_disk_release(struct device *dev)
struct scsi_disk *sdkp = to_scsi_disk(dev);
struct gendisk *disk = sdkp->disk;
- spin_lock(&sd_index_lock);
- ida_remove(&sd_index_ida, sdkp->index);
- spin_unlock(&sd_index_lock);
-
+ put_disk_devt(disk->disk_devt);
disk->private_data = NULL;
put_disk(disk);
put_device(&sdkp->device->sdev_gendev);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 3c0ff78b1219..53195a4d597a 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -433,6 +433,7 @@ struct request_queue {
struct delayed_work delay_work;
struct backing_dev_info *backing_dev_info;
+ struct disk_devt *disk_devt;
/*
* The queue owner gets to use this for whatever they like.
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 76f39754e7b0..27e7e12c0c2d 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -167,6 +167,22 @@ struct blk_integrity {
};
#endif /* CONFIG_BLK_DEV_INTEGRITY */
+struct disk_devt {
+ atomic_t count;
+ void (*release)(struct disk_devt *disk_devt);
+};
+
+static inline void put_disk_devt(struct disk_devt *disk_devt)
+{
+ if (disk_devt && atomic_dec_and_test(&disk_devt->count))
+ disk_devt->release(disk_devt);
+}
+
+static inline void get_disk_devt(struct disk_devt *disk_devt)
+{
+ if (disk_devt)
+ atomic_inc(&disk_devt->count);
+}
struct gendisk {
/* major, first_minor and minors are input parameters only,
@@ -176,6 +192,7 @@ struct gendisk {
int first_minor;
int minors; /* maximum number of minors, =1 for
* disks that can't be partitioned. */
+ struct disk_devt *disk_devt;
char disk_name[DISK_NAME_LEN]; /* name of major driver */
char *(*devnode)(struct gendisk *gd, umode_t *mode);
^ permalink raw reply related
* Re: [PATCH 4/4] block: Make blk_get_backing_dev_info() safe without open bdev
From: Dan Williams @ 2017-02-01 19:25 UTC (permalink / raw)
To: Jan Kara
Cc: Jens Axboe, linux-block, Christoph Hellwig, Thiago Jung Bauermann,
Laurent Dufour
In-Reply-To: <20170131125429.14303-5-jack@suse.cz>
On Tue, Jan 31, 2017 at 4:54 AM, Jan Kara <jack@suse.cz> wrote:
> Currenly blk_get_backing_dev_info() is not safe to be called when the
> block device is not open as bdev->bd_disk is NULL in that case. However
> inode_to_bdi() uses this function and may be call called from flusher
> worker or other writeback related functions without bdev being open
> which leads to crashes such as:
>
> [113031.075540] Unable to handle kernel paging request for data at address 0x00000000
> [113031.075614] Faulting instruction address: 0xc0000000003692e0
> 0:mon> t
> [c0000000fb65f900] c00000000036cb6c writeback_sb_inodes+0x30c/0x590
> [c0000000fb65fa10] c00000000036ced4 __writeback_inodes_wb+0xe4/0x150
> [c0000000fb65fa70] c00000000036d33c wb_writeback+0x30c/0x450
> [c0000000fb65fb40] c00000000036e198 wb_workfn+0x268/0x580
> [c0000000fb65fc50] c0000000000f3470 process_one_work+0x1e0/0x590
> [c0000000fb65fce0] c0000000000f38c8 worker_thread+0xa8/0x660
> [c0000000fb65fd80] c0000000000fc4b0 kthread+0x110/0x130
> [c0000000fb65fe30] c0000000000098f0 ret_from_kernel_thread+0x5c/0x6c
> --- Exception: 0 at 0000000000000000
> 0:mon> e
> cpu 0x0: Vector: 300 (Data Access) at [c0000000fb65f620]
> pc: c0000000003692e0: locked_inode_to_wb_and_lock_list+0x50/0x290
> lr: c00000000036cb6c: writeback_sb_inodes+0x30c/0x590
> sp: c0000000fb65f8a0
> msr: 800000010280b033
> dar: 0
> dsisr: 40000000
> current = 0xc0000001d69be400
> paca = 0xc000000003480000 softe: 0 irq_happened: 0x01
> pid = 18689, comm = kworker/u16:10
>
> Fix the problem by grabbing reference to bdi on first open of the block
> device and drop the reference only once the inode is evicted from
> memory. This pins struct backing_dev_info in memory and thus fixes the
> crashes.
>
> Reported-by: Dan Williams <dan.j.williams@intel.com>
> Reported-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
Tested-by: Dan Williams <dan.j.williams@intel.com>
^ permalink raw reply
* Re: [PATCH 0/4] Four patches for the blk-mq debugfs code
From: Jens Axboe @ 2017-02-01 19:23 UTC (permalink / raw)
To: Bart Van Assche; +Cc: linux-block
In-Reply-To: <20170201182059.25601-1-bart.vanassche@sandisk.com>
On 02/01/2017 10:20 AM, Bart Van Assche wrote:
> Hello Jens,
>
> Please consider the four patches in this series for kernel v4.11.
Added, thanks Bart.
--
Jens Axboe
^ permalink raw reply
* Re: [dm-devel] split scsi passthrough fields out of struct request V2
From: Bart Van Assche @ 2017-02-01 19:21 UTC (permalink / raw)
To: axboe@kernel.dk; +Cc: linux-block@vger.kernel.org
In-Reply-To: <7e963480-edf9-5687-25f3-83890373a26f@kernel.dk>
On Wed, 2017-02-01 at 09:13 -0800, Jens Axboe wrote:
> So that's changing the elevator - did this happen while heavy IO was
> going to the drive, or was it idle?
I just ran into an I/O hang while running test 02-sq on top of kernel
v4.9.6. I will have a closer look at the dm code to see whether I can
find anything that is wrong in the dm code.
Bart.=
^ permalink raw reply
* Re: [PATCH V2 1/3] blk-mq: allocate blk_mq_tags and requests in correct node
From: Jens Axboe @ 2017-02-01 19:09 UTC (permalink / raw)
To: Shaohua Li, linux-kernel, linux-block; +Cc: bhelgaas, hch
In-Reply-To: <a52afb3e09691267f2240331ef0b9962f793703d.1485971427.git.shli@fb.com>
On 02/01/2017 09:53 AM, Shaohua Li wrote:
> blk_mq_tags/requests of specific hardware queue are mostly used in
> specific cpus, which might not be in the same numa node as disk. For
> example, a nvme card is in node 0. half hardware queue will be used by
> node 0, the other node 1.
All three patches look good to me. Bjorn, to avoid complications, if
you can review/ack patch #2, then I will queue it up through the block
tree for 4.11.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 4/4] blk-mq-debug: Introduce debugfs_create_files()
From: Omar Sandoval @ 2017-02-01 19:08 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Omar Sandoval
In-Reply-To: <20170201182059.25601-5-bart.vanassche@sandisk.com>
On Wed, Feb 01, 2017 at 10:20:59AM -0800, Bart Van Assche wrote:
> Replace the two debugfs_create_file() loops by a call to the new
> debugfs_create_files() function. Add an empty element at the end
> of the two attribute arrays such that the array size does not have
> to be passed to debugfs_create_files().
Reviewed-by: Omar Sandoval <osandov@fb.com>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Omar Sandoval <osandov@fb.com>
> ---
> block/blk-mq-debugfs.c | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
^ permalink raw reply
* Re: [PATCH 3/4] blk-mq-debug: Make show() operations interruptible
From: Omar Sandoval @ 2017-02-01 19:08 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Omar Sandoval
In-Reply-To: <20170201182059.25601-4-bart.vanassche@sandisk.com>
On Wed, Feb 01, 2017 at 10:20:58AM -0800, Bart Van Assche wrote:
> Allow users to interrupt show operations instead of making a user
> space process unkillable if ownership of q->sysfs_lock cannot be
> obtained.
Reviewed-by: Omar Sandoval <osandov@fb.com>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Omar Sandoval <osandov@fb.com>
> ---
> block/blk-mq-debugfs.c | 34 ++++++++++++++++++++++++++--------
> 1 file changed, 26 insertions(+), 8 deletions(-)
^ permalink raw reply
* Re: [PATCH 2/4] blk-mq-debug: Avoid that sparse complains about req_flags_t usage
From: Omar Sandoval @ 2017-02-01 19:08 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Omar Sandoval
In-Reply-To: <20170201182059.25601-3-bart.vanassche@sandisk.com>
On Wed, Feb 01, 2017 at 10:20:57AM -0800, Bart Van Assche wrote:
> Avoid that sparse reports the following complaints:
>
> block/elevator.c:541:29: warning: incorrect type in assignment (different base types)
> block/elevator.c:541:29: expected bool [unsigned] [usertype] next_sorted
> block/elevator.c:541:29: got restricted req_flags_t
>
> block/blk-mq-debugfs.c:92:54: warning: cast from restricted req_flags_t
Reviewed-by: Omar Sandoval <osandov@fb.com>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Omar Sandoval <osandov@fb.com>
> ---
> block/blk-mq-debugfs.c | 3 ++-
> block/elevator.c | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
^ permalink raw reply
* Re: [PATCH 1/4] blk-mq-debugfs: Add missing __acquires() / __releases() annotations
From: Omar Sandoval @ 2017-02-01 19:07 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Omar Sandoval
In-Reply-To: <20170201182059.25601-2-bart.vanassche@sandisk.com>
On Wed, Feb 01, 2017 at 10:20:56AM -0800, Bart Van Assche wrote:
> This patch avoids that sparse complains about lock imbalances.
Reviewed-by: Omar Sandoval <osandov@fb.com>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Omar Sandoval <osandov@fb.com>
> ---
> block/blk-mq-debugfs.c | 4 ++++
> 1 file changed, 4 insertions(+)
^ permalink raw reply
* [PATCH 4/4] blk-mq-debug: Introduce debugfs_create_files()
From: Bart Van Assche @ 2017-02-01 18:20 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval
In-Reply-To: <20170201182059.25601-1-bart.vanassche@sandisk.com>
Replace the two debugfs_create_file() loops by a call to the new
debugfs_create_files() function. Add an empty element at the end
of the two attribute arrays such that the array size does not have
to be passed to debugfs_create_files().
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Omar Sandoval <osandov@fb.com>
---
block/blk-mq-debugfs.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 846943728939..b3bc9f02a5f5 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -653,6 +653,7 @@ static const struct blk_mq_debugfs_attr blk_mq_debugfs_hctx_attrs[] = {
{"queued", 0600, &hctx_queued_fops},
{"run", 0600, &hctx_run_fops},
{"active", 0400, &hctx_active_fops},
+ {},
};
static const struct blk_mq_debugfs_attr blk_mq_debugfs_ctx_attrs[] = {
@@ -660,6 +661,7 @@ static const struct blk_mq_debugfs_attr blk_mq_debugfs_ctx_attrs[] = {
{"dispatched", 0600, &ctx_dispatched_fops},
{"merged", 0600, &ctx_merged_fops},
{"completed", 0600, &ctx_completed_fops},
+ {},
};
int blk_mq_debugfs_register(struct request_queue *q, const char *name)
@@ -688,27 +690,31 @@ void blk_mq_debugfs_unregister(struct request_queue *q)
q->debugfs_dir = NULL;
}
+static bool debugfs_create_files(struct dentry *parent, void *data,
+ const struct blk_mq_debugfs_attr *attr)
+{
+ for (; attr->name; attr++) {
+ if (!debugfs_create_file(attr->name, attr->mode, parent,
+ data, attr->fops))
+ return false;
+ }
+ return true;
+}
+
static int blk_mq_debugfs_register_ctx(struct request_queue *q,
struct blk_mq_ctx *ctx,
struct dentry *hctx_dir)
{
struct dentry *ctx_dir;
char name[20];
- int i;
snprintf(name, sizeof(name), "cpu%u", ctx->cpu);
ctx_dir = debugfs_create_dir(name, hctx_dir);
if (!ctx_dir)
return -ENOMEM;
- for (i = 0; i < ARRAY_SIZE(blk_mq_debugfs_ctx_attrs); i++) {
- const struct blk_mq_debugfs_attr *attr;
-
- attr = &blk_mq_debugfs_ctx_attrs[i];
- if (!debugfs_create_file(attr->name, attr->mode, ctx_dir, ctx,
- attr->fops))
- return -ENOMEM;
- }
+ if (!debugfs_create_files(ctx_dir, ctx, blk_mq_debugfs_ctx_attrs))
+ return -ENOMEM;
return 0;
}
@@ -726,14 +732,8 @@ static int blk_mq_debugfs_register_hctx(struct request_queue *q,
if (!hctx_dir)
return -ENOMEM;
- for (i = 0; i < ARRAY_SIZE(blk_mq_debugfs_hctx_attrs); i++) {
- const struct blk_mq_debugfs_attr *attr;
-
- attr = &blk_mq_debugfs_hctx_attrs[i];
- if (!debugfs_create_file(attr->name, attr->mode, hctx_dir, hctx,
- attr->fops))
- return -ENOMEM;
- }
+ if (!debugfs_create_files(hctx_dir, hctx, blk_mq_debugfs_hctx_attrs))
+ return -ENOMEM;
hctx_for_each_ctx(hctx, ctx, i) {
if (blk_mq_debugfs_register_ctx(q, ctx, hctx_dir))
--
2.11.0
^ permalink raw reply related
* [PATCH 3/4] blk-mq-debug: Make show() operations interruptible
From: Bart Van Assche @ 2017-02-01 18:20 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval
In-Reply-To: <20170201182059.25601-1-bart.vanassche@sandisk.com>
Allow users to interrupt show operations instead of making a user
space process unkillable if ownership of q->sysfs_lock cannot be
obtained.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Omar Sandoval <osandov@fb.com>
---
block/blk-mq-debugfs.c | 34 ++++++++++++++++++++++++++--------
1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index aece9116b4f6..846943728939 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -179,13 +179,17 @@ static int hctx_tags_show(struct seq_file *m, void *v)
{
struct blk_mq_hw_ctx *hctx = m->private;
struct request_queue *q = hctx->queue;
+ int res;
- mutex_lock(&q->sysfs_lock);
+ res = mutex_lock_interruptible(&q->sysfs_lock);
+ if (res)
+ goto out;
if (hctx->tags)
blk_mq_debugfs_tags_show(m, hctx->tags);
mutex_unlock(&q->sysfs_lock);
- return 0;
+out:
+ return res;
}
static int hctx_tags_open(struct inode *inode, struct file *file)
@@ -204,12 +208,17 @@ static int hctx_tags_bitmap_show(struct seq_file *m, void *v)
{
struct blk_mq_hw_ctx *hctx = m->private;
struct request_queue *q = hctx->queue;
+ int res;
- mutex_lock(&q->sysfs_lock);
+ res = mutex_lock_interruptible(&q->sysfs_lock);
+ if (res)
+ goto out;
if (hctx->tags)
sbitmap_bitmap_show(&hctx->tags->bitmap_tags.sb, m);
mutex_unlock(&q->sysfs_lock);
- return 0;
+
+out:
+ return res;
}
static int hctx_tags_bitmap_open(struct inode *inode, struct file *file)
@@ -228,13 +237,17 @@ static int hctx_sched_tags_show(struct seq_file *m, void *v)
{
struct blk_mq_hw_ctx *hctx = m->private;
struct request_queue *q = hctx->queue;
+ int res;
- mutex_lock(&q->sysfs_lock);
+ res = mutex_lock_interruptible(&q->sysfs_lock);
+ if (res)
+ goto out;
if (hctx->sched_tags)
blk_mq_debugfs_tags_show(m, hctx->sched_tags);
mutex_unlock(&q->sysfs_lock);
- return 0;
+out:
+ return res;
}
static int hctx_sched_tags_open(struct inode *inode, struct file *file)
@@ -253,12 +266,17 @@ static int hctx_sched_tags_bitmap_show(struct seq_file *m, void *v)
{
struct blk_mq_hw_ctx *hctx = m->private;
struct request_queue *q = hctx->queue;
+ int res;
- mutex_lock(&q->sysfs_lock);
+ res = mutex_lock_interruptible(&q->sysfs_lock);
+ if (res)
+ goto out;
if (hctx->sched_tags)
sbitmap_bitmap_show(&hctx->sched_tags->bitmap_tags.sb, m);
mutex_unlock(&q->sysfs_lock);
- return 0;
+
+out:
+ return res;
}
static int hctx_sched_tags_bitmap_open(struct inode *inode, struct file *file)
--
2.11.0
^ permalink raw reply related
* [PATCH 2/4] blk-mq-debug: Avoid that sparse complains about req_flags_t usage
From: Bart Van Assche @ 2017-02-01 18:20 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval
In-Reply-To: <20170201182059.25601-1-bart.vanassche@sandisk.com>
Avoid that sparse reports the following complaints:
block/elevator.c:541:29: warning: incorrect type in assignment (different base types)
block/elevator.c:541:29: expected bool [unsigned] [usertype] next_sorted
block/elevator.c:541:29: got restricted req_flags_t
block/blk-mq-debugfs.c:92:54: warning: cast from restricted req_flags_t
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Omar Sandoval <osandov@fb.com>
---
block/blk-mq-debugfs.c | 3 ++-
block/elevator.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 7bcd4b6edf83..aece9116b4f6 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -89,7 +89,8 @@ static int blk_mq_debugfs_rq_show(struct seq_file *m, void *v)
struct request *rq = list_entry_rq(v);
seq_printf(m, "%p {.cmd_type=%u, .cmd_flags=0x%x, .rq_flags=0x%x, .tag=%d, .internal_tag=%d}\n",
- rq, rq->cmd_type, rq->cmd_flags, (unsigned int)rq->rq_flags,
+ rq, rq->cmd_type, rq->cmd_flags,
+ (__force unsigned int)rq->rq_flags,
rq->tag, rq->internal_tag);
return 0;
}
diff --git a/block/elevator.c b/block/elevator.c
index ef7f59469acc..9138efeee0c8 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -538,7 +538,7 @@ void elv_merge_requests(struct request_queue *q, struct request *rq,
if (e->uses_mq && e->type->ops.mq.requests_merged)
e->type->ops.mq.requests_merged(q, rq, next);
else if (e->type->ops.sq.elevator_merge_req_fn) {
- next_sorted = next->rq_flags & RQF_SORTED;
+ next_sorted = (__force bool)(next->rq_flags & RQF_SORTED);
if (next_sorted)
e->type->ops.sq.elevator_merge_req_fn(q, rq, next);
}
--
2.11.0
^ permalink raw reply related
* [PATCH 1/4] blk-mq-debugfs: Add missing __acquires() / __releases() annotations
From: Bart Van Assche @ 2017-02-01 18:20 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval
In-Reply-To: <20170201182059.25601-1-bart.vanassche@sandisk.com>
This patch avoids that sparse complains about lock imbalances.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Omar Sandoval <osandov@fb.com>
---
block/blk-mq-debugfs.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 5cd2b435a9f5..7bcd4b6edf83 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -95,6 +95,7 @@ static int blk_mq_debugfs_rq_show(struct seq_file *m, void *v)
}
static void *hctx_dispatch_start(struct seq_file *m, loff_t *pos)
+ __acquires(&hctx->lock)
{
struct blk_mq_hw_ctx *hctx = m->private;
@@ -110,6 +111,7 @@ static void *hctx_dispatch_next(struct seq_file *m, void *v, loff_t *pos)
}
static void hctx_dispatch_stop(struct seq_file *m, void *v)
+ __releases(&hctx->lock)
{
struct blk_mq_hw_ctx *hctx = m->private;
@@ -482,6 +484,7 @@ static const struct file_operations hctx_active_fops = {
};
static void *ctx_rq_list_start(struct seq_file *m, loff_t *pos)
+ __acquires(&ctx->lock)
{
struct blk_mq_ctx *ctx = m->private;
@@ -497,6 +500,7 @@ static void *ctx_rq_list_next(struct seq_file *m, void *v, loff_t *pos)
}
static void ctx_rq_list_stop(struct seq_file *m, void *v)
+ __releases(&ctx->lock)
{
struct blk_mq_ctx *ctx = m->private;
--
2.11.0
^ permalink raw reply related
* [PATCH 0/4] Four patches for the blk-mq debugfs code
From: Bart Van Assche @ 2017-02-01 18:20 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche
Hello Jens,
Please consider the four patches in this series for kernel v4.11.
Thanks,
Bart.
Bart Van Assche (4):
blk-mq-debugfs: Add missing __acquires() / __releases() annotations
blk-mq-debug: Avoid that sparse complains about req_flags_t usage
blk-mq-debug: Make show() operations interruptible
blk-mq-debug: Introduce debugfs_create_files()
block/blk-mq-debugfs.c | 75 +++++++++++++++++++++++++++++++++-----------------
block/elevator.c | 2 +-
2 files changed, 50 insertions(+), 27 deletions(-)
--
2.11.0
^ permalink raw reply
* Re: [PATCH] block: Update comments that refer to __bio_map_user() and bio_map_user()
From: Christoph Hellwig @ 2017-02-01 18:15 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Ming Lei, Jens Axboe
In-Reply-To: <20170201162008.4628-1-bart.vanassche@sandisk.com>
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH V2 3/3] nvme: allocate nvme_queue in correct node
From: Christoph Hellwig @ 2017-02-01 18:14 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-kernel, linux-block, bhelgaas, hch, axboe
In-Reply-To: <127b7a810d47b6b3530f29cb5ce36d7057a24e7e.1485971427.git.shli@fb.com>
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH V2 2/3] PCI: add an API to get node from vector
From: Christoph Hellwig @ 2017-02-01 18:11 UTC (permalink / raw)
To: Shaohua Li; +Cc: linux-kernel, linux-block, bhelgaas, hch, axboe
In-Reply-To: <d0f9b1bda8c27f058c95f88015e013b9d8131dc0.1485971427.git.shli@fb.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox