On Apr 28, 2009, at 6:03 PM, Måns Rullgård wrote:
"J. Bruce Fields" <bfields@xxxxxxxxxxxx>
writes:
On Tue, Apr 28, 2009 at 01:24:24PM -0400, Chuck Lever wrote:
On Apr 28, 2009, at 12:40 PM, J. Bruce Fields wrote:
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.
It's not. Note the phrase above in the description: "permits us to
_continue_ to define these" -- meaning, I'm not changing the
structures.
Err, but that's not right either, is it?: We don't need to apply
this
patch in order to continue to define the structures as they're
currently
defined.
Help! I'm confused!
I too fail to see the reasoning with the memcpy(). It achieves
exactly the same end result using twice as much work.
Well, OK, but then you might want to review all the other places we
use memcpy() to access an RPC opaque field. There's no reason to
treat nsm_private any differently than, say, an NFS readdir cookie.
If there is, then it should be documented here.
Also, what if
SM_PRIV_SIZE is ever increased? Then it will be copying "random" data
from the stack into the nsm_private data. This strikes me as a little
fragile.
SM_PRIV_SIZE is a size defined by the NSM network protocol. It will
never change.
--
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
