Re: [PATCH 3/5] Refine packed annotations in stat.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/13/19 1:25 PM, Jens Axboe wrote:
> On 8/13/19 9:12 AM, Bart Van Assche wrote:
>> On 8/12/19 7:53 PM, Jens Axboe wrote:
>>> On 8/12/19 8:01 PM, Bart Van Assche wrote:
>>>> Instead of declaring the whole structure packed, only declare non-aligned
>>>> members packed. This patch is an alternative way to fix the following gcc 9
>>>> compiler warnings:
>>>>
>>>> eta.c: In function 'calc_thread_status':
>>>> eta.c:510:7: error: taking address of packed member of 'struct jobs_eta' may result in an unaligned pointer value [-Werror=address-of-packed-member]
>>>>      510 |     je->rate);
>>>>          |     ~~^~~~~~
>>>> eta.c:522:66: error: taking address of packed member of 'struct jobs_eta' may result in an unaligned pointer value [-Werror=address-of-packed-member]
>>>>      522 |  calc_rate(unified_rw_rep, disp_time, io_bytes, disp_io_bytes, je->rate);
>>>>          |                                                                ~~^~~~~~
>>>> eta.c:523:64: error: taking address of packed member of 'struct jobs_eta' may result in an unaligned pointer value [-Werror=address-of-packed-member]
>>>>      523 |  calc_iops(unified_rw_rep, disp_time, io_iops, disp_io_iops, je->iops);
>>>>          |
>>>
>>> This seems fragile. Not that we change the struct all the time, or even often,
>>> but it'd be easy to add members and end up with different layout on 32-bit
>>> vs 64-bit.
>>>
>>> How do we improve on that?
>>
>> Hi Jens,
>>
>> Do you agree that the "BUILD_BUG_ON(sizeof(struct jobs_eta) != 160)"
>> statement added by the previous patch should catch such differences? 160
>> bytes namely is the size of an entirely packed jobs_eta structure.
> 
> I guess that's good enough, though it'd be nice to check for holes
> explicitly. I wonder if there's an easy way to do that, though...

Hi Jens,

The pahole tool should be able to check for holes. Unfortunately that
tool doesn't seem to work very well on Ubuntu:

$ pahole -H 1 *.o
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x12d> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x175> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x58> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x15e> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x44> not handled!
die__process_function: tag not supported (INVALID)!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xb5> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xf2> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x48> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x4cc> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x1ab> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x4b6> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x736> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x4ba> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x187> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x4e4> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x42> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xd1> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xfd> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xaf> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x103> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x73> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xb5> not handled!
struct _IO_FILE {
	int                        _flags;               /*     0     4 */

	/* XXX 4 bytes hole, try to pack */

	char *                     _IO_read_ptr;         /*     8     8 */
	char *                     _IO_read_end;         /*    16     8 */
	char *                     _IO_read_base;        /*    24     8 */
	char *                     _IO_write_base;       /*    32     8 */
	char *                     _IO_write_ptr;        /*    40     8 */
	char *                     _IO_write_end;        /*    48     8 */
	char *                     _IO_buf_base;         /*    56     8 */
	/* --- cacheline 1 boundary (64 bytes) --- */
	char *                     _IO_buf_end;          /*    64     8 */
	char *                     _IO_save_base;        /*    72     8 */
	char *                     _IO_backup_base;      /*    80     8 */
	char *                     _IO_save_end;         /*    88     8 */
	struct _IO_marker *        _markers;             /*    96     8 */
	struct _IO_FILE *          _chain;               /*   104     8 */
	int                        _fileno;              /*   112     4 */
	int                        _flags2;              /*   116     4 */
	__off_t                    _old_offset;          /*   120     8 */
	/* --- cacheline 2 boundary (128 bytes) --- */
	short unsigned int         _cur_column;          /*   128     2 */
	signed char                _vtable_offset;       /*   130     1 */
	char                       _shortbuf[1];         /*   131     1 */

	/* XXX 4 bytes hole, try to pack */

	_IO_lock_t *               _lock;                /*   136     8 */
	__off64_t                  _offset;              /*   144     8 */
	struct _IO_codecvt *       _codecvt;             /*   152     8 */
	struct _IO_wide_data *     _wide_data;           /*   160     8 */
	struct _IO_FILE *          _freeres_list;        /*   168     8 */
	void *                     _freeres_buf;         /*   176     8 */
	size_t                     __pad5;               /*   184     8 */
	/* --- cacheline 3 boundary (192 bytes) --- */
	int                        _mode;                /*   192     4 */
	char                       _unused2[20];         /*   196    20 */

	/* size: 216, cachelines: 4, members: 29 */
	/* sum members: 208, holes: 2, sum holes: 8 */
	/* last cacheline: 24 bytes */
};
struct fio_filelock {
	uint32_t                   hash;                 /*     0     4 */

	/* XXX 4 bytes hole, try to pack */

	struct fio_sem             lock;                 /*     8   104 */

	/* XXX last struct has 4 bytes of padding */

	/* --- cacheline 1 boundary (64 bytes) was 48 bytes ago --- */
	struct flist_head          list;                 /*   112    16 */
	/* --- cacheline 2 boundary (128 bytes) --- */
	unsigned int               references;           /*   128     4 */

	/* size: 136, cachelines: 3, members: 4 */
	/* sum members: 128, holes: 1, sum holes: 4 */
	/* padding: 4 */
	/* paddings: 1, sum paddings: 4 */
	/* last cacheline: 8 bytes */
};
struct io_u_ring {
	unsigned int               head;                 /*     0     4 */
	unsigned int               tail;                 /*     4     4 */
	unsigned int               max;                  /*     8     4 */

	/* XXX 4 bytes hole, try to pack */

	struct io_u * *            ring;                 /*    16     8 */

	/* size: 24, cachelines: 1, members: 4 */
	/* sum members: 20, holes: 1, sum holes: 4 */
	/* last cacheline: 24 bytes */
};
struct json_object {
	struct json_pair * *       pairs;                /*     0     8 */
	int                        pair_cnt;             /*     8     4 */

	/* XXX 4 bytes hole, try to pack */

	struct json_value *        parent;               /*    16     8 */

	/* size: 24, cachelines: 1, members: 3 */
	/* sum members: 20, holes: 1, sum holes: 4 */
	/* last cacheline: 24 bytes */
};
struct json_array {
	struct json_value * *      values;               /*     0     8 */
	int                        value_cnt;            /*     8     4 */

	/* XXX 4 bytes hole, try to pack */

	struct json_value *        parent;      die__process_unit:
DW_TAG_restrict_type (0x37) @ <0x283> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xfc> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x14d> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x127> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xae> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x4f> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x4ba> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xda> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x552> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xa5> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xfb> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x59> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x72> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x4ca> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xe5> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x4ba> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xe5> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0xf6> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x5d> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x41> not handled!
die__process_unit: DW_TAG_restrict_type (0x37) @ <0x4c5> not handled!
         /*    16     8 */

	/* size: 24, cachelines: 1, members: 3 */
	/* sum members: 20, holes: 1, sum holes: 4 */
	/* last cacheline: 24 bytes */
};
struct json_value {
	int                        type;                 /*     0     4 */

	/* XXX 4 bytes hole, try to pack */

	union {
		long long int      integer_number;       /*           8 */
		double             float_number;         /*           8 */
		char *             string;               /*           8 */
		struct json_object * object;             /*           8 */
		struct json_array * array;               /*           8 */
	};                                               /*     8     8 */
	int                        parent_type;          /*    16     4 */

	/* XXX 4 bytes hole, try to pack */

	union {
		struct json_pair * parent_pair;          /*           8 */
		struct json_array * parent_array;        /*           8 */
	};                                               /*    24     8 */

	/* size: 32, cachelines: 1, members: 4 */
	/* sum members: 24, holes: 2, sum holes: 8 */
	/* last cacheline: 32 bytes */
};

Bart.




[Index of Archives]     [Linux Kernel]     [Linux SCSI]     [Linux IDE]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux