[dm-devel] [PATCH] dm table: fix logic bug in dm_table_supports_write_same [was: Re: BLKZEROOUT on dm devices]

[Mike Snitzer]

On Tue, Apr 30 2013 at 12:07am -0400,
Bharata B Rao <bharata.rao at gmail.com> wrote:

> Hi,
> 
> Is BLKZEROOUT ioctl supposed to work on dm devices ? When I try this
> ioctl on an LV that sits on a WRITE SAME capable SCSI device, I see
> that conventional zeroing is done since the WRITE SAME support isn't
> advertised by the dm device.
> 
> # lsblk
> NAME                             MAJ:MIN RM   SIZE RO TYPE  MOUNTPOINT
> sda                                8:0    0    50G  0 disk
> └─mpathb (dm-12)                 252:12   0    50G  0 mpath
>   └─ws_vg-lvol0 (dm-11)          252:11   0     1G  0 lvm
> sde                                8:64   0    50G  0 disk
> └─mpathb (dm-12)                 252:12   0    50G  0 mpath
>   └─ws_vg-lvol0 (dm-11)          252:11   0     1G  0 lvm
> 
> # cat /sys/block/sda/queue/write_same_max_bytes
> 268435456
> # cat /sys/block/dm-12/queue/write_same_max_bytes
> 0
> # cat /sys/block/dm-11/queue/write_same_max_bytes
> 0
> 
> [root at llmvm02 ~]# cat /sys/dev/block/8\:0/queue/write_same_max_bytes
> 268435456
> [root at llmvm02 ~]# cat /sys/dev/block/252\:12/queue/write_same_max_bytes
> 0
> [root at llmvm02 ~]# cat /sys/dev/block/252\:11/queue/write_same_max_bytes
> 0

Sure enough I see the same limits stacking problem you reported:

# modprobe scsi_debug dev_size_mb=100 lbpws=1
...
sd 5:0:0:0: [sdh] Attached SCSI disk

# cat /sys/block/sdh/queue/write_same_max_bytes
33553920
(which reflects scsi_debug's write_same_length default of 0xffff * 512b)

# echo "0 `blockdev --getsz /dev/sdh` linear /dev/sdh 0" | dmsetup create test
# readlink /dev/mapper/test
../dm-4
# cat /sys/block/dm-4/queue/write_same_max_bytes 
0

> From commit d54eaa5a0fde0a202e4e91f200f818edcef15bee, I gather that
> WRITE SAME is advertised by a dm device if all of its target and
> underlying devices support WRITE SAME. Also as per
> 4f0b70b0479101522b8645ddc1f5ee7137821db3, WRITE SAME is supported for
> linear dm devices. Both of these seem to be true in my case, but still
> BLKZEROOUT ioctl resorts to conventional zeroing. What am I missing
> here ?

The first patch of my WRITE SAME patchset that I posted to dm-devel was
tweaked before going upstream:
http://www.redhat.com/archives/dm-devel/2012-December/msg00064.html

Due to iterate_devices disposition to short-circuit if the test func()
returns true, it looks like Alasdair correctly changed 
s/device_write_same_capable/device_not_write_same_capable/ but forgot to
invert the logic of the calling iterate_devices in
dm_table_supports_write_same().

Anyway, this patch fixes things for me:

From: Mike Snitzer <snitzer at redhat.com>
Date: Tue, 30 Apr 2013 12:24:58 -0400
Subject: [PATCH] dm table: fix logic bug in dm_table_supports_write_same

If device_not_write_same_capable() returns true then the iterate_devices
loop in dm_table_supports_write_same() should return false.

Reported-by: Bharata B Rao <bharata.rao at gmail.com>
Signed-off-by: Mike Snitzer <snitzer at redhat.com>
Cc: stable at vger.kernel.org # v3.8+
---
 drivers/md/dm-table.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 3fe797f..d801682 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -1445,7 +1445,7 @@ static bool dm_table_supports_write_same(struct dm_table *t)
 			return false;
 
 		if (!ti->type->iterate_devices ||
-		    !ti->type->iterate_devices(ti, device_not_write_same_capable, NULL))
+		    ti->type->iterate_devices(ti, device_not_write_same_capable, NULL))
 			return false;
 	}
 
-- 
1.7.1