| Message ID | 201108101803.p7AI36eV008484@farm-0023.internal.tilera.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/10/2011 10:56 AM, Chris Metcalf wrote: > 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. > > Signed-off-by: Chris Metcalf <cmetcalf@tilera.com> > --- > fs/nfs/blocklayout/blocklayout.c | 1 + > fs/nfs/nfs4filelayout.c | 6 +++--- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c > index e56564d..9561c8f 100644 > --- a/fs/nfs/blocklayout/blocklayout.c > +++ b/fs/nfs/blocklayout/blocklayout.c > @@ -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" > > diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c > index e8915d4..6976a72 100644 > --- a/fs/nfs/nfs4filelayout.c > +++ b/fs/nfs/nfs4filelayout.c > @@ -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); 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 > } > > /* This function is used by the layout driver to calculate the -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8/10/2011 6:02 PM, Boaz Harrosh wrote: > On 08/10/2011 10:56 AM, Chris Metcalf wrote: >> 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. >> >> [...] >> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c >> index e8915d4..6976a72 100644 >> --- a/fs/nfs/nfs4filelayout.c >> +++ b/fs/nfs/nfs4filelayout.c >> @@ -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); > 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) 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 :-)
On Thu, Aug 11, 2011 at 1:56 AM, Chris Metcalf <cmetcalf@tilera.com> wrote: > 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. > > Signed-off-by: Chris Metcalf <cmetcalf@tilera.com> > --- > fs/nfs/blocklayout/blocklayout.c | 1 + > fs/nfs/nfs4filelayout.c | 6 +++--- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c > index e56564d..9561c8f 100644 > --- a/fs/nfs/blocklayout/blocklayout.c > +++ b/fs/nfs/blocklayout/blocklayout.c > @@ -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. > > #include "blocklayout.h" > > diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c > index e8915d4..6976a72 100644 > --- a/fs/nfs/nfs4filelayout.c > +++ b/fs/nfs/nfs4filelayout.c > @@ -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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On 08/11/2011 06:26 AM, Chris Metcalf wrote: > > 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 :-) > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c index e56564d..9561c8f 100644 --- a/fs/nfs/blocklayout/blocklayout.c +++ b/fs/nfs/blocklayout/blocklayout.c @@ -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" diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c index e8915d4..6976a72 100644 --- a/fs/nfs/nfs4filelayout.c +++ b/fs/nfs/nfs4filelayout.c @@ -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
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. Signed-off-by: Chris Metcalf <cmetcalf@tilera.com> --- fs/nfs/blocklayout/blocklayout.c | 1 + fs/nfs/nfs4filelayout.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-)