nfs: fix a couple of minor portability issues
Subsystems (2)
PATCH 1/1 nfs: fix a couple of minor portability issues
Building on tilepro revealed two minor portability issues: the blocklayout.c file used prefetchw() without #include <linux/prefetch.h>, and the nfs4filelayout.c file used do_div() on an s64 not a u64. This change fixes those two issues so the NFS code builds on tilepro.
--- fs/nfs/blocklayout/blocklayout.c | 1 + fs/nfs/nfs4filelayout.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-)
fs/nfs/blocklayout/blocklayout.c +1 -0
@@ -36,6 +36,7 @@#include <linux/namei.h> #include <linux/bio.h> /* struct bio */ #include <linux/buffer_head.h> /* various write calls */ +#include <linux/prefetch.h> #include "blocklayout.h"
fs/nfs/nfs4filelayout.c +3 -3
@@ -48,13 +48,13 @@ filelayout_get_dense_offset(struct nfs4_filelayout_segment *flseg, loff_t offset) {u32 stripe_width = flseg->stripe_unit * flseg->dsaddr->stripe_count; -u64 tmp; +u64 tmp, uoff; offset -= flseg->pattern_offset; - tmp = offset; + tmp = uoff = offset;do_div (tmp, stripe_width); -return tmp * flseg->stripe_unit +do_div (offset, flseg->stripe_unit); +return tmp * flseg->stripe_unit +do_div (uoff, flseg->stripe_unit); }/* This function is used by the layout driver to calculate the
Signature · 1 line
1.6.5.2
Quoted 31 lines · Chris Metcalf
Building on tilepro revealed two minor portability issues: the
blocklayout.cfile usedprefetchw()without #include <linux/prefetch.h>, and thenfs4filelayout.cfile useddo_div()on an s64 not a u64. This change fixes those two issues so the NFS code builds on tilepro.Signed-off-by Chris Metcalf <[email protected]>--- fs/nfs/blocklayout/blocklayout.c | 1 + fs/nfs/nfs4filelayout.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-)fs/nfs/blocklayout/blocklayout.c +1
-0@@ -36,6 +36,7 @@#include <linux/namei.h> #include <linux/bio.h> /* struct bio */ #include <linux/buffer_head.h> /* various write calls */ +#include <linux/prefetch.h> #include "blocklayout.h" fs/nfs/nfs4filelayout.c +0
-0@@ -48,13 +48,13 @@ filelayout_get_dense_offset(struct nfs4_filelayout_segment *flseg, loff_t offset) {u32 stripe_width = flseg->stripe_unit * flseg->dsaddr->stripe_count;
fs/nfs/nfs4filelayout.c filelayout_get_dense_offset(struct nfs4_filelayout_segment *flseg, · line 57
Quoted 10 lines
fs/nfs/nfs4filelayout.c +3
-3-u64 tmp; +u64 tmp, uoff; offset -= flseg->pattern_offset; - tmp = offset; + tmp = uoff = offset;do_div (tmp, stripe_width); -return tmp * flseg->stripe_unit +do_div (offset, flseg->stripe_unit); +return tmp * flseg->stripe_unit +do_div (uoff, flseg->stripe_unit);
For proper u64 divisions it is best to use the div_u64 (and div64_u64) and not use do_div. (And please don't add an unnecessary variable, just use a cast)
Thanks
Boaz
Quoted 3 lines
fs/nfs/nfs4filelayout.c +0
-0}/* This function is used by the layout driver to calculate the
Quoted 23 lines · Boaz Harrosh
Quoted 22 lines · Chris Metcalf
Building on tilepro revealed two minor portability issues: the
blocklayout.cfile usedprefetchw()without #include <linux/prefetch.h>, and thenfs4filelayout.cfile useddo_div()on an s64 not a u64. This change fixes those two issues so the NFS code builds on tilepro.[...]
fs/nfs/nfs4filelayout.c +2
-2@@ -48,13 +48,13 @@ filelayout_get_dense_offset(struct nfs4_filelayout_segment *flseg, loff_t offset) {u32 stripe_width = flseg->stripe_unit * flseg->dsaddr->stripe_count; -u64 tmp; +u64 tmp, uoff; offset -= flseg->pattern_offset; - tmp = offset; + tmp = uoff = offset;do_div (tmp, stripe_width);
Quoted 4 lines
fs/nfs/nfs4filelayout.c filelayout_get_dense_offset(struct nfs4_filelayout_segment *flseg, · line 57
Quoted 2 lines
fs/nfs/nfs4filelayout.c +1
-1-return tmp * flseg->stripe_unit +do_div (offset, flseg->stripe_unit); +return tmp * flseg->stripe_unit +do_div (uoff, flseg->stripe_unit);For proper u64 divisions it is best to use the
div_u64(anddiv64_u64) and not usedo_div. (And please don't add an unnecessary variable, just use a cast)
You can't use a cast with do_div() or you get errors about non-lvalues.
And the context here is that we want the remainder, not the divided result, so we'd need a temporary variable anyway if we were going to use a routine from math64.h, presumably div_u64_rem(). But that's just an inline wrapper around do_div() anyway, so it's no more efficient, and not obviously any less "cluttered" in the source code here. The original code author (who I'm adding belatedly to this email) may have a stronger opinion.
I'm not the author or maintainer of this code, I just want it to compile without warning on 32-bit architectures :-)
Signature · 2 lines
Chris Metcalf, Tilera Corp. http://www.tilera.com
Quoted 10 lines · Chris Metcalf
You can't use a cast with
do_div()or you get errors about non-lvalues.And the context here is that we want the remainder, not the divided result, so we'd need a temporary variable anyway if we were going to use a routine from
math64.h, presumablydiv_u64_rem(). But that's just an inline wrapper arounddo_div()anyway, so it's no more efficient, and not obviously any less "cluttered" in the source code here. The original code author (who I'm adding belatedly to this email) may have a stronger opinion.
Quoted 3 lines
I'm not the author or maintainer of this code, I just want it to compile without warning on 32-bit architectures :-)
Still, if you fix it do it properly by using, as you said, div_u64_rem(). It's what needs to be used with u64 types.
Thanks
Boaz
PATCH v2 1/1 nfs: fix a minor do_div portability issue
This change modifies filelayout_get_dense_offset() to use the functions in math64.h and thus avoid a 32-bit platform compile error trying to use do_div() on an s64 type.
--- fs/nfs/nfs4filelayout.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
fs/nfs/nfs4filelayout.c +4 -3
@@ -49,12 +49,13 @@ filelayout_get_dense_offset(struct nfs4_filelayout_segment *flseg, {u32 stripe_width = flseg->stripe_unit * flseg->dsaddr->stripe_count;u64 tmp; +u32 rem; offset -= flseg->pattern_offset; - tmp = offset; -do_div (tmp, stripe_width); + tmp =div_u64 (offset, stripe_width); +div_u64_rem (offset, flseg->stripe_unit, &rem); -return tmp * flseg->stripe_unit +do_div (offset, flseg->stripe_unit); +return tmp * flseg->stripe_unit + rem; }/* This function is used by the layout driver to calculate the
Signature · 1 line
1.6.5.2
PATCH v3 1/1 nfs: fix a minor do_div portability issue
Quoted 6 lines · Chris Metcalf
This change modifies
filelayout_get_dense_offset()to use the functions inmath64.hand thus avoid a 32-bit platform compile error trying to usedo_div()on an s64 type.Signed-off-by Chris Metcalf <[email protected]>Cc Boaz Harrosh <[email protected]>
Thanks
Just if you don't mind a name change. tmp => stripe_no while at it.
---
From: Chris Metcalf <[email protected]>
Subject: [PATCH v3] nfs: fix a minor do_div portability issue
This change modifies filelayout_get_dense_offset() to use the functions in math64.h and thus avoid a 32-bit platform compile error trying to use do_div() on an s64 type.
--- fs/nfs/nfs4filelayout.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
fs/nfs/nfs4filelayout.c +4 -3
@@ -49,12 +49,13 @@ filelayout_get_dense_offset(struct nfs4_filelayout_segment *flseg, {u32 stripe_width = flseg->stripe_unit * flseg->dsaddr->stripe_count;u64 stripe_no; +u32 rem; offset -= flseg->pattern_offset; - tmp = offset; -do_div (tmp, stripe_width); + stripe_no =div_u64 (offset, stripe_width); +div_u64_rem (offset, flseg->stripe_unit, &rem); -return tmp * flseg->stripe_unit +do_div (offset, flseg->stripe_unit); +return stripe_no * flseg->stripe_unit + rem; }/* This function is used by the layout driver to calculate the
-- 1.6.5.2
Quoted 16 lines · Chris Metcalf
Building on tilepro revealed two minor portability issues: the
blocklayout.cfile usedprefetchw()without #include <linux/prefetch.h>, and thenfs4filelayout.cfile useddo_div()on an s64 not a u64. This change fixes those two issues so the NFS code builds on tilepro.Signed-off-by Chris Metcalf <[email protected]>---
fs/nfs/blocklayout/blocklayout.c | 1 +
fs/nfs/nfs4filelayout.c | 6 +++---2 files changed, 4 insertions(+), 3 deletions(-)fs/nfs/blocklayout/blocklayout.c +0
-0
fs/nfs/blocklayout/blocklayout.c line 39
Quoted 4 lines
fs/nfs/blocklayout/blocklayout.c +1
-0@@ -36,6 +36,7 @@ #include <linux/namei.h> #include <linux/bio.h> /* struct bio */ #include <linux/buffer_head.h> /* various write calls */ +#include <linux/prefetch.h>
This is already fixed in Trond's nfs-for-next branch by commit 88c9e4219.
Quoted 32 lines
fs/nfs/blocklayout/blocklayout.c +0
-0#include "blocklayout.h"fs/nfs/nfs4filelayout.c +3
-4@@ -48,13 +48,13 @@ filelayout_get_dense_offset(struct nfs4_filelayout_segment *flseg, loff_t offset) { u32 stripe_width = flseg->stripe_unit * flseg->dsaddr->stripe_count; - u64 tmp; + u64 tmp, uoff; offset -= flseg->pattern_offset; - tmp = offset; + tmp = uoff = offset; do_div(tmp, stripe_width); - return tmp * flseg->stripe_unit + do_div(offset, flseg->stripe_unit); + return tmp * flseg->stripe_unit + do_div(uoff, flseg->stripe_unit); } /* This function is used by the layout driver to calculate the -- 1.6.5.2
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
Thanks, -Bergwolf