Re: [PATCH 19/19] lockd: clean up 64-bit alignment fix in nsm_init_private()

["J. Bruce Fields" <bfields@fieldses.org>]

On Tue, Apr 28, 2009 at 06:11:06PM -0400, Chuck Lever wrote:
> This patch is simply a clean up.  We don't need to use put_unaligned in 
> nsm_init_private.  There is absolutely nothing special about the  
> nsm_private data type that would require this.  It should be accessed  
> and modified the way all other RPC opaques are, via memset/memcpy.
>
> Take a look at http://thread.gmane.org/gmane.linux.nfs/25607 and commit 
> ad5b365c.
>
> The controversy is over how to define opaques so they are accessible on 
> both 32- and 64-bit hardware platforms.  My first pass at  
> nsm_init_private worked on 32-bit systems, but broke on 64-bit systems.  
> An expedient fix for this was to add the put_unaligned in there so 64-bit 
> systems could access the field no matter how it was aligned.  I argue 
> this is unneeded complexity, and inconsistent with the way most other RPC 
> opaques are treated in the kernel.
>
> Andrew Morton proposed making RPC opaques a union of u8, u32 (or  
> __be32), and u64 -- the u8 would allow us to treat an opaque as a byte  
> array when needed, the u32 would allow access via XDR quads, and the u64 
> would force 64-bit alignment.  The issues with this are:
>
> 1.  Defined this way, opaque fields in data structures will force the  
> encompassing structures to be large enough to honor the alignment  
> requirements of the fields, and
>
> 2.  Most other RPC opaques are already defined as character arrays, so  
> we would have to visit all of them to see if there were issues.
>
> If we insist on accessing opaques only via memset() and memcpy() problem 
> 1 goes away and we remain compatible with the traditional definition of 
> an RPC opaque as an array of bytes, on both 64- and 32-bit systems.
>
> The description for this patch can be rewritten this way:
>
> "Clean up: There is nothing special about the nsm_private data type that 
> requires the use of put_unaligned() to access it.  Rewrite  
> nsm_init_private so it accesses nsm_private the way other code accesses 
> other RPC opaques.
>
> See kernel bugzilla 12995."

OK.--b.