fs: fsstress: make test more POSIX compliant

PATCH In review Caspar Zhang 2 participants 4 messages Mar 14, 2011 – Mar 16, 2011 1 message not in the archive

lore

Share & tools

lore

Caspar Zhang

author

2011-03-14 12:04 UTC #0 lore raw

PATCH 1/1 fs: fsstress: make test more POSIX compliant

[PATCH] fs: fsstress: make test more POSIX compliant

in commit bacc849720ec4efda5a0a8a9ea6a0e93a1415541, malloc.h was removed and stdlib.h got used instead. However, it's not enough. The function memalign() is also obsolete (via man memalign) and we should use posix_memalign instead. Also if we keep memalign() function here, the program would probably hit segfault once it enters dwrite_t and dread_t.

Besides the POSIX compliant fix, I also did a small fix to remove compling warnings.

I tried to update this program output to tst_resm/tst_brkm style but failed, I got confused by some printf output.

Signed-off-by Caspar Zhang <[email protected]>

---
 testcases/kernel/fs/fsstress/fsstress.c |   38
++++++++++++++++++------------
 1 files changed, 23 insertions(+), 15 deletions(-)

(quoted diff) +0 -0

b/testcases/kernel/fs/fsstress/fsstress.c
index e3b48ea..0e5fba2 100644

testcases/kernel/fs/fsstress/fsstress.c +23 ~~-15~~

@@ -1746,7 +1746,7 @@ void
 dread_f(int opno, long r)
 {
 	__int64_t	align;
-	char		*buf;
+	char		*buf=NULL;
 	struct dioattr	diob;
 	int		e;
 	pathname_t	f;

@@ -1799,11 +1799,9 @@ dread_f(int opno, long r)
 		return;
 	}
 
-	if (no_xfs) {
-		diob.d_miniosz = stb.st_blksize;
-		diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
-		diob.d_mem = stb.st_blksize;
-	}
+	diob.d_miniosz = stb.st_blksize;
+	diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
+	diob.d_mem = stb.st_blksize;
 #ifndef NO_XFS
 	else if (ioctl(fd, XFS_IOC_DIOINFO, &diob) < 0) {
 		if (v)

@@ -1826,7 +1824,13 @@ dread_f(int opno, long r)
 		len = align;
 	else if (len > diob.d_maxiosz)
 		len = diob.d_maxiosz;
-	buf = memalign(diob.d_mem, len);
+	if (posix_memalign((void **)buf, diob.d_mem, len) != 0) {
+		perror("posix_memalign");
+		exit (1);
+	} else if (buf == NULL) {
+		fprintf(stderr, "buf remains NULL unexpectly\n");
+		exit (1);
+	}
 	e = read(fd, buf, len) < 0 ? errno : 0;
 	free(buf);
 	if (v)

@@ -1840,7 +1844,7 @@ void
 dwrite_f(int opno, long r)
 {
 	__int64_t	align;
-	char		*buf;
+	char		*buf=NULL;
 	struct dioattr	diob;
 	int		e;
 	pathname_t	f;

@@ -1882,11 +1886,9 @@ dwrite_f(int opno, long r)
 		close(fd);
 		return;
 	}
-	if (no_xfs) {
-		diob.d_miniosz = stb.st_blksize;
-		diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
-		diob.d_mem = stb.st_blksize;
-	}
+	diob.d_miniosz = stb.st_blksize;
+	diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
+	diob.d_mem = stb.st_blksize;
 #ifndef NO_XFS
 	else if (ioctl(fd, XFS_IOC_DIOINFO, &diob) < 0) {
 		if (v)

@@ -1909,7 +1911,13 @@ dwrite_f(int opno, long r)
 		len = align;
 	else if (len > diob.d_maxiosz)
 		len = diob.d_maxiosz;
-	buf = memalign(diob.d_mem, len);
+	if (posix_memalign((void **)buf, diob.d_mem, len) != 0) {
+		perror("posix_memalign");
+		exit (1);
+	} else if (buf == NULL)	{
+		fprintf(stderr, "buf remains NULL unexpectly\n");
+		exit (1);
+	}
 	off %= maxfsize;
 	lseek64(fd, off, SEEK_SET);
 	memset(buf, nameseq & 0xff, len);

@@ -2650,4 +2658,4 @@ write_f(int opno, long r)
 			procid, opno, f.path, (long long)off, (long int)len, e);
 	free_pathname(&f);
 	close(fd);
-}
\ No newline at end of file
+}

Signature · 8 lines

1.7.4.1


-- 
Quality Engineer (Kernel) in
Red Hat Software (Beijing) Co., R&D Branch
http://www.cn.redhat.com/
TEL: +86-10-62608150

[PATCH] fs: fsstress: make test more POSIX compliant

Besides the POSIX compliant fix, I also did a small fix to remove compling warnings.

I tried to update this program output to tst_resm/tst_brkm style but failed, I got confused by some printf output.

Signed-off-by Caspar Zhang <[email protected]>

---
 testcases/kernel/fs/fsstress/fsstress.c |   38 ++++++++++++++++++------------
 1 files changed, 23 insertions(+), 15 deletions(-)

testcases/kernel/fs/fsstress/fsstress.c +23 ~~-15~~

index e3b48ea..0e5fba2 100644
@@ -1746,7 +1746,7 @@ void
 dread_f(int opno, long r)
 {
 	__int64_t	align;
-	char		*buf;
+	char		*buf=NULL;
 	struct dioattr	diob;
 	int		e;
 	pathname_t	f;

@@ -1799,11 +1799,9 @@ dread_f(int opno, long r)
 		return;
 	}
 
-	if (no_xfs) {
-		diob.d_miniosz = stb.st_blksize;
-		diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
-		diob.d_mem = stb.st_blksize;
-	}
+	diob.d_miniosz = stb.st_blksize;
+	diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
+	diob.d_mem = stb.st_blksize;
 #ifndef NO_XFS
 	else if (ioctl(fd, XFS_IOC_DIOINFO, &diob) < 0) {
 		if (v)

@@ -1826,7 +1824,13 @@ dread_f(int opno, long r)
 		len = align;
 	else if (len > diob.d_maxiosz)
 		len = diob.d_maxiosz;
-	buf = memalign(diob.d_mem, len);
+	if (posix_memalign((void **)buf, diob.d_mem, len) != 0) {
+		perror("posix_memalign");
+		exit (1);
+	} else if (buf == NULL) {
+		fprintf(stderr, "buf remains NULL unexpectly\n");
+		exit (1);
+	}
 	e = read(fd, buf, len) < 0 ? errno : 0;
 	free(buf);
 	if (v)

@@ -1840,7 +1844,7 @@ void
 dwrite_f(int opno, long r)
 {
 	__int64_t	align;
-	char		*buf;
+	char		*buf=NULL;
 	struct dioattr	diob;
 	int		e;
 	pathname_t	f;

@@ -1882,11 +1886,9 @@ dwrite_f(int opno, long r)
 		close(fd);
 		return;
 	}
-	if (no_xfs) {
-		diob.d_miniosz = stb.st_blksize;
-		diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
-		diob.d_mem = stb.st_blksize;
-	}
+	diob.d_miniosz = stb.st_blksize;
+	diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
+	diob.d_mem = stb.st_blksize;
 #ifndef NO_XFS
 	else if (ioctl(fd, XFS_IOC_DIOINFO, &diob) < 0) {
 		if (v)

@@ -1909,7 +1911,13 @@ dwrite_f(int opno, long r)
 		len = align;
 	else if (len > diob.d_maxiosz)
 		len = diob.d_maxiosz;
-	buf = memalign(diob.d_mem, len);
+	if (posix_memalign((void **)buf, diob.d_mem, len) != 0) {
+		perror("posix_memalign");
+		exit (1);
+	} else if (buf == NULL)	{
+		fprintf(stderr, "buf remains NULL unexpectly\n");
+		exit (1);
+	}
 	off %= maxfsize;
 	lseek64(fd, off, SEEK_SET);
 	memset(buf, nameseq & 0xff, len);

@@ -2650,4 +2658,4 @@ write_f(int opno, long r)
 			procid, opno, f.path, (long long)off, (long int)len, e);
 	free_pathname(&f);
 	close(fd);
-}
\ No newline at end of file
+}

Signature · 1 line

1.7.4.1

------------------------------------------------------------------------------
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d

_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Cyril Hrubis

2011-03-16 11:46 UTC #1 lore raw

Hi!

testcases/kernel/fs/fsstress/fsstress.c dread_f(int opno, long r) · line 1804

Quoted 8 lines

testcases/kernel/fs/fsstress/fsstress.c +3 -5

-	if (no_xfs) {
-		diob.d_miniosz = stb.st_blksize;
-		diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
-		diob.d_mem = stb.st_blksize;
-	}
+	diob.d_miniosz = stb.st_blksize;
+	diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
+	diob.d_mem = stb.st_blksize;

Hmm, why you are removing the if (no_xfs) here (I don't say it's wrong, I only want to know why)?

testcases/kernel/fs/fsstress/fsstress.c dread_f(int opno, long r) · line 1827

Quoted 9 lines

#ifndef NO_XFS

testcases/kernel/fs/fsstress/fsstress.c +1 -1

 	else if (ioctl(fd, XFS_IOC_DIOINFO, &diob) < 0) {
 		if (v)

@@ -1826,7 +1824,13 @@ dread_f(int opno, long r)
 		len = align;
 	else if (len > diob.d_maxiosz)
 		len = diob.d_maxiosz;
-	buf = memalign(diob.d_mem, len);
+	if (posix_memalign((void **)buf, diob.d_mem, len) != 0) {

The posix_memalign() actually returns pointer to the memory via buf, so instead of (void **)buf, you should write &buf (and that would silence the warning about uninitalized buf too).

Signature · 12 lines

Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

A message that is not in the archive (sent off-list or not yet received): [email protected]

Cyril Hrubis

2011-03-16 12:29 UTC #3 lore raw

Hi!

Quoted 32 lines

Quoted 11 lines
testcases/kernel/fs/fsstress/fsstress.c dread_f(int opno, long r) · line 1804
Quoted 8 lines
testcases/kernel/fs/fsstress/fsstress.c +3 -5
-	if (no_xfs) {
-		diob.d_miniosz = stb.st_blksize;
-		diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
-		diob.d_mem = stb.st_blksize;
-	}
+	diob.d_miniosz = stb.st_blksize;
+	diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
+	diob.d_mem = stb.st_blksize;
Hmm, why you are removing the if (no_xfs) here (I don't say it's wrong, I only want to know why)?
The compiler gave warning that diob.d_* may not initialized. I don't know what the values diob.d_* should be if XFS exists (no_xfs == 0), but I guess they should be the same as when XFS not available.
Quoted 15 lines
testcases/kernel/fs/fsstress/fsstress.c dread_f(int opno, long r) · line 1827
Quoted 10 lines
#ifndef NO_XFS
testcases/kernel/fs/fsstress/fsstress.c +1 -1
 	else if (ioctl(fd, XFS_IOC_DIOINFO, &diob) < 0) {
 		if (v)
@@ -1826,7 +1824,13 @@ dread_f(int opno, long r)
 		len = align;
 	else if (len > diob.d_maxiosz)
 		len = diob.d_maxiosz;
-	buf = memalign(diob.d_mem, len);
+	if (posix_memalign((void **)buf, diob.d_mem, len) != 0) {
The posix_memalign() actually returns pointer to the memory via buf, so instead of (void **)buf, you should write &buf (and that would silence the warning about uninitalized buf too).

Quoted 2 lines

sorry, my mistake. I'll correct it. Btw, compiler kept silent when I missed & sign..

That's because of the (void **) cast. The char * and void ** are not compatible pointers and copiler should print a warning about incompatible type. On the other hand void ** and char ** are incompatible pointers too.

As one wise man said: "C is a Spartan language" ...

Signature · 12 lines

Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Cyril Hrubis

2011-03-16 12:50 UTC #4 lore raw

In reply to a message not in the archive

Hi!

Quoted 12 lines

Quoted 11 lines
testcases/kernel/fs/fsstress/fsstress.c dread_f(int opno, long r) · line 1804
Quoted 8 lines
testcases/kernel/fs/fsstress/fsstress.c +3 -5
-	if (no_xfs) {
-		diob.d_miniosz = stb.st_blksize;
-		diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
-		diob.d_mem = stb.st_blksize;
-	}
+	diob.d_miniosz = stb.st_blksize;
+	diob.d_maxiosz = stb.st_blksize * 256;  /* good number ? */
+	diob.d_mem = stb.st_blksize;
Hmm, why you are removing the if (no_xfs) here (I don't say it's wrong, I only want to know why)?

Quoted 3 lines

The compiler gave warning that diob.d_* may not initialized. I don't know what the values diob.d_* should be if XFS exists (no_xfs == 0), but I guess they should be the same as when XFS not available.

Actually it's a little more complicated. This change breaks compilation without -DNO_XFS (that is defined as default in Makefile).

The whole point of NO_XFS and no_xfs is that the test may be compiled with and without xfs specialities as well as executed with and without xfs specialities.

Signature · 12 lines

Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
Colocation vs. Managed Hosting
A question and answer guide to determining the best fit
for your organization - today and in the future.
http://p.sf.net/sfu/internap-sfd2d
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list