drivers: staging: lustre: lustre: include: add "__attribute__((packed))" for the related union

Unfortunately, not all compilers assumes the structures within a pack region also need be packed (e.g. metag), so need add a pack explicitly to satisfy all compilers.

The related error (under metag with allmodconfig):

CC [M] drivers/staging/lustre/lustre/lov/lov_pack.o
drivers/staging/lustre/lustre/lov/lov_pack.c: In function 'lov_getstripe':
drivers/staging/lustre/lustre/lov/lov_pack.c:630: error: duplicate case value drivers/staging/lustre/lustre/lov/lov_pack.c:630: error: previously used here

And originally, all related code used "__attribute__((packed))", so still use it instead of '__packed'.

---
 drivers/staging/lustre/lustre/include/lustre/lustre_user.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 6b6c0240..0828b31 100644
@@ -345,7 +345,7 @@ struct lov_user_md_v3 {	   /* LOV EA user data (host-endian) */
 					   * lmm_objects, use when writing */
 		__u16 lmm_layout_gen;     /* layout generation number
 					   * used when reading */
-	};
+	} __attribute__((packed));
 	char  lmm_pool_name[LOV_MAXPOOLNAME]; /* pool name */
 	struct lov_user_ost_data_v1 lmm_objects[0]; /* per-stripe data */
 } __attribute__((packed));

1.7.11.7

Use __packed. Then at least one line will be correct which is better than nothing.

regards,
dan carpenter

Hmm... but that will break the 'consistency' (which is not quite good for readers).

For me, it will be better to provide another patch to change all "__attribute__((packed))" to "__packed" within this file.

What about your ideas?

Thanks.

Chen Gang

Open, share and attitude like air, water and life which God blessed

In the end, it's not something we care about enough to ask you to redo the patch. But what I'm saying is that you should prefer kernel style over local style. We'll fix the surrounding lines later.

regards,
dan carpenter

OK, thanks, and also thank James and "./scripts/checkpatch.pl" who/which also mentioned about it to me.

BTW: this patch is related with another patch which is discussing (so I have cc that patch to you and Greg too): "if we could sure that it is a compiler's feature issue, we will skip this patch".

Thanks

Chen Gang

Open, share and attitude like air, water and life which God blessed
--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Chen,

If you're referring to the #pragma pack portability issue then this issue is unrelated since it doesn't use #pragma pack.

The issue is *not* that the compiler is defectively failing to pack nested structs. Doing that would be utterly broken since it would change the layout of the same struct depending on where it is placed in the program, Consider this example (which uses a nested struct rather than union to demonstrate the point):

struct a {
	struct b {
		unsigned int x;
		unsigned short y;
	} x;
	unsigned short y;
} __packed;

Both ABIs behave the same here:

Arch sizeof(struct b) sizeof(struct a)
x86_64 8 10
metag 8 10

If struct b is made __packed, again both ABIs behave differently in the same way:

Arch sizeof(struct b) sizeof(struct a)
x86_64 6 8
metag 6 8

The issue is that C compiler ABIs may (and unfortunately metag ABI does) pack structs and unions to at least 4 bytes, even if no members of the struct or union are that large, which means that the nested struct/union should be __packed too to portably ensure it's the expected size.

Cheers
James

--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Ah. From so metag is a new arch and not a compiler like the changelog says.

This is not the code we are discussing. It should look like:

struct a {
	union {
		short x;
		short y;
	}
	short z;
};

Any normal person would assume that sizeof(struct a) would be 4 but apparently on metag it is 8. That totally defeats the point of using a union in the first place. It's easy enough to add a __packed to the lustre declaration but I expect this to cause an endless stream of bugs.

It it is really stupid.

regards,
dan carpenter

I agree completely (and did request this be changed when I first found out about it, but since it's an ABI issue it was really too late). That's why I'm not actively pushing for every case to be fixed unless it's in generic code that actually affects metag.

Cheers
James

It would be easy enough to make the compiler complain about any union which would normally have size which is not a multiple of 4.

Warning: union will be padded with 2 bytes unless __attribute__((packed)).

Otherwise you will be fighting this for ever.

regards,
dan carpenter

Are you sure it's padding the unions, and not just treating the unions as structs? What is the size of this union?

union a {
	int x;
	short y;
};

regards,
dan carpenter

A good idea, but the problem is that most of the time it just doesn't matter since all users of the data structure do so with the same ABI. We already expect the compiler to take some liberties with padding and alignment since the C standard permits it, so it's only when the exact layout really matters (data structures dealing with hardware, stored to disk, or passed over networks) that it ever becomes a problem.

Yes

4

Thanks
James

--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

I made a quick and dirty sparse patch to check for this. I don't think I will bother trying to send it to sparse upstream, but you can if you want to.

It found 289 unions which might need a __packed added. The lustre unions were not in my allmodconfig so they're not listed.

Perhaps there could be a command line option or a pragma so that unions will work in the kernel. We don't care about linking to outside libraries.

regards,
dan carpenter

index ebba56deaf94..596e47883aad 100644
@@ -187,6 +187,12 @@ static struct symbol * examine_struct_union_type(struct symbol *sym, int advance
 		bit_size = (bit_size + bit_align) & ~bit_align;
 	}
 	sym->bit_size = bit_size;
+
+	if (!advance && (info.bit_size / 8) % 4) {
+		int pad = 4 - (info.bit_size / 8) % 4;
+		warning(sym->pos, "'%s' union will be padded with %d bytes unless __attribute__((packed)).",  sym->ident ? sym->ident->name: "<null>", pad);
+	}
+
 	return sym;
 }
 

drivers/acpi/sbshc.c:55:7: warning: 'acpi_smb_status' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/atm/horizon.h:317:9: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:2173:15: warning: 'DAC960_V1_StatusMailbox' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/block/DAC960.h:2580:15: warning: 'DAC960_GEM_OutboundDoorBellRegister' union will be padded with 3 bytes unless __attribute__((packed)). drivers/block/DAC960.h:2618:15: warning: 'DAC960_GEM_ErrorStatusRegister' union will be padded with 3 bytes unless __attribute__((packed)). drivers/block/DAC960.h:2867:15: warning: 'DAC960_BA_InboundDoorBellRegister' union will be padded with 3 bytes unless __attribute__((packed)). drivers/block/DAC960.h:2891:15: warning: 'DAC960_BA_OutboundDoorBellRegister' union will be padded with 2 bytes unless __attribute__((packed)). drivers/block/DAC960.h:2912:15: warning: 'DAC960_BA_InterruptMaskRegister' union will be padded with 1 bytes unless __attribute__((packed)). drivers/block/DAC960.h:2929:15: warning: 'DAC960_BA_ErrorStatusRegister' union will be padded with 2 bytes unless __attribute__((packed)). drivers/block/DAC960.h:3173:15: warning: 'DAC960_LP_InboundDoorBellRegister' union will be padded with 3 bytes unless __attribute__((packed)). drivers/block/DAC960.h:3197:15: warning: 'DAC960_LP_OutboundDoorBellRegister' union will be padded with 2 bytes unless __attribute__((packed)). drivers/block/DAC960.h:3218:15: warning: 'DAC960_LP_InterruptMaskRegister' union will be padded with 2 bytes unless __attribute__((packed)). drivers/block/DAC960.h:3234:15: warning: 'DAC960_LP_ErrorStatusRegister' union will be padded with 2 bytes unless __attribute__((packed)). drivers/block/DAC960.h:3487:15: warning: 'DAC960_LA_InboundDoorBellRegister' union will be padded with 3 bytes unless __attribute__((packed)). drivers/block/DAC960.h:3511:15: warning: 'DAC960_LA_OutboundDoorBellRegister' union will be padded with 2 bytes unless __attribute__((packed)). drivers/block/DAC960.h:3532:15: warning: 'DAC960_LA_InterruptMaskRegister' union will be padded with 2 bytes unless __attribute__((packed)). drivers/block/DAC960.h:3548:15: warning: 'DAC960_LA_ErrorStatusRegister' union will be padded with 2 bytes unless __attribute__((packed)). drivers/block/DAC960.h:3869:15: warning: 'DAC960_PG_ErrorStatusRegister' union will be padded with 2 bytes unless __attribute__((packed)). drivers/block/DAC960.h:4155:15: warning: 'DAC960_PD_OutboundDoorBellRegister' union will be padded with 3 bytes unless __attribute__((packed)). drivers/block/DAC960.h:4174:15: warning: 'DAC960_PD_InterruptEnableRegister' union will be padded with 3 bytes unless __attribute__((packed)). drivers/block/DAC960.h:4189:15: warning: 'DAC960_PD_ErrorStatusRegister' union will be padded with 2 bytes unless __attribute__((packed)). drivers/block/mtip32xx/mtip32xx.h:221:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/block/mtip32xx/mtip32xx.h:225:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/block/mtip32xx/mtip32xx.h:229:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/block/mtip32xx/mtip32xx.h:233:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/block/mtip32xx/mtip32xx.h:238:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/block/mtip32xx/mtip32xx.h:242:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/block/mtip32xx/mtip32xx.h:246:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/block/paride/paride.h:120:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/block/xen-blkback/common.h:197:1: warning: 'blkif_common_sring_entry' union will be padded with 3 bytes unless __attribute__((packed)). drivers/char/tpm/tpm_acpi.c:34:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/char/tpm/tpm.h:209:15: warning: 'cap_t' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/firmware/dcdbas.h:88:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/gpu/drm/ast/ast_mode.c:991:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/gpu/drm/gma500/oaktrail.h:138:7: warning: 'oaktrail_panel_rx' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/gma500/oaktrail.h:157:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/gma500/oaktrail.h:174:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/i915/intel_bios.h:245:7: warning: 'child_device_config' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:1079:9: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:1118:9: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:1169:9: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:1213:3: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:1219:3: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:1279:8: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:1363:8: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:1620:9: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:1636:8: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:1747:9: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:2491:15: warning: '_ATOM_FIRMWARE_CAPABILITY_ACCESS' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:3194:15: warning: '_ATOM_I2C_ID_CONFIG_ACCESS' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:3269:15: warning: '_ATOM_MODE_MISC_INFO_ACCESS' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:4139:8: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:4337:9: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:502:3: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/gpu/drm/radeon/atombios.h:548:8: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:6444:8: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:6448:8: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:6467:3: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:6549:8: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:6553:8: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:6617:8: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:6621:8: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:7122:9: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:7137:9: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:7153:9: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:7225:9: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:7483:15: warning: '_ATOM_CONNECTOR_INFO_ACCESS' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:7562:15: warning: '_ATOM_ENCODER_ATTRIBUTE' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/atombios.h:847:9: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/gpu/drm/radeon/atombios.h:901:8: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/gpu/drm/radeon/atombios.h:906:9: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/gpu/drm/radeon/atombios.h:951:9: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/gpu/drm/radeon/ci_dpm.c:4828:7: warning: 'pplib_power_state' union will be padded with 1 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/kv_dpm.c:2441:7: warning: 'pplib_power_state' union will be padded with 1 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/ni_dpm.c:3910:7: warning: 'pplib_power_state' union will be padded with 1 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/radeon_atombios.c:1994:7: warning: 'pplib_power_state' union will be padded with 1 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/radeon_atombios.c:3261:7: warning: 'voltage_object_info' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/radeon_atombios.c:3267:7: warning: 'voltage_object' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/radeon_atombios.c:42:7: warning: 'atom_supported_devices' union will be padded with 2 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/rs780_dpm.c:711:7: warning: 'pplib_power_state' union will be padded with 1 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/rv6xx_dpm.c:1789:7: warning: 'pplib_power_state' union will be padded with 1 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/rv770_dpm.c:2134:7: warning: 'pplib_power_state' union will be padded with 1 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/rv770_smc.h:96:10: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/gpu/drm/radeon/si_dpm.c:6131:7: warning: 'pplib_power_state' union will be padded with 1 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/sumo_dpm.c:1385:7: warning: 'pplib_power_state' union will be padded with 1 bytes unless __attribute__((packed)). drivers/gpu/drm/radeon/trinity_dpm.c:1606:7: warning: 'pplib_power_state' union will be padded with 1 bytes unless __attribute__((packed)). drivers/gpu/drm/vmwgfx/svga3d_surfacedefs.h:211:17: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/gpu/drm/vmwgfx/svga3d_surfacedefs.h:215:17: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/hv/hyperv_vmbus.h:161:7: warning: 'hv_message_flags' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/input/misc/yealink.c:87:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/isdn/hisax/hisax.h:330:9: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/media/dvb-frontends/bcm3510_priv.h:38:15: warning: 'bcm3510_register_value' union will be padded with 3 bytes unless __attribute__((packed)). drivers/media/usb/dvb-usb/dib0700_core.c:661:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/mfd/lm3533-core.c:250:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/mmc/host/vub300.c:156:7: warning: 'sd_response' union will be padded with 1 bytes unless __attribute__((packed)).
drivers/net/can/usb/ems_usb.c:183:23: warning: '<null>' union will be padded with 1 bytes unless __attribute__((packed)). drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.h:501:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.h:115:7: warning: 'bnx2x_classification_ramrod_data' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/broadcom/cnic_defs.h:3255:7: warning: 'fcoe_idx16_field_union' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/broadcom/cnic_defs.h:3539:7: warning: 'fcoe_vlan_field_union' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/broadcom/cnic_defs.h:3547:7: warning: 'fcoe_vlan_vif_field_union' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/brocade/bna/bfi_enet.h:204:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/brocade/bna/bfi.h:34:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/brocade/bna/bfi.h:451:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/brocade/bna/bfi.h:460:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/intel/e1000e/hw.h:679:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/intel/e1000e/ich8lan.c:62:7: warning: 'ich8_hws_flash_status' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/intel/e1000e/ich8lan.c:79:7: warning: 'ich8_hws_flash_ctrl' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/intel/i40e/i40e_type.h:422:39: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/intel/i40e/i40e_type.h:452:39: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/intel/i40evf/i40e_type.h:430:39: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/intel/i40evf/i40e_type.h:460:39: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/intel/igb/e1000_hw.h:556:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/ethernet/intel/ixgbe/ixgbe.h:275:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/ethernet/intel/ixgbe/ixgbe_type.h:2283:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/ethernet/micrel/ks8851.c:58:7: warning: 'ks8851_tx_hdr' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/net/plip/plip.c:187:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/net/wireless/at76c50x-usb.h:227:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/ath/ath10k/htc.h:68:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/ath/ath10k/htc.h:72:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/ath/ath10k/htt.h:1013:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/ath/ath10k/htt.h:246:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/ath/ath10k/htt.h:545:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/ath/ath10k/htt.h:592:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/ath/ath10k/htt.h:633:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/ath/carl9170/wlan.h:345:15: warning: '<null>' union will be padded with 1 bytes unless __attribute__((packed)). drivers/net/wireless/b43legacy/xmit.h:18:1: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/net/wireless/b43/xmit.h:19:1: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/net/wireless/b43/xmit.h:238:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/net/wireless/b43/xmit.h:251:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/net/wireless/b43/xmit.h:266:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/hostap/hostap_wlan.h:906:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/ipw2x00/ipw2100.h:152:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/ipw2x00/libipw.h:428:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/iwlegacy/3945.h:144:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/iwlegacy/commands.h:1396:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/iwlegacy/commands.h:1498:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/iwlegacy/common.h:535:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/iwlwifi/dvm/commands.h:1298:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/libertas/host.h:327:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/mwifiex/fw.h:621:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/mwifiex/fw.h:640:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/mwifiex/fw.h:724:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/mwifiex/scan.c:61:7: warning: 'mwifiex_scan_cmd_config_tlv' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/ray_cs.h:36:11: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/net/wireless/rayctl.h:521:11: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/net/wireless/rayctl.h:604:11: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/rtlwifi/rtl8188ee/../wifi.h:629:7: warning: 'aci_aifsn' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/rtlwifi/rtl8192ce/../wifi.h:629:7: warning: 'aci_aifsn' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/rtlwifi/rtl8192cu/../wifi.h:629:7: warning: 'aci_aifsn' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/rtlwifi/rtl8192de/../wifi.h:629:7: warning: 'aci_aifsn' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/rtlwifi/rtl8192se/../wifi.h:629:7: warning: 'aci_aifsn' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/rtlwifi/rtl8723ae/../wifi.h:629:7: warning: 'aci_aifsn' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/ti/wl12xx/../wlcore/tx.h:116:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/ti/wl12xx/../wlcore/tx.h:137:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/ti/wl18xx/../wlcore/tx.h:116:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/ti/wl18xx/../wlcore/tx.h:137:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/net/wireless/ti/wlcore/tx.h:116:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/net/wireless/ti/wlcore/tx.h:137:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/nfc/pn533.c:222:7: warning: 'pn533_cmd_poll_initdata' union will be padded with 2 bytes unless __attribute__((packed)). drivers/platform/x86/dell-laptop.c:49:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/platform/x86/wmi.c:53:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/3w-9xxx.h:499:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/3w-sas.h:229:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/3w-xxxx.h:304:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/advansys.c:2354:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/bfa/bfa_defs.h:1013:7: warning: 'sfp_xcvr_so1_code_u' union will be padded with 3 bytes unless __attribute__((packed)). drivers/scsi/bfa/bfa_defs.h:1023:7: warning: 'sfp_xcvr_so2_code_u' union will be padded with 3 bytes unless __attribute__((packed)). drivers/scsi/bfa/bfa_defs.h:1033:7: warning: 'sfp_xcvr_eth_code_u' union will be padded with 3 bytes unless __attribute__((packed)). drivers/scsi/bfa/bfa_defs.h:1054:7: warning: 'sfp_xcvr_fc2_code_u' union will be padded with 3 bytes unless __attribute__((packed)). drivers/scsi/bfa/bfa_defs.h:1068:7: warning: 'sfp_xcvr_fc3_code_u' union will be padded with 3 bytes unless __attribute__((packed)). drivers/scsi/bfa/bfa_defs.h:988:7: warning: 'sfp_xcvr_e10g_code_u' union will be padded with 3 bytes unless __attribute__((packed)). drivers/scsi/bfa/bfa_fcs.h:203:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/bfa/bfi.h:57:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/BusLogic.h:287:7: warning: 'blogic_cntrl_reg' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/scsi/BusLogic.h:320:7: warning: 'blogic_int_reg' union will be padded with 3 bytes unless __attribute__((packed)). drivers/scsi/device_handler/scsi_dh_rdac.c:206:33: warning: '<null>' union will be padded with 1 bytes unless __attribute__((packed)). drivers/scsi/esas2r/atvda.h:198:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/scsi/esas2r/atvda.h:250:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/esas2r/atvda.h:301:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/esas2r/atvda.h:372:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/scsi/hpsa_cmd.h:192:7: warning: 'SCSI3Addr' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/ips.h:933:10: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/isci/phy.h:143:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/isci/probe_roms.h:231:23: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). drivers/scsi/isci/scu_task_context.h:921:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/megaraid.h:588:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/megaraid/megaraid_sas.h:389:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/scsi/megaraid/megaraid_sas.h:400:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/scsi/qla1280.h:400:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/scsi/qla1280.h:469:31: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/scsi/qla1280.h:485:31: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/scsi/qla1280.h:502:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/scsi/qla2xxx/qla_def.h:1525:15: warning: 'target_id_t' union will be padded with 2 bytes unless __attribute__((packed)). drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c:992:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/staging/ft1000/ft1000-usb/ft1000_hw.c:1141:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/staging/media/as102/as102_fw.h:29:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/staging/media/as102/as10x_cmd.h:193:7: warning: 'as10x_add_pid_filter' union will be padded with 2 bytes unless __attribute__((packed)). drivers/staging/slicoss/slichw.h:272:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
drivers/staging/slicoss/slichw.h:690:7: warning: 'oemfru' union will be padded with 1 bytes unless __attribute__((packed)).
drivers/staging/slicoss/slichw.h:723:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
drivers/staging/slicoss/slichw.h:817:15: warning: '<null>' union will be padded with 1 bytes unless __attribute__((packed)). drivers/staging/slicoss/slicoss.c:1192:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/staging/wlan-ng/p80211hdr.h:169:7: warning: 'p80211_hdr' union will be padded with 2 bytes unless __attribute__((packed)). drivers/staging/wlan-ng/prism2fw.c:98:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). drivers/tty/ipwireless/hardware.c:153:7: warning: 'nl_packet' union will be padded with 2 bytes unless __attribute__((packed)). drivers/usb/storage/realtek_cr.c:92:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
fs/adfs/dir_f.h:42:7: warning: 'adfs_dirtail' union will be padded with 3 bytes unless __attribute__((packed)).
fs/cifs/cifspdu.h:555:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
fs/ocfs2/stack_user.c:164:7: warning: 'ocfs2_control_message' union will be padded with 1 bytes unless __attribute__((packed)).
fs/reiserfs/reiserfs.h:1169:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
fs/ufs/ufs_fs.h:293:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). /home/dcarpenter/progs/kernel/devel/arch/x86/include/asm/kvm_emulate.h:232:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). /home/dcarpenter/progs/kernel/devel/include/uapi/linux/atmsap.h:105:11: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). /home/dcarpenter/progs/kernel/devel/include/uapi/linux/atmsap.h:114:11: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). /home/dcarpenter/progs/kernel/devel/include/uapi/linux/bcache.h:200:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). /home/dcarpenter/progs/kernel/devel/include/uapi/linux/cciss_defs.h:56:15: warning: '_SCSI3Addr_struct' union will be padded with 2 bytes unless __attribute__((packed)). /home/dcarpenter/progs/kernel/devel/include/uapi/linux/if_arcnet.h:93:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). /home/dcarpenter/progs/kernel/devel/include/uapi/linux/irda.h:148:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). /home/dcarpenter/progs/kernel/devel/include/uapi/linux/netfilter_ipv4/ipt_ECN.h:26:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). /home/dcarpenter/progs/kernel/devel/include/uapi/linux/netfilter/nf_conntrack_tuple_common.h:13:7: warning: 'nf_conntrack_man_proto' union will be padded with 2 bytes unless __attribute__((packed)). /home/dcarpenter/progs/kernel/devel/include/uapi/linux/netfilter/xt_ecn.h:28:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). /home/dcarpenter/progs/kernel/devel/include/uapi/linux/netfilter/xt_string.h:20:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)). /home/dcarpenter/progs/kernel/devel/include/uapi/linux/nfsd/cld.h:50:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). /home/dcarpenter/progs/kernel/devel/include/uapi/sound/hdspm.h:158:15: warning: '<null>' union will be padded with 1 bytes unless __attribute__((packed)). include/drm/drm_edid.h:139:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
include/drm/drm_edid.h:160:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/linux/f2fs_fs.h:360:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
include/linux/hyperv.h:330:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
include/linux/ide.h:269:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/linux/ide.h:278:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/linux/ieee80211.h:1025:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
include/linux/ieee80211.h:2021:15: warning: '<null>' union will be padded with 1 bytes unless __attribute__((packed)).
include/linux/input/sparse-keymap.h:38:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
include/linux/ipv6.h:170:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
include/linux/isdnif.h:372:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/linux/mlx4/cq.h:47:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
include/linux/phonet.h:35:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/linux/usb/wusb-wa.h:101:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/linux/usb/wusb-wa.h:106:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/linux/usb/wusb-wa.h:97:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/net/caif/cfctrl.h:65:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
include/net/irda/discovery.h:53:15: warning: '__u16_host_order' union will be padded with 2 bytes unless __attribute__((packed)).
include/net/mac80211.h:681:31: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
include/net/netfilter/nf_conntrack_tuple.h:42:23: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
include/net/phonet/pep.h:61:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/net/xfrm.h:121:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/rxrpc/packet.h:62:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
include/scsi/osd_initiator.h:307:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
include/scsi/sas.h:218:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/scsi/sas.h:219:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/scsi/sas.h:227:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/scsi/sas.h:242:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/scsi/sas.h:243:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/scsi/sas.h:251:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/scsi/sas.h:272:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/scsi/sas.h:284:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
include/uapi/linux/i2c.h:128:7: warning: 'i2c_smbus_data' union will be padded with 2 bytes unless __attribute__((packed)). include/uapi/linux/netfilter_bridge/ebt_802_3.h:49:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). include/uapi/linux/phonet.h:71:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
include/xen/interface/io/netif.h:125:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
lib/zlib_deflate/defutil.h:41:11: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
lib/zlib_deflate/defutil.h:45:11: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
lib/zlib_inflate/inffast.c:24:7: warning: 'uu' union will be padded with 2 bytes unless __attribute__((packed)).
net/decnet/af_decnet.c:1345:15: warning: '<null>' union will be padded with 1 bytes unless __attribute__((packed)).
net/irda/wrapper.c:89:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
net/netfilter/ipset/ip_set_hash_netnet.c:269:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). net/netfilter/ipset/ip_set_hash_netnet.c:48:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). net/netfilter/ipset/ip_set_hash_netportnet.c:316:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). net/netfilter/ipset/ip_set_hash_netportnet.c:50:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
net/netfilter/nft_ct.c:26:14: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
net/netfilter/nft_meta.c:24:15: warning: '<null>' union will be padded with 3 bytes unless __attribute__((packed)).
security/integrity/evm/../integrity.h:70:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). security/integrity/ima/../integrity.h:70:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)). security/integrity/integrity.h:70:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).
sound/pci/ctxfi/cthw20k1.c:115:7: warning: 'src_mgr_dirty' union will be padded with 2 bytes unless __attribute__((packed)).
sound/pci/ctxfi/cthw20k1.c:148:7: warning: 'srcimp_mgr_dirty' union will be padded with 2 bytes unless __attribute__((packed)).
sound/pci/ctxfi/cthw20k1.c:594:7: warning: 'amixer_dirty' union will be padded with 2 bytes unless __attribute__((packed)).
sound/pci/ctxfi/cthw20k1.c:756:7: warning: 'dai_dirty' union will be padded with 2 bytes unless __attribute__((packed)).
sound/pci/ctxfi/cthw20k1.c:771:7: warning: 'dao_dirty' union will be padded with 2 bytes unless __attribute__((packed)).
sound/pci/ctxfi/cthw20k1.c:90:7: warning: 'src_dirty' union will be padded with 2 bytes unless __attribute__((packed)).
sound/pci/ctxfi/cthw20k2.c:115:7: warning: 'src_mgr_dirty' union will be padded with 2 bytes unless __attribute__((packed)).
sound/pci/ctxfi/cthw20k2.c:148:7: warning: 'srcimp_mgr_dirty' union will be padded with 2 bytes unless __attribute__((packed)).
sound/pci/ctxfi/cthw20k2.c:593:7: warning: 'amixer_dirty' union will be padded with 2 bytes unless __attribute__((packed)).
sound/pci/ctxfi/cthw20k2.c:747:7: warning: 'dai_dirty' union will be padded with 2 bytes unless __attribute__((packed)).
sound/pci/ctxfi/cthw20k2.c:787:7: warning: 'dao_dirty' union will be padded with 2 bytes unless __attribute__((packed)).
sound/pci/ctxfi/cthw20k2.c:90:7: warning: 'src_dirty' union will be padded with 2 bytes unless __attribute__((packed)).
sound/pci/hda/patch_hdmi.c:169:7: warning: 'audio_infoframe' union will be padded with 3 bytes unless __attribute__((packed)). sound/usb/usx2y/usbus428ctldefs.h:90:15: warning: '<null>' union will be padded with 2 bytes unless __attribute__((packed)).

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Hi Dan,

Thanks a lot for this, it seems to be useful. I'm adapting it to reduce false negatives (e.g. omitting the check if the struct/union is already packed), and I imagine it could be made to only warn about padded unpacked structs/unions which are used as nested members of packed structs/unions. It wouldn't catch everything but would probably catch a lot of cases that are most likely to be genuine since they would have been packed at the outer level for a reason.

We still interact with userland via structs and unions, so it would probably have to exclude anything in uapi/.

Cheers
James

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Thank all of you firstly.

But excuse me, I am still not quit clear that: "what need we do enough to solve this feature issue?"

So I guess our current result is:

- It is not a good idea to only let kernel to fit with compiler.

- It is not a good idea to only let compiler to fit with kernel.

- Need let compiler and kernel to fit with each other:

- compiler will print related warning, but not break compiling. so metag compiler need be improvement (check and warn for it).

- if check alignment explicitly in kernel source code, it need be fixed within kernel: "apply related patches (pack each struct or union), but the related patch comments need be improved".

Is what I guess correct?

Thanks.

Chen Gang

Open, share and attitude like air, water and life which God blessed

It seems, our kernel still stick to treate 'pack' region have effect with both 'align' and 'sizeof'.

So for compiler, could we add one additional cflag parameter to tell compiler to switch it (compatible with ABI, or satisfy upstream kernel).

And for kernel, it will be OK enough to only append this parameter to KBUILD_CFLAGS in "arch/metag/Makefile".

Thanks.

Chen Gang

Open, share and attitude like air, water and life which God blessed

It's not about packed regions. It's about unions. It's saying the sizeof() a union is a multiple of 4 unless it's packed.

union foo {
	short x;
	short y;
};

The author intended the sizeof(union foo) to be 2 but on metag arch then it is 4.

regards,
dan carpenter

Yeah, just like your original discussion. :-)

Hmm... can we say: "for metag compiler, in a pack region, it considers variables alignment, but does not consider about struct/union alignment (except append packed to each related struct/union)".

For compatible (consider about its ABI), it has to keep this features, but for kernel, it needs be changed.

So, I suggest to add one parameter to compiler to switch this feature, and append this parameter to KBUILD_CFLAGS in "arch/metag/Makefile" which can satisfy both ABI and kernel.

Thanks.

Chen Gang

Open, share and attitude like air, water and life which God blessed

After append the parameter to KBUILD_CFLAGS in "arch/metag/Makefile",

- I guess/assume "include/uapi/*" should/will not need be modified.

- but need check all files in "arch/metag/include/uapi/*". (add padding data for packed struct/union when __KERNEL__ defined)

- maybe we have to process metag related ABI which not in "*/uapi/*" (add padding data for packed struct/union when __KERNEL__ defined) and when we find them, recommend to move all of them to "*/uapi/*".

Sorry, I don't know whether this way is the best way or not, but for me it is an executable way to solve this feature issue and satisfy both kernel and ABI.

Thanks.

Chen Gang

Open, share and attitude like air, water and life which God blessed
--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Dan Carpenter

The same is probably be true of: struct foo { _u16 bar; };

Architectures that define such alignment rules are a right PITA. You either need to get the size to 2 without using 'packed', or just not define such structures.
It is worth seeing if adding aligned(2) will change the size - I'm not sure.

David

Yes indeed.

__aligned(2) alone doesn't seem to have any effect on sizeof() or __alignof__() unless it is accompanied by __packed. x86_64 is similar in that respect (it just packs sanely in the first place).

Combining __packed with __aligned(2) does the trick though (__packed alone sets __aligned(1) which is obviously going to be suboptimal).

Cheers
James

Oh, thank you for your explanation.

And hope this feature issue can be fixed, and satisfy both kernel and ABI. :-)

Thanks.

Chen Gang

Open, share and attitude like air, water and life which God blessed

From: James Hogan

Compile some code for a cpu that doesn't support misaligned transfers (probably one of sparc, arm, ppc) and see if the compiler generates a single 16bit request or two 8 bits ones.
You don't want the compiler generating multiple byte-sized memory transfers.

David

--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Meta is also one of those arches, and according to my quick tests, __packed alone does correctly make it fall back to byte loads/stores, but with __packed __aligned(2) it uses 16bit loads/stores. I've also confirmed that with an ARM toolchain (see below for example).

Cheers
James

input:

#define __aligned(x) __attribute__((aligned(x)))
#define __packed __attribute__((packed))

union a {
	short x, y;
} __aligned(2) __packed;

struct b {
	short x;
} __aligned(2) __packed;

unsigned int soa = sizeof(union a);
unsigned int aoa = __alignof__(union a);
unsigned int sob = sizeof(struct b);
unsigned int aob = __alignof__(struct b);

void t(struct b *x, union a *y)

{
	++x->x;
	++y->x;
}

ARM output (-O2):

	.cpu arm10tdmi
	.fpu softvfp
	.eabi_attribute 20, 1
	.eabi_attribute 21, 1
	.eabi_attribute 23, 3
	.eabi_attribute 24, 1
	.eabi_attribute 25, 1
	.eabi_attribute 26, 2
	.eabi_attribute 30, 2
	.eabi_attribute 34, 0
	.eabi_attribute 18, 4
	.file	"alignment4.c"
	.text
	.align	2
	.global	t
	.type	t, %function
t:
	@ args = 0, pretend = 0, frame = 0
	@ frame_needed = 0, uses_anonymous_args = 0
	@ link register save eliminated.
	ldrh	r3, [r0, #0]
	add	r3, r3, #1
	strh	r3, [r0, #0]	@ movhi
	ldrh	r3, [r1, #0]
	add	r3, r3, #1
	strh	r3, [r1, #0]	@ movhi
	bx	lr
	.size	t, .-t
	.global	aob
	.global	sob
	.global	aoa
	.global	soa
	.data
	.align	2
	.type	aob, %object
	.size	aob, 4
aob:
	.word	2
	.type	sob, %object
	.size	sob, 4
sob:
	.word	2
	.type	aoa, %object
	.size	aoa, 4
aoa:
	.word	2
	.type	soa, %object
	.size	soa, 4
soa:
	.word	2
	.ident	"GCC: (GNU) 4.7.1 20120606 (Red Hat 4.7.1-0.1.20120606)"
	.section	.note.GNU-stack,"",%progbits

From: James Hogan

...

I would either:
1a) Add explicit padding to the relevant structures so that they are
multiple of 4 bytes.
or:
1b) #define some token to "__packed __aligned(2)" before all the structures
that require changing, and use that in there definitions. This lets you comment on WHY you are doing it.
and:
2) Add a compile-time assert that the structures are the correct size.

Clearly you don't want to mark anything that contains a 32bit value with __packed __aligned(2).

I'm not at all clear whether you are sometimes using a different compiler.

David

I guess so.

Sorry, I do not know about PITA (after google or wiki, I can not get more related information).

Could you provide more information about PITA, thanks?

Excuse me, I don't quite understand your meaning. I guess your meaning is:

"normally, we should not use a struct/union like that, no matter what it is (2 or 4)".

Is it correct.

Yes, it will/should make sure that it must be 2.

Thanks.

Chen Gang

Open, share and attitude like air, water and life which God blessed
--
To unsubscribe from this list: send the line "unsubscribe linux-metag" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html