| Message ID | 20171111172155.31941-3-zlang@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On 11/11/17 11:21 AM, Zorro Lang wrote: > do_preadv and do_pwritev all have a 'buffer_size' argument, but they > never used it. Instead of it, they use global 'buffersize' variable, > which is initialized in alloc_buffer(). As the 'buffer_size' is > useless, so remove it for clear code. > > Signed-off-by: Zorro Lang <zlang@redhat.com> I'm going to hold off on this one for 4.14; I want to think a bit about why we have this global in the first place, and if it makes more sense to eliminate the global and pass the buffer size around properly. Thanks, -Eric > --- > io/pread.c | 7 +++---- > io/pwrite.c | 5 ++--- > 2 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/io/pread.c b/io/pread.c > index 60650aa3..98e992b0 100644 > --- a/io/pread.c > +++ b/io/pread.c > @@ -176,8 +176,7 @@ static ssize_t > do_preadv( > int fd, > off64_t offset, > - size_t count, > - size_t buffer_size) > + size_t count) > { > int vecs = 0; > ssize_t oldlen = 0; > @@ -205,7 +204,7 @@ do_preadv( > return bytes; > } > #else > -#define do_preadv(fd, offset, count, buffer_size) (0) > +#define do_preadv(fd, offset, count) (0) > #endif > > static ssize_t > @@ -218,7 +217,7 @@ do_pread( > if (!vectors) > return pread(fd, buffer, min(count, buffer_size), offset); > > - return do_preadv(fd, offset, count, buffer_size); > + return do_preadv(fd, offset, count); > } > > static int > diff --git a/io/pwrite.c b/io/pwrite.c > index a89edfd0..f75c6164 100644 > --- a/io/pwrite.c > +++ b/io/pwrite.c > @@ -66,7 +66,6 @@ do_pwritev( > int fd, > off64_t offset, > size_t count, > - size_t buffer_size, > int pwritev2_flags) > { > int vecs = 0; > @@ -102,7 +101,7 @@ do_pwritev( > return bytes; > } > #else > -#define do_pwritev(fd, offset, count, buffer_size, pwritev2_flags) (0) > +#define do_pwritev(fd, offset, count, pwritev2_flags) (0) > #endif > > static ssize_t > @@ -116,7 +115,7 @@ do_pwrite( > if (!vectors) > return pwrite(fd, buffer, min(count, buffer_size), offset); > > - return do_pwritev(fd, offset, count, buffer_size, pwritev2_flags); > + return do_pwritev(fd, offset, count, pwritev2_flags); > } > > static int > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/11/17 11:21 AM, Zorro Lang wrote: > do_preadv and do_pwritev all have a 'buffer_size' argument, but they > never used it. Instead of it, they use global 'buffersize' variable, > which is initialized in alloc_buffer(). As the 'buffer_size' is > useless, so remove it for clear code. > > Signed-off-by: Zorro Lang <zlang@redhat.com> Hi Zorro, going through old patches and remembered that I never came back to this one, sorry. I think that just removing it is ok. do_preadv & do_pwritev both use iov and buffersize, each is a global variable; there is no need to pass in buffer size any more than there is a need to pass in the iov itself. I'll go ahead & merge this as is. Reviewed-by: Eric Sandeen <sandeen@redhat.com> Thanks, -Eric > --- > io/pread.c | 7 +++---- > io/pwrite.c | 5 ++--- > 2 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/io/pread.c b/io/pread.c > index 60650aa3..98e992b0 100644 > --- a/io/pread.c > +++ b/io/pread.c > @@ -176,8 +176,7 @@ static ssize_t > do_preadv( > int fd, > off64_t offset, > - size_t count, > - size_t buffer_size) > + size_t count) > { > int vecs = 0; > ssize_t oldlen = 0; > @@ -205,7 +204,7 @@ do_preadv( > return bytes; > } > #else > -#define do_preadv(fd, offset, count, buffer_size) (0) > +#define do_preadv(fd, offset, count) (0) > #endif > > static ssize_t > @@ -218,7 +217,7 @@ do_pread( > if (!vectors) > return pread(fd, buffer, min(count, buffer_size), offset); > > - return do_preadv(fd, offset, count, buffer_size); > + return do_preadv(fd, offset, count); > } > > static int > diff --git a/io/pwrite.c b/io/pwrite.c > index a89edfd0..f75c6164 100644 > --- a/io/pwrite.c > +++ b/io/pwrite.c > @@ -66,7 +66,6 @@ do_pwritev( > int fd, > off64_t offset, > size_t count, > - size_t buffer_size, > int pwritev2_flags) > { > int vecs = 0; > @@ -102,7 +101,7 @@ do_pwritev( > return bytes; > } > #else > -#define do_pwritev(fd, offset, count, buffer_size, pwritev2_flags) (0) > +#define do_pwritev(fd, offset, count, pwritev2_flags) (0) > #endif > > static ssize_t > @@ -116,7 +115,7 @@ do_pwrite( > if (!vectors) > return pwrite(fd, buffer, min(count, buffer_size), offset); > > - return do_pwritev(fd, offset, count, buffer_size, pwritev2_flags); > + return do_pwritev(fd, offset, count, pwritev2_flags); > } > > static int >
On Thu, Sep 27, 2018 at 01:38:30PM -0500, Eric Sandeen wrote: > On 11/11/17 11:21 AM, Zorro Lang wrote: > > do_preadv and do_pwritev all have a 'buffer_size' argument, but they > > never used it. Instead of it, they use global 'buffersize' variable, > > which is initialized in alloc_buffer(). As the 'buffer_size' is > > useless, so remove it for clear code. > > > > Signed-off-by: Zorro Lang <zlang@redhat.com> > > Hi Zorro, going through old patches and remembered that I never came > back to this one, sorry. > > I think that just removing it is ok. do_preadv & do_pwritev > both use iov and buffersize, each is a global variable; there is no > need to pass in buffer size any more than there is a need to > pass in the iov itself. I'll go ahead & merge this as is. Hi Eric, Thanks so much, I already forgot this patch, I don't know if it can be merged directly now, hope there's not conflict :-P Feel free to tell me if you need a new version. Thanks, Zorro > > Reviewed-by: Eric Sandeen <sandeen@redhat.com> > > Thanks, > -Eric > > > --- > > io/pread.c | 7 +++---- > > io/pwrite.c | 5 ++--- > > 2 files changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/io/pread.c b/io/pread.c > > index 60650aa3..98e992b0 100644 > > --- a/io/pread.c > > +++ b/io/pread.c > > @@ -176,8 +176,7 @@ static ssize_t > > do_preadv( > > int fd, > > off64_t offset, > > - size_t count, > > - size_t buffer_size) > > + size_t count) > > { > > int vecs = 0; > > ssize_t oldlen = 0; > > @@ -205,7 +204,7 @@ do_preadv( > > return bytes; > > } > > #else > > -#define do_preadv(fd, offset, count, buffer_size) (0) > > +#define do_preadv(fd, offset, count) (0) > > #endif > > > > static ssize_t > > @@ -218,7 +217,7 @@ do_pread( > > if (!vectors) > > return pread(fd, buffer, min(count, buffer_size), offset); > > > > - return do_preadv(fd, offset, count, buffer_size); > > + return do_preadv(fd, offset, count); > > } > > > > static int > > diff --git a/io/pwrite.c b/io/pwrite.c > > index a89edfd0..f75c6164 100644 > > --- a/io/pwrite.c > > +++ b/io/pwrite.c > > @@ -66,7 +66,6 @@ do_pwritev( > > int fd, > > off64_t offset, > > size_t count, > > - size_t buffer_size, > > int pwritev2_flags) > > { > > int vecs = 0; > > @@ -102,7 +101,7 @@ do_pwritev( > > return bytes; > > } > > #else > > -#define do_pwritev(fd, offset, count, buffer_size, pwritev2_flags) (0) > > +#define do_pwritev(fd, offset, count, pwritev2_flags) (0) > > #endif > > > > static ssize_t > > @@ -116,7 +115,7 @@ do_pwrite( > > if (!vectors) > > return pwrite(fd, buffer, min(count, buffer_size), offset); > > > > - return do_pwritev(fd, offset, count, buffer_size, pwritev2_flags); > > + return do_pwritev(fd, offset, count, pwritev2_flags); > > } > > > > static int > >
diff --git a/io/pread.c b/io/pread.c index 60650aa3..98e992b0 100644 --- a/io/pread.c +++ b/io/pread.c @@ -176,8 +176,7 @@ static ssize_t do_preadv( int fd, off64_t offset, - size_t count, - size_t buffer_size) + size_t count) { int vecs = 0; ssize_t oldlen = 0; @@ -205,7 +204,7 @@ do_preadv( return bytes; } #else -#define do_preadv(fd, offset, count, buffer_size) (0) +#define do_preadv(fd, offset, count) (0) #endif static ssize_t @@ -218,7 +217,7 @@ do_pread( if (!vectors) return pread(fd, buffer, min(count, buffer_size), offset); - return do_preadv(fd, offset, count, buffer_size); + return do_preadv(fd, offset, count); } static int diff --git a/io/pwrite.c b/io/pwrite.c index a89edfd0..f75c6164 100644 --- a/io/pwrite.c +++ b/io/pwrite.c @@ -66,7 +66,6 @@ do_pwritev( int fd, off64_t offset, size_t count, - size_t buffer_size, int pwritev2_flags) { int vecs = 0; @@ -102,7 +101,7 @@ do_pwritev( return bytes; } #else -#define do_pwritev(fd, offset, count, buffer_size, pwritev2_flags) (0) +#define do_pwritev(fd, offset, count, pwritev2_flags) (0) #endif static ssize_t @@ -116,7 +115,7 @@ do_pwrite( if (!vectors) return pwrite(fd, buffer, min(count, buffer_size), offset); - return do_pwritev(fd, offset, count, buffer_size, pwritev2_flags); + return do_pwritev(fd, offset, count, pwritev2_flags); } static int
do_preadv and do_pwritev all have a 'buffer_size' argument, but they never used it. Instead of it, they use global 'buffersize' variable, which is initialized in alloc_buffer(). As the 'buffer_size' is useless, so remove it for clear code. Signed-off-by: Zorro Lang <zlang@redhat.com> --- io/pread.c | 7 +++---- io/pwrite.c | 5 ++--- 2 files changed, 5 insertions(+), 7 deletions(-)