Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 7 Jan 2019 23:27:48 +0530
From:   Himanshu Jha <himanshujha199640@gmail.com>
To:     Matt Ranostay <matt.ranostay@konsulko.com>
Cc:     Amir Mahdi Ghorbanian <indigoomega021@gmail.com>, lars@metafoo.de,
        Michael.Hennerich@analog.com, jic23@kernel.org, knaack.h@gmx.de,
        pmeerw@pmeerw.net, gregkh@linuxfoundation.org,
        linux-iio@vger.kernel.org, devel@driverdev.osuosl.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Staging: iio: ad7192: replaced bool in struct
Message-ID: <20190107175748.GA29220@himanshu-Vostro-3559>
References: <1545434786-15220-1-git-send-email-indigoomega021@gmail.com>
 <20181224095823.GA17061@himanshu-Vostro-3559>
 <8E720BF2-7AFC-4FFA-8FA2-5A403FD5DD1D@konsulko.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <8E720BF2-7AFC-4FFA-8FA2-5A403FD5DD1D@konsulko.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Jan 02, 2019 at 01:25:27PM -0800, Matt Ranostay wrote:
> 
> > On Dec 24, 2018, at 01:58, Himanshu Jha <himanshujha199640@gmail.com> wrote:
> > 
> >> On Fri, Dec 21, 2018 at 03:26:26PM -0800, Amir Mahdi Ghorbanian wrote:
> >> Replaced bool in struct with unsigned int bitfield to conserve space and
> >> more clearly define size of varibales
> 
> Important thing to note is depending on padding, alignment, and position of field it probably won’t save any space.

Well, then what do you say about the following results ;-)

Before:
------

himanshu@himanshu-Vostro-3559:~/gsoc/linux$ pahole -C ad7192_platform_data drivers/staging/iio/adc/ad7192.o
struct ad7192_platform_data {
	u16                        vref_mv;              /*     0     2 */
	u8                         clock_source_sel;     /*     2     1 */

	/* XXX 1 byte hole, try to pack */

	u32                        ext_clk_hz;           /*     4     4 */
	bool                       refin2_en;            /*     8     1 */
	bool                       rej60_en;             /*     9     1 */
	bool                       sinc3_en;             /*    10     1 */
	bool                       chop_en;              /*    11     1 */
	bool                       buf_en;               /*    12     1 */
	bool                       unipolar_en;          /*    13     1 */
	bool                       burnout_curr_en;      /*    14     1 */

	/* size: 16, cachelines: 1, members: 10 */
	/* sum members: 14, holes: 1, sum holes: 1 */
	/* padding: 1 */
	/* last cacheline: 16 bytes */
};

After:
-----

himanshu@himanshu-Vostro-3559:~/gsoc/linux$ pahole -C ad7192_platform_data drivers/staging/iio/adc/ad7192.o
struct ad7192_platform_data {
	u16                        vref_mv;              /*     0     2 */
	u8                         clock_source_sel;     /*     2     1 */

	/* XXX 1 byte hole, try to pack */

	u32                        ext_clk_hz;           /*     4     4 */
	unsigned int               refin2_en:1;          /*     8:31  4 */
	unsigned int               rej60_en:1;           /*     8:30  4 */
	unsigned int               sinc3_en:1;           /*     8:29  4 */
	unsigned int               chop_en:1;            /*     8:28  4 */
	unsigned int               buf_en:1;             /*     8:27  4 */
	unsigned int               unipolar_en:1;        /*     8:26  4 */
	unsigned int               burnout_curr_en:1;    /*     8:25  4 */

	/* size: 12, cachelines: 1, members: 10 */
	/* sum members: 11, holes: 1, sum holes: 1 */
	/* bit_padding: 25 bits */
	/* last cacheline: 12 bytes */
};

> Also for internal unpacked structures it really makes little sense to save a few bytes of data. Don’t read into that packed structures are good.. they usually aren’t :)

Yes, agreed, but I often see patches to save few bytes by marking
array as static.

There is some effort in this direction:
https://lore.kernel.org/lkml/20181221235230.GC13168@ziepe.ca/

Very likely to get more patches if the above patch gets merged.


-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology