Skip to contentSkip to conversation

xfsprog: remove duplicate vector memalign from xfs_io

PATCH Superseded Mark Tinguely 3 participants 10 messages Sep 25, 2012 – Sep 28, 2012 1 message not in the archive

lore
Share & tools
lore

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

Mark Tinguely

author
#1 lore raw

PATCH 1/1 xfsprog: remove duplicate vector memalign from xfs_io

The vector feature of xfs_io uses its own memory buffer in the iov structure and does not use the buffer entry. Remove the duplicate memalign.

Signed-off-by Mark Tinguely <[email protected]>
---
 io/pread.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
io/pread.c +1 -2
Index: b/io/pread.c
===================================================================
@@ -76,8 +76,7 @@ alloc_iovec(
 	buffersize = 0;
 	for (i = 0; i < vectors; i++) {
 		iov[i].iov_base = memalign(pagesize, bsize);
-		buffer = memalign(pagesize, bsize);
-		if (!buffer) {
+		if (!iov[i].iov_base) {
 			perror("memalign");
 			goto unwind;
 		}

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Reviewed-by on v1 1/1
#2 lore raw
Quoted 2 lines · Mark Tinguely

The vector feature of xfs_io uses its own memory buffer in the iov structure and does not use the buffer entry. Remove the duplicate memalign.

Looks to me that there is a usage of buffer in __dump_buffer. However, I suspect there shouldn't be. I suggest (untested,ymmv,ianal,etc):

:!quilt di

io/pread.c +1 -1
Index: xfsprogs/io/pread.c
===================================================================
@@ -130,7 +130,7 @@ __dump_buffer(
        int             i, j;
        char            *p;
 
-       for (i = 0, p = (char *)buffer; i < len; i += 16) {
+       for (i = 0, p = (char *)buf; i < len; i += 16) {
                char    *s = p;
 
                printf("%08llx:  ", (unsigned long long)offset + i);

Otherwise your patch looks good to me. If I'm not off-my-rocker with the above you're welcome to pull it in to your patch, or I can submit it to the list more formally tomorrow.

Reviewed-by Ben Myers <[email protected]>

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#3 lore raw
Quoted 23 lines · Ben Myers
Quoted 2 lines · Mark Tinguely

The vector feature of xfs_io uses its own memory buffer in the iov structure and does not use the buffer entry. Remove the duplicate memalign.

Looks to me that there is a usage of buffer in __dump_buffer. However, I suspect there shouldn't be. I suggest (untested,ymmv,ianal,etc):

:!quilt di
Index: xfsprogs/io/pread.c
===================================================================

io/pread.c +1 -1
@@ -130,7 +130,7 @@ __dump_buffer(
         int             i, j;
         char            *p;
 
-       for (i = 0, p = (char *)buffer; i<  len; i += 16) {
+       for (i = 0, p = (char *)buf; i<  len; i += 16) {
                 char    *s = p;
 
                 printf("%08llx:  ", (unsigned long long)offset + i);
Quoted 5 lines

Otherwise your patch looks good to me. If I'm not off-my-rocker with the above you're welcome to pull it in to your patch, or I can submit it to the list more formally tomorrow.

Reviewed-by Ben Myers<[email protected]>

Interesting. You are on your rocker, I missed the second bug.

--Mark.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#4 lore raw
In reply to Ben Myers
Quoted 13 lines · Ben Myers
Quoted 2 lines · Mark Tinguely

The vector feature of xfs_io uses its own memory buffer in the iov structure and does not use the buffer entry. Remove the duplicate memalign.

Looks to me that there is a usage of buffer in __dump_buffer. However, I suspect there shouldn't be. I suggest (untested,ymmv,ianal,etc):

:!quilt di
Index: xfsprogs/io/pread.c
===================================================================

io/pread.c __dump_buffer( · line 136

Quoted 8 lines
io/pread.c +1 -1
@@ -130,7 +130,7 @@ __dump_buffer(
        int             i, j;
        char            *p;
 
-       for (i = 0, p = (char *)buffer; i < len; i += 16) {
+       for (i = 0, p = (char *)buf; i < len; i += 16) {
                char    *s = p;
 
                printf("%08llx:  ", (unsigned long long)offset + i);

Can you post that as a proper patch?

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#5 lore raw

Hey Dave,

Quoted 17 lines · Dave Chinner
Quoted 16 lines · Ben Myers
Quoted 2 lines · Mark Tinguely

The vector feature of xfs_io uses its own memory buffer in the iov structure and does not use the buffer entry. Remove the duplicate memalign.

Looks to me that there is a usage of buffer in __dump_buffer. However, I suspect there shouldn't be. I suggest (untested,ymmv,ianal,etc):

:!quilt di
Index: xfsprogs/io/pread.c
===================================================================

io/pread.c +0 -0
@@ -130,7 +130,7 @@ __dump_buffer(
        int             i, j;
        char            *p;
Quoted 7 lines

io/pread.c __dump_buffer( · line 136

Quoted 5 lines
io/pread.c +1 -1
-       for (i = 0, p = (char *)buffer; i < len; i += 16) {
+       for (i = 0, p = (char *)buf; i < len; i += 16) {
                char    *s = p;
 
                printf("%08llx:  ", (unsigned long long)offset + i);

Can you post that as a proper patch?

Looks like Mark posted it in v2. ;)

-Ben

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

author
#6 lore raw

PATCH v2 1/1 xfsprog: remove duplicate vector memalign from xfs_io

In reply to Ben Myers

The vector feature of xfs_io uses its own memory buffer in the iov structure and does not use the buffer entry. Remove the duplicate memalign.

v2: removed global buffer in __dump_buffer() and use passed buf pointer per
Ben's detection.

Signed-off-by Mark Tinguely <[email protected]>
Reviewed-by Ben Myers <[email protected]>
---
 io/pread.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
io/pread.c +2 -3
Index: b/io/pread.c
===================================================================
@@ -76,8 +76,7 @@ alloc_iovec(
 	buffersize = 0;
 	for (i = 0; i < vectors; i++) {
 		iov[i].iov_base = memalign(pagesize, bsize);
-		buffer = memalign(pagesize, bsize);
-		if (!buffer) {
+		if (!iov[i].iov_base) {
 			perror("memalign");
 			goto unwind;
 		}

@@ -130,7 +129,7 @@ __dump_buffer(
 	int		i, j;
 	char		*p;
 
-	for (i = 0, p = (char *)buffer; i < len; i += 16) {
+	for (i = 0, p = (char *)buf; i < len; i += 16) {
 		char	*s = p;
 
 		printf("%08llx:  ", (unsigned long long)offset + i);

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#7 lore raw
Quoted 8 lines · Mark Tinguely

The vector feature of xfs_io uses its own memory buffer in the iov structure and does not use the buffer entry. Remove the duplicate memalign.

v2: removed global buffer in __dump_buffer() and use passed buf pointer per
Ben's detection.

Signed-off-by Mark Tinguely <[email protected]>
Reviewed-by Ben Myers <[email protected]>

Still looks good. Don't forget to update the subject to xfsprogs. Also the line width looks like it might wrap.

-Ben

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Reviewed-by on v2 1/1
#8 lore raw
In reply to Mark Tinguely
Quoted 8 lines · Mark Tinguely

The vector feature of xfs_io uses its own memory buffer in the iov structure and does not use the buffer entry. Remove the duplicate memalign.

v2: removed global buffer in __dump_buffer() and use passed buf pointer per
Ben's detection.

Signed-off-by Mark Tinguely <[email protected]>
Reviewed-by Ben Myers <[email protected]>

Looks good, Thanks for fixing this.

Reviewed-by Dave Chinner <[email protected]>
Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
author
#9 lore raw
Quoted 7 lines · Dave Chinner
Quoted 6 lines · Mark Tinguely

The vector feature of xfs_io uses its own memory buffer in the iov structure and does not use the buffer entry. Remove the duplicate memalign.

v2: removed global buffer in __dump_buffer() and use passed buf pointer per
Ben's detection.

Quoted 7 lines
Quoted 2 lines
Signed-off-by Mark Tinguely<[email protected]>
Reviewed-by Ben Myers<[email protected]>

Looks good, Thanks for fixing this.

Reviewed-by Dave Chinner<[email protected]>

Committed to git://oss.sgi.com/xfs/cmds/xfsprogs.git, master branch.

--Mark.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Reviewed-by on v1 1/1
#10 lore raw
In reply to Mark Tinguely
Quoted 13 lines · Mark Tinguely

The vector feature of xfs_io uses its own memory buffer in the iov structure and does not use the buffer entry. Remove the duplicate memalign.

Signed-off-by Mark Tinguely <[email protected]>
---
 io/pread.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: b/io/pread.c
===================================================================

io/pread.c alloc_iovec( · line 82

Quoted 9 lines
io/pread.c +1 -2
@@ -76,8 +76,7 @@ alloc_iovec(
 	buffersize = 0;
 	for (i = 0; i < vectors; i++) {
 		iov[i].iov_base = memalign(pagesize, bsize);
-		buffer = memalign(pagesize, bsize);
-		if (!buffer) {
+		if (!iov[i].iov_base) {
 			perror("memalign");
 			goto unwind;
 		}

Classic copy'n'paste bug. My fault.

Reviewed-by Dave Chinner <[email protected]>
Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs