[PATCH] block/025: test discard sector alignement and sector size overflow

[PATCH] block/025: test discard sector alignement and sector size overflow

[Ming Lei]

This test covers the following two issues:

1) discard sector need to be aligned with logical block size

2) make sure 'sector_t' instead of 'unsigned int' is used when comparing
with discard sector size

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 tests/block/025     | 37 +++++++++++++++++++++++++++++++++++++
 tests/block/025.out |  2 ++
 2 files changed, 39 insertions(+)
 create mode 100755 tests/block/025
 create mode 100644 tests/block/025.out

diff --git a/tests/block/025 b/tests/block/025
new file mode 100755
index 000000000000..32b632431793
--- /dev/null
+++ b/tests/block/025
@@ -0,0 +1,37 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2018 Ming Lei <ming.lei@redhat.com>
+#
+# Check two corener cases of BLKDISCARD.
+#
+# 1) test if discard bio's sector is algined with logical size, fixed by
+#    1adfc5e4136f ("block: make sure discard bio is aligned with logical block size")
+# 2) test 32 bit overflow when comparing discard sector size. Fixed by
+#    4800bf7bc8c72 ("block: fix 32 bit overflow in __blkdev_issue_discard()")
+
+. tests/block/rc
+. common/scsi_debug
+
+DESCRIPTION="check sector alignment and sector size overflow of BLKDISCARD"
+
+requires() {
+	_have_scsi_debug
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	rm -f "$FULL"
+
+	# Create virtual device with unmap_zeroes_data support
+	if ! _init_scsi_debug virtual_gb=2049 sector_size=4096 lbpws10=1 dev_size_mb=512; then
+		return 1
+	fi
+
+	local dev="/dev/${SCSI_DEBUG_DEVICES[0]}"
+	blkdiscard "$dev"
+
+	_exit_scsi_debug
+
+	echo "Test complete"
+}
diff --git a/tests/block/025.out b/tests/block/025.out
new file mode 100644
index 000000000000..fd9a6d5f70de
--- /dev/null
+++ b/tests/block/025.out
@@ -0,0 +1,2 @@
+Running block/025
+Test complete
-- 
2.9.5

Re: [PATCH] block/025: test discard sector alignement and sector size overflow

[Ming Lei]

On Thu, Nov 15, 2018 at 12:01 PM Ming Lei <ming.lei@redhat.com> wrote:
>
> This test covers the following two issues:
>
> 1) discard sector need to be aligned with logical block size
>
> 2) make sure 'sector_t' instead of 'unsigned int' is used when comparing
> with discard sector size
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  tests/block/025     | 37 +++++++++++++++++++++++++++++++++++++
>  tests/block/025.out |  2 ++
>  2 files changed, 39 insertions(+)
>  create mode 100755 tests/block/025
>  create mode 100644 tests/block/025.out
>
> diff --git a/tests/block/025 b/tests/block/025
> new file mode 100755
> index 000000000000..32b632431793
> --- /dev/null
> +++ b/tests/block/025
> @@ -0,0 +1,37 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2018 Ming Lei <ming.lei@redhat.com>
> +#
> +# Check two corener cases of BLKDISCARD.
> +#
> +# 1) test if discard bio's sector is algined with logical size, fixed by
> +#    1adfc5e4136f ("block: make sure discard bio is aligned with logical block size")
> +# 2) test 32 bit overflow when comparing discard sector size. Fixed by
> +#    4800bf7bc8c72 ("block: fix 32 bit overflow in __blkdev_issue_discard()")
> +
> +. tests/block/rc
> +. common/scsi_debug
> +
> +DESCRIPTION="check sector alignment and sector size overflow of BLKDISCARD"
> +
> +requires() {
> +       _have_scsi_debug
> +}
> +
> +test() {
> +       echo "Running ${TEST_NAME}"
> +
> +       rm -f "$FULL"
> +
> +       # Create virtual device with unmap_zeroes_data support
> +       if ! _init_scsi_debug virtual_gb=2049 sector_size=4096 lbpws10=1 dev_size_mb=512; then
> +               return 1
> +       fi
> +
> +       local dev="/dev/${SCSI_DEBUG_DEVICES[0]}"
> +       blkdiscard "$dev"
> +
> +       _exit_scsi_debug
> +
> +       echo "Test complete"
> +}
> diff --git a/tests/block/025.out b/tests/block/025.out
> new file mode 100644
> index 000000000000..fd9a6d5f70de
> --- /dev/null
> +++ b/tests/block/025.out
> @@ -0,0 +1,2 @@
> +Running block/025
> +Test complete
> --
> 2.9.5
>

Gentle ping...

thanks,
Ming Lei

Re: [PATCH] block/025: test discard sector alignement and sector size overflow

[Omar Sandoval]

On Thu, Nov 15, 2018 at 12:00:17PM +0800, Ming Lei wrote:
> This test covers the following two issues:
> 
> 1) discard sector need to be aligned with logical block size
> 
> 2) make sure 'sector_t' instead of 'unsigned int' is used when comparing
> with discard sector size
> 
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  tests/block/025     | 37 +++++++++++++++++++++++++++++++++++++
>  tests/block/025.out |  2 ++
>  2 files changed, 39 insertions(+)
>  create mode 100755 tests/block/025
>  create mode 100644 tests/block/025.out
> 
> diff --git a/tests/block/025 b/tests/block/025
> new file mode 100755
> index 000000000000..32b632431793
> --- /dev/null
> +++ b/tests/block/025
> @@ -0,0 +1,37 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2018 Ming Lei <ming.lei@redhat.com>
> +#
> +# Check two corener cases of BLKDISCARD.
> +#
> +# 1) test if discard bio's sector is algined with logical size, fixed by
> +#    1adfc5e4136f ("block: make sure discard bio is aligned with logical block size")

Hm, I'm not seeing how this test case tests this commit. Aren't 2049G
and 512M both aligned to 4096 bytes?

> +# 2) test 32 bit overflow when comparing discard sector size. Fixed by
> +#    4800bf7bc8c72 ("block: fix 32 bit overflow in __blkdev_issue_discard()")
> +
> +. tests/block/rc
> +. common/scsi_debug
> +
> +DESCRIPTION="check sector alignment and sector size overflow of BLKDISCARD"
> +
> +requires() {
> +	_have_scsi_debug
> +}
> +
> +test() {
> +	echo "Running ${TEST_NAME}"
> +
> +	rm -f "$FULL"
> +
> +	# Create virtual device with unmap_zeroes_data support
> +	if ! _init_scsi_debug virtual_gb=2049 sector_size=4096 lbpws10=1 dev_size_mb=512; then
> +		return 1
> +	fi
> +
> +	local dev="/dev/${SCSI_DEBUG_DEVICES[0]}"
> +	blkdiscard "$dev"
> +
> +	_exit_scsi_debug
> +
> +	echo "Test complete"
> +}
> diff --git a/tests/block/025.out b/tests/block/025.out
> new file mode 100644
> index 000000000000..fd9a6d5f70de
> --- /dev/null
> +++ b/tests/block/025.out
> @@ -0,0 +1,2 @@
> +Running block/025
> +Test complete
> -- 
> 2.9.5
> 

Re: [PATCH] block/025: test discard sector alignement and sector size overflow

[Ming Lei]

On Mon, Nov 26, 2018 at 04:33:10PM -0800, Omar Sandoval wrote:
> On Thu, Nov 15, 2018 at 12:00:17PM +0800, Ming Lei wrote:
> > This test covers the following two issues:
> > 
> > 1) discard sector need to be aligned with logical block size
> > 
> > 2) make sure 'sector_t' instead of 'unsigned int' is used when comparing
> > with discard sector size
> > 
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >  tests/block/025     | 37 +++++++++++++++++++++++++++++++++++++
> >  tests/block/025.out |  2 ++
> >  2 files changed, 39 insertions(+)
> >  create mode 100755 tests/block/025
> >  create mode 100644 tests/block/025.out
> > 
> > diff --git a/tests/block/025 b/tests/block/025
> > new file mode 100755
> > index 000000000000..32b632431793
> > --- /dev/null
> > +++ b/tests/block/025
> > @@ -0,0 +1,37 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-3.0+
> > +# Copyright (C) 2018 Ming Lei <ming.lei@redhat.com>
> > +#
> > +# Check two corener cases of BLKDISCARD.
> > +#
> > +# 1) test if discard bio's sector is algined with logical size, fixed by
> > +#    1adfc5e4136f ("block: make sure discard bio is aligned with logical block size")
> 
> Hm, I'm not seeing how this test case tests this commit. Aren't 2049G
> and 512M both aligned to 4096 bytes?

2049G caused 32bit 'nr_sects' in __blkdev_issue_discard() overflow,
please see commit 4800bf7bc8c725e955fcb ("block: fix 32 bit overflow in __blkdev_issue_discard()").

4096 logical block size may trigger 'req_sects' un-alignment issue
because 'nr_sects' from fs is 512byte aligned, please see 1adfc5e4136f5967d59 ("block:
make sure discard bio is aligned with logical block size").

Thanks,
Ming

Re: [PATCH] block/025: test discard sector alignement and sector size overflow

[Omar Sandoval]

On Thu, Nov 15, 2018 at 12:00:17PM +0800, Ming Lei wrote:
> This test covers the following two issues:
> 
> 1) discard sector need to be aligned with logical block size
> 
> 2) make sure 'sector_t' instead of 'unsigned int' is used when comparing
> with discard sector size
> 
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  tests/block/025     | 37 +++++++++++++++++++++++++++++++++++++
>  tests/block/025.out |  2 ++
>  2 files changed, 39 insertions(+)
>  create mode 100755 tests/block/025
>  create mode 100644 tests/block/025.out

Applied, thanks Ming.