dm-thinp bug

dm-thinp bug

[Christoph Hellwig]

I can reproduce a bug in virtio triggered by running under dm-thinp by
just copying lots of files from my home directory into a freshly created
filesystem on dm-thinp.

The table setup I use for the thinly provisoned volume is:

0 73400320 thin-prov /dev/vdc1 /dev/vdc2 1048576 32

The virtio bug on says that it gets more segments than it allows to
higher layers.

[ 1944.721745] ------------[ cut here ]------------
[ 1944.724771] kernel BUG at /home/hch/work/linux-2.6/drivers/block/virtio_blk.c:178!
[ 1944.725511] invalid opcode: 0000 [#1] SMP 
[ 1944.725511] last sysfs file: /sys/devices/virtual/block/dm-0/dm/name
[ 1944.725511] Modules linked in:
[ 1944.725511] 
[ 1944.725511] Pid: 0, comm: swapper Not tainted 2.6.37+ #95 /Bochs
[ 1944.725511] EIP: 0060:[<c06d6808>] EFLAGS: 00010002 CPU: 0
[ 1944.725511] EIP is at do_virtblk_request+0x378/0x3a0
[ 1944.725511] EAX: 00000081 EBX: f5df7980 ECX: 00000000 EDX: 00000000
[ 1944.725511] ESI: f5dbf000 EDI: f5988670 EBP: f6409f6c ESP: f6409f28
[ 1944.725511]  DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
[ 1944.725511] Process swapper (pid: 0, ti=f6408000 task=c0d30ee0 task.ti=c0d1c000)
[ 1944.725511] Stack:
[ 1944.725511]  00000001 f5df7980 00000020 f5dbf4d8 f5df79ac f5dbf010 f5dbf020 00000004
[ 1944.725511]  f5c75bf0 0000003d f5dbf034 00000001 f77dda00 00000001 f5c75bf0 f5c75da8
[ 1944.725511]  f6409f94 f6409f7c c063b055 f5c75bf0 f5dbf000 f6409f88 c063b215 00000000
[ 1944.725511] Call Trace:
[ 1944.725511]  [<c063b055>] __blk_run_queue+0x65/0x120
[ 1944.725511]  [<c063b215>] blk_start_queue+0x35/0x70
[ 1944.725511]  [<c06d68fe>] blk_done+0xce/0xe0
[ 1944.725511]  [<c06a1594>] vring_interrupt+0x24/0x40
[ 1944.725511]  [<c01ad594>] handle_IRQ_event+0x44/0x150
[ 1944.725511]  [<c01ad594>] ? handle_IRQ_event+0x44/0x150
[ 1944.725511]  [<c01b0884>] ? move_native_irq+0x14/0x50
[ 1944.725511]  [<c01af42b>] handle_edge_irq+0x9b/0x130
[ 1944.725511]  [<c01af53c>] ? handle_fasteoi_irq+0x7c/0xc0
[ 1944.725511]  [<c01af390>] ? handle_edge_irq+0x0/0x130
[ 1944.725511]  <IRQ> 
[ 1944.725511]  [<c01377ed>] ? do_IRQ+0x3d/0xb0
[ 1944.725511]  [<c016f136>] ? irq_exit+0x56/0x70
[ 1944.725511]  [<c014c266>] ? smp_apic_timer_interrupt+0x56/0x90
[ 1944.725511]  [<c0136aa9>] ? common_interrupt+0x29/0x30
[ 1944.725511]  [<c0153525>] ? native_safe_halt+0x5/0x10
[ 1944.725511]  [<c013d493>] ? default_idle+0x73/0x170
[ 1944.725511]  [<c01353ea>] ? cpu_idle+0x4a/0x80
[ 1944.725511]  [<c09810d8>] ? rest_init+0x58/0x60
[ 1944.725511]  [<c0d858e3>] ? start_kernel+0x2e5/0x2eb
[ 1944.725511]  [<c0d85428>] ? unknown_bootoption+0x0/0x1a0
[ 1944.725511]  [<c0d850e0>] ? i386_start_kernel+0xe0/0xe8

Re: dm-thinp bug

[Joe Thornber]

On Tue, 2011-04-26 at 14:47 -0400, Christoph Hellwig wrote:
> I can reproduce a bug in virtio triggered by running under dm-thinp by
> just copying lots of files from my home directory into a freshly created
> filesystem on dm-thinp.

Finally someone has tried using it!  I'll look into this today.

- Joe

Re: dm-thinp bug

[Christoph Hellwig]

On Wed, Apr 27, 2011 at 09:27:37AM +0100, Joe Thornber wrote:
> On Tue, 2011-04-26 at 14:47 -0400, Christoph Hellwig wrote:
> > I can reproduce a bug in virtio triggered by running under dm-thinp by
> > just copying lots of files from my home directory into a freshly created
> > filesystem on dm-thinp.
> 
> Finally someone has tried using it!  I'll look into this today.

I actually used the earlier version you posted and didn't reproduce it
there.  I have a gut feeling it's related to barriers, as I probably
just did a basic ext3 test back then.

One thing that springs to mind is that both the thinp and multisnap
targets don't appear to support empty barriers (that is REQ_FLUSH
without a payload), because they don't set ti->num_flush_requests, but
I'm not sure that's related.

Re: dm-thinp bug

[Joe Thornber]

On Wed, 2011-04-27 at 04:39 -0400, Christoph Hellwig wrote:
> On Wed, Apr 27, 2011 at 09:27:37AM +0100, Joe Thornber wrote:
> > On Tue, 2011-04-26 at 14:47 -0400, Christoph Hellwig wrote:
> > > I can reproduce a bug in virtio triggered by running under dm-thinp by
> > > just copying lots of files from my home directory into a freshly created
> > > filesystem on dm-thinp.
> > 
> > Finally someone has tried using it!  I'll look into this today.
> 
> I actually used the earlier version you posted and didn't reproduce it
> there.  I have a gut feeling it's related to barriers, as I probably
> just did a basic ext3 test back then.

I hope this isn't the case.

> 
> One thing that springs to mind is that both the thinp and multisnap
> targets don't appear to support empty barriers (that is REQ_FLUSH
> without a payload), because they don't set ti->num_flush_requests, but
> I'm not sure that's related.

Fixed in thinp and multisnap public repos.

- Joe

Re: dm-thinp bug

[Joe Thornber]

On Tue, 2011-04-26 at 14:47 -0400, Christoph Hellwig wrote:
> The virtio bug on says that it gets more segments than it allows to
> higher layers. 

I think this is simply because I omitted the iterate_devices callback in
the thinp target.

I'll try and find time to test this patch later this week.
Alternatively you could just switch to multisnap which already has this
in.

- Joe



diff --git a/drivers/md/dm-thin-prov.c b/drivers/md/dm-thin-prov.c
index 4d382c8..054b4f9 100644
--- a/drivers/md/dm-thin-prov.c
+++ b/drivers/md/dm-thin-prov.c
@@ -643,6 +643,14 @@ thinp_io_hints(struct dm_target *ti, struct queue_limits *limits)
 	blk_limits_io_opt(limits, data_dev_block_size(tc));
 }
 
+static int thinp_iterate_devices(struct dm_target *ti,
+				 iterate_devices_callout_fn fn,
+				 void *data)
+{
+	struct thinp_c *tc = ti->private;
+	return fn(ti, tc->data_dev, 0, tc->data_size << tc->block_shift, data);
+}
+
 /* Thinp pool control target interface. */
 static struct target_type thinp_target = {
 	.name = "thin-prov",
@@ -658,6 +666,7 @@ static struct target_type thinp_target = {
 	.status = thinp_status,
 	.merge = thinp_bvec_merge,
 	.io_hints = thinp_io_hints,
+	.iterate_devices = thinp_iterate_devices,
 };
 
 static int __init dm_thinp_init(void)

Re: dm-thinp bug

[Christoph Hellwig]

On Wed, Apr 27, 2011 at 10:08:59AM +0100, Joe Thornber wrote:
> I think this is simply because I omitted the iterate_devices callback in
> the thinp target.
> 
> I'll try and find time to test this patch later this week.

This patch already creashes when creating the table:

qemu1:~# dmsetup create thin-volume table.txt 
[   15.186662] BUG: unable to handle kernel NULL pointer dereference at
00000010
[   15.189266] IP: [<c0863b1f>]
thinp_metadata_get_data_block_size+0xf/0x50
[   15.189266] *pde = 00000000 
[   15.189266] Oops: 0000 [#1] SMP 
[   15.189266] last sysfs file: /sys/devices/virtual/block/dm-0/removable
[   15.189266] Modules linked in:
[   15.189266] 
[   15.189266] Pid: 2027, comm: dmsetup Not tainted 2.6.37+ #99 /Bochs
[   15.189266] EIP: 0060:[<c0863b1f>] EFLAGS: 00010292 CPU: 0
[   15.189266] EIP is at thinp_metadata_get_data_block_size+0xf/0x50
[   15.189266] EAX: 00000000 EBX: 00000000 ECX: 00000200 EDX: f5f33da8
[   15.189266] ESI: f5a40140 EDI: c0862ba0 EBP: f5f33da0 ESP: f5f33d98
[   15.189266]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[   15.189266] Process dmsetup (pid: 2027, ti=f5f32000 task=f6616040 task.ti=f5f32000)
[   15.189266] Stack:
[   15.189266]  f5f33e04 f5a40140 f5f33db0 c0863940 09200000 00000000 f5f33dc0 c0863982
[   15.189266]  f8522040 00000000 f5f33e4c c084d864 f5f33de4 c015db89 00000000 00000000
[   15.189266]  00000000 f6802b40 f58cb9c0 f5f33e08 c017fed1 00000000 00000000 f5f33e64
[   15.189266] Call Trace:
[   15.189266]  [<c0863940>] data_dev_block_size+0x10/0x30
[   15.189266]  [<c0863982>] thinp_io_hints+0x22/0x40
[   15.189266]  [<c084d864>] dm_calculate_queue_limits+0xa4/0x140
[   15.189266]  [<c015db89>] ? complete+0x49/0x60
[   15.189266]  [<c017fed1>] ? flush_workqueue_prep_cwqs+0x71/0x1f0
[   15.189266]  [<c084badb>] dm_swap_table+0x4b/0x1c0
[   15.189266]  [<c018585c>] ? remove_wait_queue+0x3c/0x50
[   15.189266]  [<c084ffa8>] dev_suspend+0xe8/0x1f0
[   15.189266]  [<c06535de>] ? copy_to_user+0x2e/0x50
[   15.189266]  [<c0850bbb>] dm_ctl_ioctl+0x16b/0x210
[   15.189266]  [<c01edc93>] ? __pte_alloc+0x93/0xb0
[   15.189266]  [<c084fec0>] ? dev_suspend+0x0/0x1f0
[   15.189266]  [<c0850a50>] ? dm_ctl_ioctl+0x0/0x210
[   15.189266]  [<c02146c9>] do_vfs_ioctl+0x79/0x580
[   15.189266]  [<c064a9fd>] ? kobject_put+0x1d/0x50
[   15.189266]  [<c021821a>] ? dput+0x1a/0x230
[   15.189266]  [<c021e220>] ? mntput_no_expire+0x20/0xf0
[   15.189266]  [<c021e303>] ? mntput+0x13/0x20
[   15.189266]  [<c0207b4d>] ? fput+0x12d/0x1d0
[   15.189266]  [<c0608470>] ? sys_ipc+0x80/0x220
[   15.189266]  [<c0214c09>] sys_ioctl+0x39/0x70
[   15.189266]  [<c099dac9>] syscall_call+0x7/0xb

Re: dm-thinp bug

[Joe Thornber]

On Wed, 2011-04-27 at 05:32 -0400, Christoph Hellwig wrote:
> 
> This patch already creashes when creating the table: 

Sigh, it's revealed a bug in io_hints, which isn't ever run if there's
no iterate_devices callback.

The patch below has been regression tested, but not tried above a virtio
device.  It's been pushed too.

- Joe



diff --git a/drivers/md/dm-thin-prov.c b/drivers/md/dm-thin-prov.c
index 6ac802e..a168ee6 100644
--- a/drivers/md/dm-thin-prov.c
+++ b/drivers/md/dm-thin-prov.c
@@ -641,7 +641,15 @@ thinp_io_hints(struct dm_target *ti, struct
queue_limits *limits)
        struct thinp_c *tc = ti->private;
 
        blk_limits_io_min(limits, 0);
-       blk_limits_io_opt(limits, data_dev_block_size(tc));
+       blk_limits_io_opt(limits, tc->block_size << tc->block_shift);
+}
+
+static int thinp_iterate_devices(struct dm_target *ti,
+                                iterate_devices_callout_fn fn,
+                                void *data)
+{
+       struct thinp_c *tc = ti->private;
+       return fn(ti, tc->data_dev, 0, tc->data_size << tc->block_shift,
data);
 }
 
 /* Thinp pool control target interface. */
@@ -659,6 +667,7 @@ static struct target_type thinp_target = {
        .status = thinp_status,
        .merge = thinp_bvec_merge,
        .io_hints = thinp_io_hints,
+       .iterate_devices = thinp_iterate_devices,
 };
 
 static int __init dm_thinp_init(void)