On Tue, Apr 28, 2009 at 12:35:50PM -0400, Chuck Lever wrote: > On Apr 28, 2009, at 12:31 PM, J. Bruce Fields wrote: >> On Thu, Apr 23, 2009 at 07:33:40PM -0400, Chuck Lever wrote: >>> Recently, commit ad5b365c fixed a data type alignment issue in >>> nsm_init_private() by introducing put_unaligned(). We don't actually >>> _require_ an unaligned access to nsm_private here. >>> >>> Instead, we should always use memcpy/memcmp to access the contents of >>> RPC opaque data types. This permits us to continue to define these >>> as >>> simple character arrays, as most legacy RPC code does, and to rely on >>> gcc to pack the fields in in-core structures optimally without >>> breaking >>> on platforms that require strict alignment. >> >> OK, I'm confused. What structures will get packed differently? > > Any struct that has an nsm_private embedded in it, such as struct > nlm_reboot. I don't see how that or any structure is changed by this patch. --b. > >> --b. >> >>> >>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx> >>> --- >>> >>> fs/lockd/mon.c | 12 +++++------- >>> 1 files changed, 5 insertions(+), 7 deletions(-) >>> >>> diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c >>> index 5017d50..763509a 100644 >>> --- a/fs/lockd/mon.c >>> +++ b/fs/lockd/mon.c >>> @@ -16,8 +16,6 @@ >>> #include <linux/sunrpc/svc.h> >>> #include <linux/lockd/lockd.h> >>> >>> -#include <asm/unaligned.h> >>> - >>> #define NLMDBG_FACILITY NLMDBG_MONITOR >>> #define NSM_PROGRAM 100024 >>> #define NSM_VERSION 1 >>> @@ -278,14 +276,14 @@ static struct nsm_handle >>> *nsm_lookup_priv(const struct nsm_private *priv) >>> */ >>> static void nsm_init_private(struct nsm_handle *nsm) >>> { >>> - u64 *p = (u64 *)&nsm->sm_priv.data; >>> struct timespec ts; >>> - s64 ns; >>> + u64 buf[2]; >>> >>> ktime_get_ts(&ts); >>> - ns = timespec_to_ns(&ts); >>> - put_unaligned(ns, p); >>> - put_unaligned((unsigned long)nsm, p + 1); >>> + buf[0] = timespec_to_ns(&ts); >>> + buf[1] = (unsigned long)nsm; >>> + >>> + memcpy(nsm->sm_priv.data, buf, SM_PRIV_SIZE); >>> } >>> >>> static struct nsm_handle *nsm_create_handle(const struct sockaddr >>> *sap, >>> > > -- > Chuck Lever > chuck[dot]lever[at]oracle[dot]com -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html