[LTP] [PATCH 3/5] Add test for file data integrity

[Martin Doucha]

On 04. 12. 24 14:45, Cyril Hrubis wrote:
> Hi!
>> +static void vectorize_buffer(struct iovec *vec, size_t vec_size, char *buf,
>> +	size_t buf_size, int align)
>> +{
>> +	size_t i, len, chunk = align ? blocksize : 1;
>> +
>> +	memset(vec, 0, vec_size * sizeof(struct iovec));
>> +	buf_size /= chunk;
>> +
>> +	for (i = 0; buf_size && i < vec_size; i++) {
>> +		len = 1 + rand() % (buf_size + i + 1 - vec_size);
> 
> You have 1 + ... before the rand should it be buf_size:
> 
> i - 1 - vec_size
> 
> Also since 0 sized iov_len is actually correct, it means skip the entry,
> we do not have to make sure that the length is > 0 here and neither we
> need to make sure that we have enough buffer saved for the rest of the
> vector.

If we dropped the 1+ from the calculation, it should look like this:
len = rand() % (bufsize + 1);

Because rand() % x <= x-1.

> 
> 
>> +		vec[i].iov_base = buf;
>> +		vec[i].iov_len = len * chunk;
>> +		buf += vec[i].iov_len;
>> +		buf_size -= len;
>> +	}
>> +
>> +	vec[vec_size - 1].iov_len += buf_size * chunk;
>> +}
>> +
>> +static void update_filedata(const void *buf, size_t offset, size_t size)
>> +{
>> +	memcpy(filedata + offset, buf, size * sizeof(char));
>                                                  ^
> 						This is 1 by definition.

I know, but I prefer to explicitly annotate the array type in 
allocations and copy operations for future review and reference. The 
compiler will optimize it out anyway.

>> +	blocksize = statbuf.f_bsize;
>> +	tst_res(TINFO, "Block size: %zu", blocksize);
>> +	bufsize = 4 * MAX_VEC * MAX(pagesize, blocksize);
>> +	filesize = 1024 * MAX(pagesize, blocksize);
>> +	writebuf = SAFE_MMAP(NULL, bufsize, PROT_READ | PROT_WRITE,
>> +		MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>> +	filedata = SAFE_MALLOC(filesize);
>> +
>> +	tst_set_max_runtime(bufsize * loop_count / (8 * 1024 * 1024));
> 
> I think that it may be better to define the test to be time based rather
> than iteration based. If we decide that the default runtime would be 5
> minutes the test would do as much iterations during that time as
> possible and report how many it did at the end of the test. Possibly it
> may warn if minimal amount of iterations wasn't reached. That would make
> the test runtime much more predictable

That would be a useful command line argument for the NFS test variants. 
But this patch is already merged so a separate new patch is needed 
either way.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic