| Message ID | 1439918511-2859-1-git-send-email-zlang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 19, 2015 at 01:21:51AM +0800, Zorro Lang wrote: > The below command in "Test 4": > > xfs_io -c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" > > will run failed on 4k sector drives. So I use min_alignment size > to replace the hard-code 512. > > Also I make sure the blksize won't less than min_alignment size, > after blksize=`expr $pgsize / 8`. > > If blksize really less than min_alignment size, I set blksize = > min_alignment size, and for sure the consistency of test result, > I repair pgsize(already not real page size) number according to > the new blksize. > > Because IRIX can't use _min_dio_alignment(), so remove it from > supported os list. Not true - that's what the 'feature -s' branch in _min_dio_alignment() is supposed to be for. Just add another check for "$HOSTOS" == "Linux".... > At last, make the crc flag be disabled only when blksize=512. That makes things unnecessarily complex. I'd prefer to leave it as it is (i.e. with -m crc=0), as CRCs do not affect what is being tested at all. > Signed-off-by: Zorro Lang <zlang@redhat.com> > --- > tests/xfs/194 | 23 +++++++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) > > diff --git a/tests/xfs/194 b/tests/xfs/194 > index e11b459..4491dd4 100755 > --- a/tests/xfs/194 > +++ b/tests/xfs/194 > @@ -42,7 +42,7 @@ _cleanup() > > # only xfs supported due to use of xfs_bmap > _supported_fs xfs > -_supported_os IRIX Linux > +_supported_os Linux > > # real QA test starts here > rm -f $seqres.full > @@ -50,6 +50,16 @@ rm -f $seqres.full > # For this test we use block size = 1/8 page size > pgsize=`$here/src/feature -s` > blksize=`expr $pgsize / 8` > +secsize=`_min_dio_alignment $SCRATCH_DEV` > + > +# The minimal blksize can't less than sector size, So if > +# blksize < secsize, we should adjust blksize and pgsize number. > +# Of course, if we adjust pgsize, pgsize won't equal to the > +# real page size of system. > +if [ $blksize -lt $secsize ];then > + blksize=$secsize > + pgsize=`expr $blksize \* 8` > +fi No, this is wrong. the page size stays fixed at the machine page size. We are testing *sub-page block sizes* here and the sector size must be <= page size. Increasing the "page size" to larger than the machine page size does not make the kernel use larger page sizes. IOWs, if you've got sector size = page size (e.g. 4k sector device) then no matter what you say $pgsize is, the kernel will see a block size = page size test. This whole chunk of code can simply be replaced with: blksize=`_min_dio_alignment $SCRATCH_DEV` Because that's what we actually need to test... Cheers, Dave.
On 8/18/15 5:28 PM, Dave Chinner wrote: > On Wed, Aug 19, 2015 at 01:21:51AM +0800, Zorro Lang wrote: >> The below command in "Test 4": >> >> xfs_io -c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" >> >> will run failed on 4k sector drives. So I use min_alignment size >> to replace the hard-code 512. >> >> Also I make sure the blksize won't less than min_alignment size, >> after blksize=`expr $pgsize / 8`. >> >> If blksize really less than min_alignment size, I set blksize = >> min_alignment size, and for sure the consistency of test result, >> I repair pgsize(already not real page size) number according to >> the new blksize. >> >> Because IRIX can't use _min_dio_alignment(), so remove it from >> supported os list. > > Not true - that's what the 'feature -s' branch in > _min_dio_alignment() is supposed to be for. Just add another check > for "$HOSTOS" == "Linux".... > > >> At last, make the crc flag be disabled only when blksize=512. > > That makes things unnecessarily complex. I'd prefer to leave it as > it is (i.e. with -m crc=0), as CRCs do not affect what is being > tested at all. > >> Signed-off-by: Zorro Lang <zlang@redhat.com> >> --- >> tests/xfs/194 | 23 +++++++++++++++++++---- >> 1 file changed, 19 insertions(+), 4 deletions(-) >> >> diff --git a/tests/xfs/194 b/tests/xfs/194 >> index e11b459..4491dd4 100755 >> --- a/tests/xfs/194 >> +++ b/tests/xfs/194 >> @@ -42,7 +42,7 @@ _cleanup() >> >> # only xfs supported due to use of xfs_bmap >> _supported_fs xfs >> -_supported_os IRIX Linux >> +_supported_os Linux >> >> # real QA test starts here >> rm -f $seqres.full >> @@ -50,6 +50,16 @@ rm -f $seqres.full >> # For this test we use block size = 1/8 page size >> pgsize=`$here/src/feature -s` >> blksize=`expr $pgsize / 8` >> +secsize=`_min_dio_alignment $SCRATCH_DEV` >> + >> +# The minimal blksize can't less than sector size, So if >> +# blksize < secsize, we should adjust blksize and pgsize number. >> +# Of course, if we adjust pgsize, pgsize won't equal to the >> +# real page size of system. >> +if [ $blksize -lt $secsize ];then >> + blksize=$secsize >> + pgsize=`expr $blksize \* 8` >> +fi > > No, this is wrong. the page size stays fixed at the machine page > size. We are testing *sub-page block sizes* here and the sector size > must be <= page size. Increasing the "page size" to larger than the > machine page size does not make the kernel use larger page sizes. > > IOWs, if you've got sector size = page size (e.g. 4k sector device) > then no matter what you say $pgsize is, the kernel will see a block > size = page size test. > > This whole chunk of code can simply be replaced with: > > blksize=`_min_dio_alignment $SCRATCH_DEV` > > Because that's what we actually need to test... That won't work either, because we could easily get 512 from that. and then this test: # Now try the same thing but write a sector in the middle of that hole # If things go badly stale data will be exposed either side. # This is most interesting for block size > 512 (page size > 4096) # We *should* get: # |1100|HHHH|33HH|HHHH|2222|----|----|----| echo "== Test 4 ==" xfs_io \ -c "pwrite -S 0x11 -b $pgsize 0 $pgsize" \ -c "mmap -r 0 $blksize" -c "mread 0 $blksize" -c "munmap" \ -c "truncate `expr $blksize / 2`" \ -c "truncate `expr $blksize + 1`" \ -c "pwrite -S 0x22 -b $blksize `expr $pgsize / 2` $blksize" \ -c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" \ -t -d -f $SCRATCH_MNT/testfile4 >> $seqres.full will be impossible. AFAICT everything works except for that explicit 512-byte IO. All we really need here is a sub-block-size IO, but at least as large as the logical sector size. So we want sub-page-size blocks, an sub-block-sized IOS (here). Can't be *that* tricky to work out the scaling for various pages and sector sizes, I hope? :) Famous last words! -Eric -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 18, 2015 at 05:33:05PM -0500, Eric Sandeen wrote: > On 8/18/15 5:28 PM, Dave Chinner wrote: > > On Wed, Aug 19, 2015 at 01:21:51AM +0800, Zorro Lang wrote: > >> @@ -50,6 +50,16 @@ rm -f $seqres.full > >> # For this test we use block size = 1/8 page size > >> pgsize=`$here/src/feature -s` > >> blksize=`expr $pgsize / 8` > >> +secsize=`_min_dio_alignment $SCRATCH_DEV` > >> + > >> +# The minimal blksize can't less than sector size, So if > >> +# blksize < secsize, we should adjust blksize and pgsize number. > >> +# Of course, if we adjust pgsize, pgsize won't equal to the > >> +# real page size of system. > >> +if [ $blksize -lt $secsize ];then > >> + blksize=$secsize > >> + pgsize=`expr $blksize \* 8` > >> +fi > > > > No, this is wrong. the page size stays fixed at the machine page > > size. We are testing *sub-page block sizes* here and the sector size > > must be <= page size. Increasing the "page size" to larger than the > > machine page size does not make the kernel use larger page sizes. > > > > IOWs, if you've got sector size = page size (e.g. 4k sector device) > > then no matter what you say $pgsize is, the kernel will see a block > > size = page size test. > > > > This whole chunk of code can simply be replaced with: > > > > blksize=`_min_dio_alignment $SCRATCH_DEV` > > > > Because that's what we actually need to test... > > That won't work either, because we could easily get 512 from that. If 'blockdev --getss $dev' returns 512, then the device supports 512 byte IOs and so it is fine to do 512 byte IOs in the test. > and then this test: > > # Now try the same thing but write a sector in the middle of that hole > # If things go badly stale data will be exposed either side. > # This is most interesting for block size > 512 (page size > 4096) > > # We *should* get: > # |1100|HHHH|33HH|HHHH|2222|----|----|----| > > echo "== Test 4 ==" > xfs_io \ > -c "pwrite -S 0x11 -b $pgsize 0 $pgsize" \ > -c "mmap -r 0 $blksize" -c "mread 0 $blksize" -c "munmap" \ > -c "truncate `expr $blksize / 2`" \ > -c "truncate `expr $blksize + 1`" \ > -c "pwrite -S 0x22 -b $blksize `expr $pgsize / 2` $blksize" \ > -c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" \ > -t -d -f $SCRATCH_MNT/testfile4 >> $seqres.full > > will be impossible. > > AFAICT everything works except for that explicit 512-byte IO. Right. That hard coded 512 needs to change to $blksize, because blksize is now equal to the sector size. I thought this would be obvious to the reader, so I didn't comment on it. > All we really need here is a sub-block-size IO, but at least as > large as the logical sector size. Yes. See above. > So we want sub-page-size blocks, an sub-block-sized IOS (here). Yes. See above. > Can't be *that* tricky to work out the scaling for various pages > and sector sizes, I hope? :) Famous last words! It's not. See above. :) Cheers, Dave.
On 8/18/15 5:43 PM, Dave Chinner wrote: > On Tue, Aug 18, 2015 at 05:33:05PM -0500, Eric Sandeen wrote: >> On 8/18/15 5:28 PM, Dave Chinner wrote: >>> On Wed, Aug 19, 2015 at 01:21:51AM +0800, Zorro Lang wrote: >>>> @@ -50,6 +50,16 @@ rm -f $seqres.full >>>> # For this test we use block size = 1/8 page size >>>> pgsize=`$here/src/feature -s` >>>> blksize=`expr $pgsize / 8` >>>> +secsize=`_min_dio_alignment $SCRATCH_DEV` >>>> + >>>> +# The minimal blksize can't less than sector size, So if >>>> +# blksize < secsize, we should adjust blksize and pgsize number. >>>> +# Of course, if we adjust pgsize, pgsize won't equal to the >>>> +# real page size of system. >>>> +if [ $blksize -lt $secsize ];then >>>> + blksize=$secsize >>>> + pgsize=`expr $blksize \* 8` >>>> +fi >>> >>> No, this is wrong. the page size stays fixed at the machine page >>> size. We are testing *sub-page block sizes* here and the sector size >>> must be <= page size. Increasing the "page size" to larger than the >>> machine page size does not make the kernel use larger page sizes. >>> >>> IOWs, if you've got sector size = page size (e.g. 4k sector device) >>> then no matter what you say $pgsize is, the kernel will see a block >>> size = page size test. >>> >>> This whole chunk of code can simply be replaced with: >>> >>> blksize=`_min_dio_alignment $SCRATCH_DEV` >>> >>> Because that's what we actually need to test... >> >> That won't work either, because we could easily get 512 from that. > > If 'blockdev --getss $dev' returns 512, then the device supports 512 > byte IOs and so it is fine to do 512 byte IOs in the test. > >> and then this test: >> >> # Now try the same thing but write a sector in the middle of that hole >> # If things go badly stale data will be exposed either side. >> # This is most interesting for block size > 512 (page size > 4096) >> >> # We *should* get: >> # |1100|HHHH|33HH|HHHH|2222|----|----|----| >> >> echo "== Test 4 ==" >> xfs_io \ >> -c "pwrite -S 0x11 -b $pgsize 0 $pgsize" \ >> -c "mmap -r 0 $blksize" -c "mread 0 $blksize" -c "munmap" \ >> -c "truncate `expr $blksize / 2`" \ >> -c "truncate `expr $blksize + 1`" \ >> -c "pwrite -S 0x22 -b $blksize `expr $pgsize / 2` $blksize" \ >> -c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" \ >> -t -d -f $SCRATCH_MNT/testfile4 >> $seqres.full >> >> will be impossible. >> >> AFAICT everything works except for that explicit 512-byte IO. > > Right. That hard coded 512 needs to change to $blksize, because > blksize is now equal to the sector size. I thought this would be > obvious to the reader, so I didn't comment on it. if that last IO is $blksize, and blocksize == sector size, then the test won't be testing what it's designed to test here, i.e. a sub-block direct IO write. # We *should* get: # |1100|HHHH|33HH|HHHH|2222|----|----|----| ^^ this -Eric -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
----- ???? ----- > ???: "Eric Sandeen" <sandeen@sandeen.net> > ???: "Dave Chinner" <david@fromorbit.com>, "Eric Sandeen" <sandeen@redhat.com> > ??: "Zorro Lang" <zlang@redhat.com>, fstests@vger.kernel.org > ????: ???, 2015? 8 ? 19? ?? 7:03:45 > ??: Re: [PATCH] xfs/194: fix the exception when run on 4k sector drives > > On 8/18/15 5:43 PM, Dave Chinner wrote: > > On Tue, Aug 18, 2015 at 05:33:05PM -0500, Eric Sandeen wrote: > >> On 8/18/15 5:28 PM, Dave Chinner wrote: > >>> On Wed, Aug 19, 2015 at 01:21:51AM +0800, Zorro Lang wrote: > >>>> @@ -50,6 +50,16 @@ rm -f $seqres.full > >>>> # For this test we use block size = 1/8 page size > >>>> pgsize=`$here/src/feature -s` > >>>> blksize=`expr $pgsize / 8` > >>>> +secsize=`_min_dio_alignment $SCRATCH_DEV` > >>>> + > >>>> +# The minimal blksize can't less than sector size, So if > >>>> +# blksize < secsize, we should adjust blksize and pgsize number. > >>>> +# Of course, if we adjust pgsize, pgsize won't equal to the > >>>> +# real page size of system. > >>>> +if [ $blksize -lt $secsize ];then > >>>> + blksize=$secsize > >>>> + pgsize=`expr $blksize \* 8` > >>>> +fi > >>> > >>> No, this is wrong. the page size stays fixed at the machine page > >>> size. We are testing *sub-page block sizes* here and the sector size > >>> must be <= page size. Increasing the "page size" to larger than the > >>> machine page size does not make the kernel use larger page sizes. > >>> > >>> IOWs, if you've got sector size = page size (e.g. 4k sector device) > >>> then no matter what you say $pgsize is, the kernel will see a block > >>> size = page size test. Yes, I know I can't change page size. So I said "for sure the consistency of test result, I repair pgsize(already not real page size) number according to the new blksize." in git commit message. Eric said in xfs/194 "Test mapping around/over holes for sub-page blocks", If the sector size = page size, I have no idea how to make sub-page blocks. So in this situation, I set pgsize=`expr $blksize \* 8` just for the test output suit for 194.out. Or we don't care pgsize, just set blksize=`_min_dio_alignment $SCRATCH_DEV`, and then replace all $pgsize to `expr $blksize \* 8`. If page size > blksize, that's good. If not, we can't do more things about that, so just make it run pass. > >>> > >>> This whole chunk of code can simply be replaced with: > >>> > >>> blksize=`_min_dio_alignment $SCRATCH_DEV` > >>> > >>> Because that's what we actually need to test... > >> > >> That won't work either, because we could easily get 512 from that. > > > > If 'blockdev --getss $dev' returns 512, then the device supports 512 > > byte IOs and so it is fine to do 512 byte IOs in the test. > > > >> and then this test: > >> > >> # Now try the same thing but write a sector in the middle of that hole > >> # If things go badly stale data will be exposed either side. > >> # This is most interesting for block size > 512 (page size > 4096) > >> > >> # We *should* get: > >> # |1100|HHHH|33HH|HHHH|2222|----|----|----| > >> > >> echo "== Test 4 ==" > >> xfs_io \ > >> -c "pwrite -S 0x11 -b $pgsize 0 $pgsize" \ > >> -c "mmap -r 0 $blksize" -c "mread 0 $blksize" -c "munmap" \ > >> -c "truncate `expr $blksize / 2`" \ > >> -c "truncate `expr $blksize + 1`" \ > >> -c "pwrite -S 0x22 -b $blksize `expr $pgsize / 2` $blksize" \ > >> -c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" \ > >> -t -d -f $SCRATCH_MNT/testfile4 >> $seqres.full > >> > >> will be impossible. > >> > >> AFAICT everything works except for that explicit 512-byte IO. > > > > Right. That hard coded 512 needs to change to $blksize, because > > blksize is now equal to the sector size. I thought this would be > > obvious to the reader, so I didn't comment on it. > > if that last IO is $blksize, and blocksize == sector size, then the > test won't be testing what it's designed to test here, i.e. a > sub-block direct IO write. What do you think about loop device? If page size(4k) = sector size(4k), how about use loop device? I think it will be 512 sector size as default? Thanks, Zorro > > # We *should* get: > # |1100|HHHH|33HH|HHHH|2222|----|----|----| > ^^ > this > > -Eric > > -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
----- ???? ----- > ???: "Dave Chinner" <david@fromorbit.com> > ???: "Zorro Lang" <zlang@redhat.com> > ??: fstests@vger.kernel.org, sandeen@redhat.com > ????: ???, 2015? 8 ? 19? ?? 6:28:32 > ??: Re: [PATCH] xfs/194: fix the exception when run on 4k sector drives > > On Wed, Aug 19, 2015 at 01:21:51AM +0800, Zorro Lang wrote: > > The below command in "Test 4": > > > > xfs_io -c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" > > > > will run failed on 4k sector drives. So I use min_alignment size > > to replace the hard-code 512. > > > > Also I make sure the blksize won't less than min_alignment size, > > after blksize=`expr $pgsize / 8`. > > > > If blksize really less than min_alignment size, I set blksize = > > min_alignment size, and for sure the consistency of test result, > > I repair pgsize(already not real page size) number according to > > the new blksize. > > > > Because IRIX can't use _min_dio_alignment(), so remove it from > > supported os list. > > Not true - that's what the 'feature -s' branch in > _min_dio_alignment() is supposed to be for. Just add another check > for "$HOSTOS" == "Linux".... Do you mean change _min_dio_alignment() to: _min_dio_alignment() { dev=$1 if [ -b "$dev" -a $HOSTOS" == "Linux" ]; then blockdev --getss $dev else $here/src/feature -s fi } I really don't understand why page size will be the minimum dio alignment? Do you mean in other OS(except linux), the sector size = page size? Thanks, Zorro > > > > At last, make the crc flag be disabled only when blksize=512. > > That makes things unnecessarily complex. I'd prefer to leave it as > it is (i.e. with -m crc=0), as CRCs do not affect what is being > tested at all. Yes, you're right:) > > > Signed-off-by: Zorro Lang <zlang@redhat.com> > > --- > > tests/xfs/194 | 23 +++++++++++++++++++---- > > 1 file changed, 19 insertions(+), 4 deletions(-) > > > > diff --git a/tests/xfs/194 b/tests/xfs/194 > > index e11b459..4491dd4 100755 > > --- a/tests/xfs/194 > > +++ b/tests/xfs/194 > > @@ -42,7 +42,7 @@ _cleanup() > > > > # only xfs supported due to use of xfs_bmap > > _supported_fs xfs > > -_supported_os IRIX Linux > > +_supported_os Linux > > > > # real QA test starts here > > rm -f $seqres.full > > @@ -50,6 +50,16 @@ rm -f $seqres.full > > # For this test we use block size = 1/8 page size > > pgsize=`$here/src/feature -s` > > blksize=`expr $pgsize / 8` > > +secsize=`_min_dio_alignment $SCRATCH_DEV` > > + > > +# The minimal blksize can't less than sector size, So if > > +# blksize < secsize, we should adjust blksize and pgsize number. > > +# Of course, if we adjust pgsize, pgsize won't equal to the > > +# real page size of system. > > +if [ $blksize -lt $secsize ];then > > + blksize=$secsize > > + pgsize=`expr $blksize \* 8` > > +fi > > No, this is wrong. the page size stays fixed at the machine page > size. We are testing *sub-page block sizes* here and the sector size > must be <= page size. Increasing the "page size" to larger than the > machine page size does not make the kernel use larger page sizes. > > IOWs, if you've got sector size = page size (e.g. 4k sector device) > then no matter what you say $pgsize is, the kernel will see a block > size = page size test. > > This whole chunk of code can simply be replaced with: > > blksize=`_min_dio_alignment $SCRATCH_DEV` > > Because that's what we actually need to test... > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com > -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 18, 2015 at 06:03:45PM -0500, Eric Sandeen wrote: > On 8/18/15 5:43 PM, Dave Chinner wrote: > > On Tue, Aug 18, 2015 at 05:33:05PM -0500, Eric Sandeen wrote: > >> On 8/18/15 5:28 PM, Dave Chinner wrote: > >>> On Wed, Aug 19, 2015 at 01:21:51AM +0800, Zorro Lang wrote: > >>>> @@ -50,6 +50,16 @@ rm -f $seqres.full > >>>> # For this test we use block size = 1/8 page size > >>>> pgsize=`$here/src/feature -s` > >>>> blksize=`expr $pgsize / 8` > >>>> +secsize=`_min_dio_alignment $SCRATCH_DEV` > >>>> + > >>>> +# The minimal blksize can't less than sector size, So if > >>>> +# blksize < secsize, we should adjust blksize and pgsize number. > >>>> +# Of course, if we adjust pgsize, pgsize won't equal to the > >>>> +# real page size of system. > >>>> +if [ $blksize -lt $secsize ];then > >>>> + blksize=$secsize > >>>> + pgsize=`expr $blksize \* 8` > >>>> +fi > >>> > >>> No, this is wrong. the page size stays fixed at the machine page > >>> size. We are testing *sub-page block sizes* here and the sector size > >>> must be <= page size. Increasing the "page size" to larger than the > >>> machine page size does not make the kernel use larger page sizes. > >>> > >>> IOWs, if you've got sector size = page size (e.g. 4k sector device) > >>> then no matter what you say $pgsize is, the kernel will see a block > >>> size = page size test. > >>> > >>> This whole chunk of code can simply be replaced with: > >>> > >>> blksize=`_min_dio_alignment $SCRATCH_DEV` > >>> > >>> Because that's what we actually need to test... > >> > >> That won't work either, because we could easily get 512 from that. > > > > If 'blockdev --getss $dev' returns 512, then the device supports 512 > > byte IOs and so it is fine to do 512 byte IOs in the test. > > > >> and then this test: > >> > >> # Now try the same thing but write a sector in the middle of that hole > >> # If things go badly stale data will be exposed either side. > >> # This is most interesting for block size > 512 (page size > 4096) > >> > >> # We *should* get: > >> # |1100|HHHH|33HH|HHHH|2222|----|----|----| > >> > >> echo "== Test 4 ==" > >> xfs_io \ > >> -c "pwrite -S 0x11 -b $pgsize 0 $pgsize" \ > >> -c "mmap -r 0 $blksize" -c "mread 0 $blksize" -c "munmap" \ > >> -c "truncate `expr $blksize / 2`" \ > >> -c "truncate `expr $blksize + 1`" \ > >> -c "pwrite -S 0x22 -b $blksize `expr $pgsize / 2` $blksize" \ > >> -c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" \ > >> -t -d -f $SCRATCH_MNT/testfile4 >> $seqres.full > >> > >> will be impossible. > >> > >> AFAICT everything works except for that explicit 512-byte IO. > > > > Right. That hard coded 512 needs to change to $blksize, because > > blksize is now equal to the sector size. I thought this would be > > obvious to the reader, so I didn't comment on it. > > if that last IO is $blksize, and blocksize == sector size, then the > test won't be testing what it's designed to test here, i.e. a > sub-block direct IO write. That's not what the test is exercising: # Test mapping around/over holes for sub-page blocks it's testing *sub-page block behaviour*, not sub-block direct IO. > > # We *should* get: > # |1100|HHHH|33HH|HHHH|2222|----|----|----| > ^^ > this That implies a sub-block sized direct IO, on a single page that has 8 blocks. On a 4k page size machine, that is impossible and so most of the time we are not doing what the comment implies. With a 4k page, 512 byte block size: | xfs_io \ | -c "pwrite -S 0x11 -b $pgsize 0 $pgsize" \ Write an entire page (4k) # |1111|1111|1111|1111|1111|1111|1111|1111| | -c "mmap -r 0 $blksize" -c "mread 0 $blksize" -c "munmap" \ map the first block (0-511 bytes - one sector) | -c "truncate `expr $blksize / 2`" \ | -c "truncate `expr $blksize + 1`" \ sub-block truncate down, sub-block truncate up, make sure page cache is correctly zeroed. # |1100|HHHH|----|----|----|----|----|----| | -c "pwrite -S 0x22 -b $blksize `expr $pgsize / 2` $blksize" \ DIO write of a single block half way through the original page, make sure page cache is flushed correctly before DIO. # |1100|HHHH|HHHH|HHHH|2222|----|----|----| FWIW, this write will fail on a 4k sector device on a 4k page size platform, because the IO is not sector aligned, and is why the original patch needed to multiply pgsize out to 8 * sector size.... | -c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" \ do a -minimum sized write- to the *3rd* block in the page. # |1100|HHHH|3333|HHHH|2222|----|----|----| And that matches the expected output. We do not get this output with a block size that is anything other than pgsize / 8, regardless of whether the last write is a sub-block DIO or not. IOWs, this test assumes that there are at least 8 blocks to page because to exercise the appropriate paths it needs a hole between each region that is written. 4k sector/4k page means the kernel cannot do sub-page block size operations, and the test does not exercise the code paths we're expecting it to. Hence it may simply be best to do this: if [ $sector_size > $page_size / 8 ]; then _not_run "sector size too large for platform page size" fi and replace the hard coded 512 with $sector_size. Cheers, Dave.
On Tue, Aug 18, 2015 at 10:24:59PM -0400, Zirong Lang wrote: > > > ----- ???? ----- > > ???: "Dave Chinner" <david@fromorbit.com> > > ???: "Zorro Lang" <zlang@redhat.com> > > ??: fstests@vger.kernel.org, sandeen@redhat.com > > ????: ???, 2015? 8 ? 19? ?? 6:28:32 > > ??: Re: [PATCH] xfs/194: fix the exception when run on 4k sector drives > > > > On Wed, Aug 19, 2015 at 01:21:51AM +0800, Zorro Lang wrote: > > > The below command in "Test 4": > > > > > > xfs_io -c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" > > > > > > will run failed on 4k sector drives. So I use min_alignment size > > > to replace the hard-code 512. > > > > > > Also I make sure the blksize won't less than min_alignment size, > > > after blksize=`expr $pgsize / 8`. > > > > > > If blksize really less than min_alignment size, I set blksize = > > > min_alignment size, and for sure the consistency of test result, > > > I repair pgsize(already not real page size) number according to > > > the new blksize. > > > > > > Because IRIX can't use _min_dio_alignment(), so remove it from > > > supported os list. > > > > Not true - that's what the 'feature -s' branch in > > _min_dio_alignment() is supposed to be for. Just add another check > > for "$HOSTOS" == "Linux".... > > Do you mean change _min_dio_alignment() to: > _min_dio_alignment() > { > dev=$1 > > if [ -b "$dev" -a $HOSTOS" == "Linux" ]; then > blockdev --getss $dev > else > $here/src/feature -s > fi > } Yes. > I really don't understand why page size will be the minimum dio alignment? > Do you mean in other OS(except linux), the sector size = page size? No. It means that on other platforms the page size will be used as alignment restrictions. If those platforms need anything different, then they can add a similar 'elif [ "$HOSTOS" == "foo" ];' branch in there to call the appropriate function. You don't need to worry about that. Cheers, Dave.
On 8/18/15 9:42 PM, Dave Chinner wrote: > if [ $sector_size > $page_size / 8 ]; then > _not_run "sector size too large for platform page size" > fi > > and replace the hard coded 512 with $sector_size. I agree. -Eric -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
----- ???? ----- > ???: "Dave Chinner" <david@fromorbit.com> > ???: "Eric Sandeen" <sandeen@sandeen.net> > ??: "Eric Sandeen" <sandeen@redhat.com>, "Zorro Lang" <zlang@redhat.com>, fstests@vger.kernel.org > ????: ???, 2015? 8 ? 19? ?? 10:42:16 > ??: Re: [PATCH] xfs/194: fix the exception when run on 4k sector drives > > On Tue, Aug 18, 2015 at 06:03:45PM -0500, Eric Sandeen wrote: > > On 8/18/15 5:43 PM, Dave Chinner wrote: > > > On Tue, Aug 18, 2015 at 05:33:05PM -0500, Eric Sandeen wrote: > > >> On 8/18/15 5:28 PM, Dave Chinner wrote: > > >>> On Wed, Aug 19, 2015 at 01:21:51AM +0800, Zorro Lang wrote: > > >>>> @@ -50,6 +50,16 @@ rm -f $seqres.full > > >>>> # For this test we use block size = 1/8 page size > > >>>> pgsize=`$here/src/feature -s` > > >>>> blksize=`expr $pgsize / 8` > > >>>> +secsize=`_min_dio_alignment $SCRATCH_DEV` > > >>>> + > > >>>> +# The minimal blksize can't less than sector size, So if > > >>>> +# blksize < secsize, we should adjust blksize and pgsize number. > > >>>> +# Of course, if we adjust pgsize, pgsize won't equal to the > > >>>> +# real page size of system. > > >>>> +if [ $blksize -lt $secsize ];then > > >>>> + blksize=$secsize > > >>>> + pgsize=`expr $blksize \* 8` > > >>>> +fi > > >>> > > >>> No, this is wrong. the page size stays fixed at the machine page > > >>> size. We are testing *sub-page block sizes* here and the sector size > > >>> must be <= page size. Increasing the "page size" to larger than the > > >>> machine page size does not make the kernel use larger page sizes. > > >>> > > >>> IOWs, if you've got sector size = page size (e.g. 4k sector device) > > >>> then no matter what you say $pgsize is, the kernel will see a block > > >>> size = page size test. > > >>> > > >>> This whole chunk of code can simply be replaced with: > > >>> > > >>> blksize=`_min_dio_alignment $SCRATCH_DEV` > > >>> > > >>> Because that's what we actually need to test... > > >> > > >> That won't work either, because we could easily get 512 from that. > > > > > > If 'blockdev --getss $dev' returns 512, then the device supports 512 > > > byte IOs and so it is fine to do 512 byte IOs in the test. > > > > > >> and then this test: > > >> > > >> # Now try the same thing but write a sector in the middle of that hole > > >> # If things go badly stale data will be exposed either side. > > >> # This is most interesting for block size > 512 (page size > 4096) > > >> > > >> # We *should* get: > > >> # |1100|HHHH|33HH|HHHH|2222|----|----|----| > > >> > > >> echo "== Test 4 ==" > > >> xfs_io \ > > >> -c "pwrite -S 0x11 -b $pgsize 0 $pgsize" \ > > >> -c "mmap -r 0 $blksize" -c "mread 0 $blksize" -c "munmap" \ > > >> -c "truncate `expr $blksize / 2`" \ > > >> -c "truncate `expr $blksize + 1`" \ > > >> -c "pwrite -S 0x22 -b $blksize `expr $pgsize / 2` $blksize" \ > > >> -c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" \ > > >> -t -d -f $SCRATCH_MNT/testfile4 >> $seqres.full > > >> > > >> will be impossible. > > >> > > >> AFAICT everything works except for that explicit 512-byte IO. > > > > > > Right. That hard coded 512 needs to change to $blksize, because > > > blksize is now equal to the sector size. I thought this would be > > > obvious to the reader, so I didn't comment on it. > > > > if that last IO is $blksize, and blocksize == sector size, then the > > test won't be testing what it's designed to test here, i.e. a > > sub-block direct IO write. > > That's not what the test is exercising: > > # Test mapping around/over holes for sub-page blocks > > it's testing *sub-page block behaviour*, not sub-block direct IO. > > > > > # We *should* get: > > # |1100|HHHH|33HH|HHHH|2222|----|----|----| > > ^^ > > this > > That implies a sub-block sized direct IO, on a single page that has > 8 blocks. On a 4k page size machine, that is impossible and so most > of the time we are not doing what the comment implies. > > With a 4k page, 512 byte block size: > > | xfs_io \ > | -c "pwrite -S 0x11 -b $pgsize 0 $pgsize" \ > > Write an entire page (4k) > > # |1111|1111|1111|1111|1111|1111|1111|1111| > > | -c "mmap -r 0 $blksize" -c "mread 0 $blksize" -c "munmap" \ > > map the first block (0-511 bytes - one sector) > > | -c "truncate `expr $blksize / 2`" \ > | -c "truncate `expr $blksize + 1`" \ > > sub-block truncate down, sub-block truncate up, make sure page cache > is correctly zeroed. > > # |1100|HHHH|----|----|----|----|----|----| > > | -c "pwrite -S 0x22 -b $blksize `expr $pgsize / 2` $blksize" \ > > DIO write of a single block half way through the original page, make > sure page cache is flushed correctly before DIO. > > # |1100|HHHH|HHHH|HHHH|2222|----|----|----| > > FWIW, this write will fail on a 4k sector device on a 4k page size > platform, because the IO is not sector aligned, and is why the > original patch needed to multiply pgsize out to 8 * sector size.... > > | -c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" \ > > do a -minimum sized write- to the *3rd* block in the page. > > # |1100|HHHH|3333|HHHH|2222|----|----|----| Yes, that's true. if sector size and block size all 512b, we will get this. In my test machine(64k page size, and 4k sector size), this case will mkfs with blksize=8k, and we can get: |1100|HHHH|33HH|HHHH|2222|----|----|----| (one |----| means 8k block size at here:) > > And that matches the expected output. We do not get this output with > a block size that is anything other than pgsize / 8, regardless of > whether the last write is a sub-block DIO or not. > > IOWs, this test assumes that there are at least 8 blocks to page > because to exercise the appropriate paths it needs a hole between > each region that is written. 4k sector/4k page means the kernel > cannot do sub-page block size operations, and the test does not > exercise the code paths we're expecting it to. Hence it may simply > be best to do this: > > if [ $sector_size > $page_size / 8 ]; then > _not_run "sector size too large for platform page size" > fi If so, that'll be simple. I will send V2 patch for review;) Thanks, Zorro > > and replace the hard coded 512 with $sector_size. > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com > -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tests/xfs/194 b/tests/xfs/194 index e11b459..4491dd4 100755 --- a/tests/xfs/194 +++ b/tests/xfs/194 @@ -42,7 +42,7 @@ _cleanup() # only xfs supported due to use of xfs_bmap _supported_fs xfs -_supported_os IRIX Linux +_supported_os Linux # real QA test starts here rm -f $seqres.full @@ -50,6 +50,16 @@ rm -f $seqres.full # For this test we use block size = 1/8 page size pgsize=`$here/src/feature -s` blksize=`expr $pgsize / 8` +secsize=`_min_dio_alignment $SCRATCH_DEV` + +# The minimal blksize can't less than sector size, So if +# blksize < secsize, we should adjust blksize and pgsize number. +# Of course, if we adjust pgsize, pgsize won't equal to the +# real page size of system. +if [ $blksize -lt $secsize ];then + blksize=$secsize + pgsize=`expr $blksize \* 8` +fi # Filter out file mountpoint and physical location info # Input: @@ -84,8 +94,13 @@ _require_scratch unset MKFS_OPTIONS unset XFS_MKFS_OPTIONS -# we need 512 byte block size, so crc's are turned off -_scratch_mkfs_xfs -m crc=0 -b size=$blksize >/dev/null 2>&1 +# If we use 512 byte block size, can't use crc +if [ $blksize -eq 512 ];then + crcflag=0 +else + crcflag=1 +fi +_scratch_mkfs_xfs -m crc=$crcflag -b size=$blksize >/dev/null 2>&1 _scratch_mount # 512b block / 4k page example: @@ -209,7 +224,7 @@ xfs_io \ -c "truncate `expr $blksize / 2`" \ -c "truncate `expr $blksize + 1`" \ -c "pwrite -S 0x22 -b $blksize `expr $pgsize / 2` $blksize" \ --c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" \ +-c "pwrite -S 0x33 -b $secsize `expr $blksize \* 2` $secsize" \ -t -d -f $SCRATCH_MNT/testfile4 >> $seqres.full xfs_bmap -v $SCRATCH_MNT/testfile4 | _filter_bmap
The below command in "Test 4": xfs_io -c "pwrite -S 0x33 -b 512 `expr $blksize \* 2` 512" will run failed on 4k sector drives. So I use min_alignment size to replace the hard-code 512. Also I make sure the blksize won't less than min_alignment size, after blksize=`expr $pgsize / 8`. If blksize really less than min_alignment size, I set blksize = min_alignment size, and for sure the consistency of test result, I repair pgsize(already not real page size) number according to the new blksize. Because IRIX can't use _min_dio_alignment(), so remove it from supported os list. At last, make the crc flag be disabled only when blksize=512. Signed-off-by: Zorro Lang <zlang@redhat.com> --- tests/xfs/194 | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)