[Ocfs2-tools-devel] [Ocfs2-test-devel] [PATCH 2/6] New set of inline-data testing tools:Add single-node testing binary for regular file

[tristan.ye]

On Mon, 2008-09-01 at 13:41 +0800, Tao Ma wrote:
> Hi tristan,
> 	Thanks for your patch.
> 
> tristan.ye wrote:
> > Have sent this patch out before,now just have some other minor
> > modifications,
> > 
> > This patch mainly aims at ehancing the existing inline-data testing
> > tool(ocfs2-test/programs/inline-data/inline-data.c) by introducing my
> > newly added testcases and make it more flexbile to tune workload as to
> > perform different testing requirements ,such as stress test.
> > 
> > Signed-off-by: Tristan Ye <tristan.ye at oracle.com>
> > ---
> 
> > +static char mount_point[255];
> > +static char work_place[255];
> > +static char dirent[255];
> > +static char file_name[255];
> changing all these to PATH_MAX should be safe enough.
> > +
> > +static int iteration = 1;
> > +static int do_multi_process_test = 0;
> > +static int do_multi_file_test = 0;
> > +static unsigned long child_nums = 2;
> > +static unsigned long file_nums = 2;
> > +
> > +pid_t *child_pid_list = NULL;
> > +
> > +static unsigned long get_rand(unsigned long min, unsigned long max)
> > +{
> > +	if (min == 0 && max == 0)
> > +		return 0;
> > +
> > +	return min + (rand() % (max - min + 1));
> > +}
> > +
> > +static inline char rand_char(void)
> > +{
> > +	return 'A' + (char) get_rand(0, 25);
> > +}
> > +
> > +static void fill_pattern(int size)
> > +{
> > +	int i;
> > +
> > +	/*
> > +	 * Make sure that anything in the buffer past size is zero'd,
> > +	 * as a regular file should look.
> > +	 */
> > +	memset(pattern, 0, PATTERN_SZ);
> > +
> > +	for(i = 0; i < size; i++)
> > +		pattern[i] = rand_char();
> > +}
> > +
> > +static int truncate_pattern(int fd, unsigned int old_size,
> > +			    unsigned int new_size)
> > +{
> > +	int bytes = old_size - new_size;
> > +	int ret;
> > +
> > +	memset(pattern + new_size, 0, bytes);
> > +
> > +	ret = ftruncate(fd, new_size);
> > +	if (ret == -1) {
> > +		fprintf(stderr, "ftruncate error %d: \"%s\"\n",
> > +			ret, strerror(ret));
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int write_at(int fd, const void *buf, size_t count, off_t
> > offset);
> > +
> > +static int extend_pattern(int fd, unsigned int old_size,
> > +			  unsigned int new_size)
> > +{
> > +	int bytes = new_size - old_size;
> > +	int ret;
> > +	int i;
> > +
> > +	memset(pattern + old_size, 0, bytes);
> > +	/*fill extended area with reandom chars*/
> > +	for (i = 0; i < bytes; i++)
> > +		pattern[old_size + i] = rand_char();
> > +
> > +	ret = ftruncate(fd, new_size);
> > +	if (ret == -1) {
> > +		fprintf(stderr, "ftruncate error %d: \"%s\"\n",
> > +			ret, strerror(ret));
> > +		return -1;
> > +	}
> > +
> > +	ret = write_at(fd, pattern + old_size, bytes, old_size);
> > +	if (ret < 0)
> > +		return ret;
> > +	
> > +	return 0;
> > +}
> > +
> > +static int try_ioctl(int which, int fd, unsigned int offset, unsigned
> > int count)
> > +{
> > +	struct ocfs2_space_resv sr;
> > +
> > +	memset(&sr, 0, sizeof(sr));
> > +	sr.l_whence = SEEK_SET;
> > +	sr.l_start = offset;
> > +	sr.l_len = count;
> > +
> > +	return ioctl(fd, which, &sr);
> > +}
> > +
> > +static int try_reserve(int fd, unsigned int offset, unsigned int count)
> > +{
> > +	int ret;
> > +
> > +	ret = try_ioctl(OCFS2_IOC_RESVSP, fd, offset, count);
> > +	if (ret == -1) {
> > +		ret = errno;
> > +		if (ret == ENOTTY)
> > +			return ret;
> > +		fprintf(stderr, "IOC_RESVSP error %d: \"%s\"\n",
> > +			ret, strerror(ret));
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int try_punch_hole(int fd, unsigned int offset, unsigned int
> > count)
> > +{
> > +	int ret;
> > +
> > +	memset(pattern + offset, 0, count);
> > +
> > +	ret = try_ioctl(OCFS2_IOC_UNRESVSP, fd, offset, count);
> > +	if (ret == -1) {
> > +		ret = errno;
> > +		if (ret == ENOTTY)
> > +			return ret;
> > +		fprintf(stderr, "IOC_UNRESVSP error %d: \"%s\"\n",
> > +			ret, strerror(ret));
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int mmap_write_at(int fd, const char *buf, size_t count, off_t
> > offset)
> > +{
> > +	unsigned int mmap_size = page_size;
> > +	unsigned int size = count + offset;
> > +	int i, j, ret;
> > +	char *region;
> > +
> > +	while (mmap_size < size)
> > +		mmap_size += page_size;
> what's this? you want to align mmap_size to page_size and larger than size?
> how about mmap_size = (size + page_size - 1) & ~(page_size - 1)?

That's exactly all what i've copied from original
version(inline-data.c),which maybe written by mark, and i saw no hurts
for us to extend the scope of mmap maped region to make it page_size
aligned.

> > +
> > +	region = mmap(NULL, mmap_size, PROT_WRITE, MAP_SHARED, fd, 0);
> > +	if (region == MAP_FAILED) {
> > +		ret = errno;
> > +		fprintf(stderr, "mmap (write) error %d: \"%s\"\n", ret,
> > +			strerror(ret));
> > +		return -1;
> > +	}
> > +
> > +	j = 0;
> > +	for(i = offset; i < size; i++)
> > +		region[i] = buf[j++];
> > +
> > +	munmap(region, mmap_size);
> > +
> > +	return 0;
> > +}
> > +
> > 
> > +
> > +static void sigint_handler()
> > +{
> > +	/*Kill all children when father interrupted*/
> > +	int i;
> > +	int process_nums;
> > +
> > +	if (do_multi_process_test)
> > +		process_nums = child_nums;
> > +	else
> > +		process_nums = file_nums;
> > +
> > +	for (i = 0; i < process_nums; i++)
> > +                kill(child_pid_list[i], SIGTERM);
> > +	
> > +	free(child_pid_list);
> > +	signal(SIGINT, SIG_DFL);
> > +	kill(getpid(), SIGINT);
> > +
> > +}
> > +
> > +static void sigterm_handler()
> > +{
> > +	/*Kill all children when father terminated*/
> > +        int i;
> > +        int process_nums;
> > +
> > +	if (do_multi_process_test)
> > +		process_nums = child_nums;
> > +	else
> > +		process_nums = file_nums;
> > +
> > +        for (i = 0; i < process_nums; i++)
> > +                kill(child_pid_list[i], SIGTERM);
> > +
> > +	free(child_pid_list);
> > +	signal(SIGTERM, SIG_DFL);
> > +	kill(getpid(), SIGTERM);
> > +}
> These 2 functions are similar, why merge them together. It looks that 
> there is only the last line is a little different but you can get the 
> original signal by the callback. see signal(2).
> 
> btw, there are still problems with style. please use checkpatch for it.
Great thanks to you for reviewing the patches.

Yes,your suggestions and notes were really helpful for me to do things
better,there seems quite a lot problems here since its a CAN-WORK
version now,I'll enhance the details in following patches.


Tristan.


> 
> Regards,
> Tao