[PATCH] xfsprog: remove duplicate vector memalign from xfs_io

[PATCH] xfsprog: remove duplicate vector memalign from xfs_io

[Mark Tinguely]

[-- Attachment #1: xfsprogs-io_remove_duplicate_vector_memalign.patch --]
[-- Type: text/plain, Size: 774 bytes --]

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 <tinguely@sgi.com>
---
 io/pread.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: b/io/pread.c
===================================================================
--- a/io/pread.c
+++ 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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfsprog: remove duplicate vector memalign from xfs_io

[Ben Myers]

On Tue, Sep 25, 2012 at 09:24:15AM -0500, Mark Tinguely wrote:
> 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
===================================================================
--- xfsprogs.orig/io/pread.c
+++ 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 <bpm@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfsprog: remove duplicate vector memalign from xfs_io

[Mark Tinguely]

On 09/26/12 17:04, Ben Myers wrote:
> On Tue, Sep 25, 2012 at 09:24:15AM -0500, Mark Tinguely wrote:
>> 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
> ===================================================================
> --- xfsprogs.orig/io/pread.c
> +++ 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<bpm@sgi.com>

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

--Mark.



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfsprog: remove duplicate vector memalign from xfs_io

[Dave Chinner]

On Tue, Sep 25, 2012 at 09:24:15AM -0500, Mark Tinguely wrote:
> 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 <tinguely@sgi.com>
> ---
>  io/pread.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> Index: b/io/pread.c
> ===================================================================
> --- a/io/pread.c
> +++ 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;
>  		}

Classic copy'n'paste bug. My fault.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfsprog: remove duplicate vector memalign from xfs_io

[Dave Chinner]

On Wed, Sep 26, 2012 at 05:04:34PM -0500, Ben Myers wrote:
> On Tue, Sep 25, 2012 at 09:24:15AM -0500, Mark Tinguely wrote:
> > 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
> ===================================================================
> --- xfsprogs.orig/io/pread.c
> +++ 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);

Can you post that as a proper patch?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH v2] xfsprog: remove duplicate vector memalign from xfs_io

[Mark Tinguely]

[-- Attachment #1: v2-xfsprogs-io_remove_duplicate_vector_memalign.patch --]
[-- Type: text/plain, Size: 1153 bytes --]

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 <tinguely@sgi.com>
Reviewed-by: Ben Myers <bpm@sgi.com>
---
 io/pread.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Index: b/io/pread.c
===================================================================
--- a/io/pread.c
+++ 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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfsprog: remove duplicate vector memalign from xfs_io

[Ben Myers]

Hey Dave,

On Thu, Sep 27, 2012 at 10:01:01AM +1000, Dave Chinner wrote:
> On Wed, Sep 26, 2012 at 05:04:34PM -0500, Ben Myers wrote:
> > On Tue, Sep 25, 2012 at 09:24:15AM -0500, Mark Tinguely wrote:
> > > 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
> > ===================================================================
> > --- xfsprogs.orig/io/pread.c
> > +++ 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);
> 
> Can you post that as a proper patch?

Looks like Mark posted it in v2.  ;)

-Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v2] xfsprog: remove duplicate vector memalign from xfs_io

[Ben Myers]

On Thu, Sep 27, 2012 at 07:47:36AM -0500, Mark Tinguely wrote:
> 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 <tinguely@sgi.com>
> Reviewed-by: Ben Myers <bpm@sgi.com>

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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v2] xfsprog: remove duplicate vector memalign from xfs_io

[Dave Chinner]

On Thu, Sep 27, 2012 at 07:47:36AM -0500, Mark Tinguely wrote:
> 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 <tinguely@sgi.com>
> Reviewed-by: Ben Myers <bpm@sgi.com>

Looks good, Thanks for fixing this.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH v2] xfsprog: remove duplicate vector memalign from xfs_io

[Mark Tinguely]

On 09/27/12 17:52, Dave Chinner wrote:
> On Thu, Sep 27, 2012 at 07:47:36AM -0500, Mark Tinguely wrote:
>> 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<tinguely@sgi.com>
>> Reviewed-by: Ben Myers<bpm@sgi.com>
>
> Looks good, Thanks for fixing this.
>
> Reviewed-by: Dave Chinner<dchinner@redhat.com>
>


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

--Mark.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs