mkswap: make the test device size is aligned to pagesize

We get failures like below because of 'DEVICE_SIZE-10000' makes the loop device size is not aligned to system PAGE_SIZE any more.

Hi!

Quoted 2 lines

We get failures like below because of 'DEVICE_SIZE-10000' makes the loop device size is not aligned to system PAGE_SIZE any more.

What version of mkswap.c you have?

Looking into util-linux-2.26.2 all it does with user specified number of blocks is:

ctl.npages = blks / (ctl.pagesize / 1024);

There is no check that blks is divideable by pagesize/1024.

My version is:

# rpm -qf /sbin/mkswap 
util-linux-2.23.2-33.el7.ppc64

if (block_count) {
	/* this silly user specified the number of blocks explicitly */
		uint64_t blks = strtou64_or_err(block_count,
					_("invalid block count argument"));
		PAGES = blks / (pagesize / 1024);
	}
	sz = get_size(device_name);

Quoted 6 lines

Looking into util-linux-2.26.2 all it does with user specified number of blocks is:

ctl.npages = blks / (ctl.pagesize / 1024);

There is no check that blks is divideable by pagesize/1024.

I build util-linux-2.28 from upstrem source and also get the errors.

# mkswap -V
mkswap from util-linux 2.28.237-47bd89

# ./runltp -f commands
mkswap01 1 TINFO : Using test device LTP_DEV='/dev/loop0'
mkswap01 1 TFAIL : 'mkswap   /dev/loop0 ' failed, not expected.
mkswap01 2 TFAIL : 'mkswap   /dev/loop0 90000' failed, not expected.
mkswap01 3 TINFO : Can not do swapon on /dev/loop0.
mkswap01 3 TINFO : Device size specified by 'mkswap' greater than real size.
mkswap01 3 TINFO : Swapon failed expectedly.
mkswap01 3 TPASS : 'mkswap -f  /dev/loop0 110000' passed.

Additionally, we'd better change the bs=1kB to bs=1024 in runltp file, since it will get page aligned block device for all arches.

The original LTP_DEV size is 102400000 bytes:

# grep 'bs=' runltp  -n
981:    dd if=/dev/zero of=${TMP}/test.img bs=1kB count=102400

From dd manual:
N and BYTES may be followed by the following multiplicative suffixes:
c =1, w =2, b =512, kB =1000, K =1024, MB =1000*1000, M =1024*1024, xM =M GB =1000*1000*1000, G =1024*1024*1024, and so on for T, P, E, Z, Y.

Hi!

Quoted 6 lines

My version is:

# rpm -qf /sbin/mkswap 
util-linux-2.23.2-33.el7.ppc64

Quoted 7 lines

if (block_count) {
	/* this silly user specified the number of blocks explicitly */
		uint64_t blks = strtou64_or_err(block_count,
					_("invalid block count argument"));
		PAGES = blks / (pagesize / 1024);
	}
	sz = get_size(device_name);

There is no check here as well. The strtou64_or_err() just converts string to number, that's all it does. Then we do integer division, no checks at all.

Quoted 12 lines

Quoted 6 lines

Looking into util-linux-2.26.2 all it does with user specified number of blocks is:

ctl.npages = blks / (ctl.pagesize / 1024);

There is no check that blks is divideable by pagesize/1024.

I build util-linux-2.28 from upstrem source and also get the errors.

# mkswap -V
mkswap from util-linux 2.28.237-47bd89

Quoted 8 lines

# ./runltp -f commands
mkswap01 1 TINFO : Using test device LTP_DEV='/dev/loop0'
mkswap01 1 TFAIL : 'mkswap   /dev/loop0 ' failed, not expected.
mkswap01 2 TFAIL : 'mkswap   /dev/loop0 90000' failed, not expected.
mkswap01 3 TINFO : Can not do swapon on /dev/loop0.
mkswap01 3 TINFO : Device size specified by 'mkswap' greater than real size.
mkswap01 3 TINFO : Swapon failed expectedly.
mkswap01 3 TPASS : 'mkswap -f  /dev/loop0 110000' passed.

Hmm, what is the exact error message from mkfs here?

I guess that it would be a good idea to cat the temp file if mkswap failed unexpectedly so that we can see the mkswap error message.

Quoted 9 lines

Additionally, we'd better change the bs=1kB to bs=1024 in runltp file, since it will get page aligned block device for all arches.

The original LTP_DEV size is 102400000 bytes:

# grep 'bs=' runltp  -n
981:    dd if=/dev/zero of=${TMP}/test.img bs=1kB count=102400

Quoted 4 lines

From dd manual:
N and BYTES may be followed by the following multiplicative suffixes:
c =1, w =2, b =512, kB =1000, K =1024, MB =1000*1000, M =1024*1024, xM =M GB =1000*1000*1000, G =1024*1024*1024, and so on for T, P, E, Z, Y.

Or change it to uppercase KB. Sounds good, feel free to send a patch.

Sorry for the vague description. To be honest, mkswap.c(util-linux) is innocent here. The error maker is mkswap_verify() in mkswap01.sh(ltp) testcase.

The key point of problem is not only mkswap01.sh (ltp) doesn't guarantee test-device(LTP_DEV) size is aligned to PAGE_SIZE on all arches, but also it increse/decrease test-device size not aligned too.

# tail -15 mkswap01.sh
mkswap_test "" "" "$TST_DEVICE"
mkswap_test "" "" "$TST_DEVICE" "$((DEVICE_SIZE-10000))"     <---
mkswap_test "-f" "" "$TST_DEVICE" "$((DEVICE_SIZE+10000))"   <---
...

As we know that mkswap.c(util-linux) format test-device to aligned with its local PAGE_SIZE by force. Therefore, the real 'filesize' of formated test-device will be decreased in that phase. But final, the testcase just compare orignal 'filesize' to 'diff'(swaptotal-after - swaptotal-before) and make conclusion to get PASS/FAIL. That makes this test result is not reliable.

Hi!

Quoted 14 lines

Sorry for the vague description. To be honest, mkswap.c(util-linux) is innocent here. The error maker is mkswap_verify() in mkswap01.sh(ltp) testcase.

The key point of problem is not only mkswap01.sh (ltp) doesn't guarantee test-device(LTP_DEV) size is aligned to PAGE_SIZE on all arches, but also it increse/decrease test-device size not aligned too.

# tail -15 mkswap01.sh
mkswap_test "" "" "$TST_DEVICE"
mkswap_test "" "" "$TST_DEVICE" "$((DEVICE_SIZE-10000))"     <---
mkswap_test "-f" "" "$TST_DEVICE" "$((DEVICE_SIZE+10000))"   <---
...

Quoted 5 lines

As we know that mkswap.c(util-linux) format test-device to aligned with its local PAGE_SIZE by force. Therefore, the real 'filesize' of formated test-device will be decreased in that phase. But final, the testcase just compare orignal 'filesize' to 'diff'(swaptotal-after - swaptotal-before) and make conclusion to get PASS/FAIL. That makes this test result is not reliable.

Now that makes sense.

But if that is the case we still have to align down the device size to the page size at the start of the test. Since the device can also be passed down to the runltp script as a parameter and we cannot make any assumptions about its size.

Quoted 6 lines

Now that makes sense.

But if that is the case we still have to align down the device size to the page size at the start of the test. Since the device can also be passed down to the runltp script as a parameter and we cannot make any assumptions about its size.

Hmm, yes, we have to face the problem.

A simple idea comes to my mind is that the loop device size should be customized by specific testcase(mkswap01.sh).

That would be happy if function "tst_acquire_device() 'NUM'" can get 'NUM' MB block device for testing.

It means we have to modify another path which about tst_acquire_device().

Hi!

Quoted 9 lines

Quoted 6 lines

Now that makes sense.

But if that is the case we still have to align down the device size to the page size at the start of the test. Since the device can also be passed down to the runltp script as a parameter and we cannot make any assumptions about its size.

Hmm, yes, we have to face the problem.

Quoted 7 lines

A simple idea comes to my mind is that the loop device size should be customized by specific testcase(mkswap01.sh).

That would be happy if function "tst_acquire_device() 'NUM'" can get 'NUM' MB block device for testing.

It means we have to modify another path which about tst_acquire_device().

This is a bad idea for two reasons.

Fist one is that the reason the user passed block device to runltp may be that the kernel does not support loop devices. In this case the test would be skipped for no good reason.

The second is that there is another patch in flight that aims to add minimal size parameter to the tst_acquire_device(). Which is much more useful.

Why cannot we just simply take the size of the device at start of the test, align it down to be the multiple of $PAGE_SIZE and use that as the size for testing?

Ok, this sounds better. I misunderstand you point at first.

As this we can get the right(aligned) size of the device by add:

# cat mkswap01.sh
...
setup()
{
	...

	# Here get the size of the device and align it down to be the
	# multiple of $PAGE_SIZE and use that as the size for testing.
	device_real=`blockdev --getsize64 $TST_DEVICE`
	device_resi=$((${device_real} % ${PAGE_SIZE}))

	DEVICE_SIZE=$(((${device_real} - ${device_resi})/1024))