From: Chuck Lever Subject: Re: [PATCH 19/19] lockd: clean up 64-bit alignment fix in nsm_init_private() Date: Tue, 28 Apr 2009 12:35:50 -0400 Message-ID: <1AF02847-8D16-4387-81CB-F36969CA9883@oracle.com> References: <20090423231550.17283.24432.stgit@ingres.1015granger.net> <20090423233340.17283.29580.stgit@ingres.1015granger.net> <20090428163101.GK17891@fieldses.org> Mime-Version: 1.0 (Apple Message framework v930.3) Content-Type: text/plain; charset=US-ASCII; format=flowed; delsp=yes Cc: linux-nfs@vger.kernel.org, Mans Rullgard To: "J. Bruce Fields" Return-path: Received: from rcsinet12.oracle.com ([148.87.113.124]:27382 "EHLO rgminet12.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753595AbZD1QgO (ORCPT ); Tue, 28 Apr 2009 12:36:14 -0400 In-Reply-To: <20090428163101.GK17891@fieldses.org> Sender: linux-nfs-owner@vger.kernel.org List-ID: 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. > --b. > >> >> Signed-off-by: Chuck Lever >> --- >> >> 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 >> #include >> >> -#include >> - >> #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