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! > It has been suggested that we use a union or __aligned attribute for RPC > opaques. The problem with that is that it would cause structs like > nlm_reboot to balloon in size; char x[] is aligned on bytes, but a union > of u8, u32, and u64 would be aligned on a double-word boundary. That > would make such structures larger. OK, I agree, so let's not do that. --b. > Legacy RPC code, and any code generated by rpcgen, generally defines an > opaque as a character array. So again, using a union would be > inconsistent with other uses of RPC opaques. > > The point is we want to define and access RPC opaque's consistently, > using memset() and memcpy(), since these are nothing more than byte > arrays. The code in nsm_init_private() was an exception to this, for no > reason. We don't really need special alignment macros in there, as long > as we access the RPC opaque field with the byte sets and copies. > > Would it help if I described this patch as a "clean up" ? -- 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
