All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sven Eckelmann <sven@narfation.org>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: ath11k@lists.infradead.org, Wen Gong <quic_wgong@quicinc.com>,
	linux-wireless@vger.kernel.org, Jason Gunthorpe <jgg@ziepe.ca>,
	Joe Perches <joe@perches.com>, Jonathan Corbet <corbet@lwn.net>
Subject: Re: [PATCH v4 3/4] ath11k: add support for device recovery for QCA6390
Date: Wed, 17 Nov 2021 09:46:08 +0100	[thread overview]
Message-ID: <2357945.8AGfkZV0UB@sven-desktop> (raw)
In-Reply-To: <87mtm3dwu0.fsf@codeaurora.org>


[-- Attachment #1.1: Type: text/plain, Size: 2165 bytes --]

On Wednesday, 17 November 2021 09:12:55 CET Kalle Valo wrote:
> > https://www.kernel.org/doc/html/v5.15/process/coding-style.html#using-bool
[...]
> 
> Yeah, I have been worried about this as well and we should fix this. But
> instead of u8 I would prefer to use bool like mt76 uses:
[...]
> I didn't even know using bool is legal until I saw it in mt76.

Interesting, I was also not aware of it. And it also seems to have some 
interesting implications when assigning values to it (example 4):

    #include <stdbool.h>
    #include <stdint.h>
    #include <stdio.h>
    
    struct test {
    	uint8_t u:1;
    	uint8_t u2:1;
    	bool b:1;
    	bool b2:1;
    };
    
    int main(void)
    {
    	struct test x;
    
    	x.u = false;
    	x.b = false;
    	printf("u %u b %u\n", x.u, x.b);
    
    	x.u = true;
    	x.b = true;
    	printf("u %u b %u\n", x.u, x.b);
    
    	x.u = 0;
    	x.b = 0;
    	printf("u %u b %u\n", x.u, x.b);
    
    	x.u = 8;
    	x.b = 8;
    	printf("u %u b %u\n", x.u, x.b);
    
    	return 0;
    }


Result:

    u 0 b 0
    u 1 b 1
    u 0 b 0
    u 0 b 1


The last example is basically the reason we see stuff like

    boolean_like_value = !!(some_retrieved_value);

when using unsigned bitfields instead of bool (bitfields).


And the memory layout (on x86-64):

    $ pahole test.o                                    
    struct test {
            uint8_t                    u:1;                  /*     0: 0  1 */
            uint8_t                    u2:1;                 /*     0: 1  1 */
            _Bool                      b:1;                  /*     0: 2  1 */
            _Bool                      b2:1;                 /*     0: 3  1 */
    
            /* size: 1, cachelines: 1, members: 4 */
            /* bit_padding: 4 bits */
            /* last cacheline: 1 bytes */
    };


To my surprise, it was already mentioned in one of the discussions [1].
Was there anything in the discussion which I might have missed and 
is a good reason to not use "bool ...:1" in structs?

Kind regards,
	Sven

[1] https://lore.kernel.org/all/CA+55aFwVZk1OfB9T2v014PTAKFhtVan_Zj2dOjnCy3x6E4UJfA@mail.gmail.com/T/#u

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 102 bytes --]

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

WARNING: multiple messages have this Message-ID (diff)
From: Sven Eckelmann <sven@narfation.org>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: ath11k@lists.infradead.org, Wen Gong <quic_wgong@quicinc.com>,
	linux-wireless@vger.kernel.org, Jason Gunthorpe <jgg@ziepe.ca>,
	Joe Perches <joe@perches.com>, Jonathan Corbet <corbet@lwn.net>
Subject: Re: [PATCH v4 3/4] ath11k: add support for device recovery for QCA6390
Date: Wed, 17 Nov 2021 09:46:08 +0100	[thread overview]
Message-ID: <2357945.8AGfkZV0UB@sven-desktop> (raw)
In-Reply-To: <87mtm3dwu0.fsf@codeaurora.org>

[-- Attachment #1: Type: text/plain, Size: 2165 bytes --]

On Wednesday, 17 November 2021 09:12:55 CET Kalle Valo wrote:
> > https://www.kernel.org/doc/html/v5.15/process/coding-style.html#using-bool
[...]
> 
> Yeah, I have been worried about this as well and we should fix this. But
> instead of u8 I would prefer to use bool like mt76 uses:
[...]
> I didn't even know using bool is legal until I saw it in mt76.

Interesting, I was also not aware of it. And it also seems to have some 
interesting implications when assigning values to it (example 4):

    #include <stdbool.h>
    #include <stdint.h>
    #include <stdio.h>
    
    struct test {
    	uint8_t u:1;
    	uint8_t u2:1;
    	bool b:1;
    	bool b2:1;
    };
    
    int main(void)
    {
    	struct test x;
    
    	x.u = false;
    	x.b = false;
    	printf("u %u b %u\n", x.u, x.b);
    
    	x.u = true;
    	x.b = true;
    	printf("u %u b %u\n", x.u, x.b);
    
    	x.u = 0;
    	x.b = 0;
    	printf("u %u b %u\n", x.u, x.b);
    
    	x.u = 8;
    	x.b = 8;
    	printf("u %u b %u\n", x.u, x.b);
    
    	return 0;
    }


Result:

    u 0 b 0
    u 1 b 1
    u 0 b 0
    u 0 b 1


The last example is basically the reason we see stuff like

    boolean_like_value = !!(some_retrieved_value);

when using unsigned bitfields instead of bool (bitfields).


And the memory layout (on x86-64):

    $ pahole test.o                                    
    struct test {
            uint8_t                    u:1;                  /*     0: 0  1 */
            uint8_t                    u2:1;                 /*     0: 1  1 */
            _Bool                      b:1;                  /*     0: 2  1 */
            _Bool                      b2:1;                 /*     0: 3  1 */
    
            /* size: 1, cachelines: 1, members: 4 */
            /* bit_padding: 4 bits */
            /* last cacheline: 1 bytes */
    };


To my surprise, it was already mentioned in one of the discussions [1].
Was there anything in the discussion which I might have missed and 
is a good reason to not use "bool ...:1" in structs?

Kind regards,
	Sven

[1] https://lore.kernel.org/all/CA+55aFwVZk1OfB9T2v014PTAKFhtVan_Zj2dOjnCy3x6E4UJfA@mail.gmail.com/T/#u

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2021-11-18  9:41 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-16  4:15 [PATCH v4 0/4] ath11k: add feature for device recovery Wen Gong
2021-11-16  4:15 ` Wen Gong
2021-11-16  4:15 ` [PATCH v4 1/4] ath11k: add ath11k_qmi_free_resource() for recovery Wen Gong
2021-11-16  4:15   ` Wen Gong
2021-11-17  8:48   ` Kalle Valo
2021-11-17  8:48     ` Kalle Valo
2021-11-16  4:15 ` [PATCH v4 2/4] ath11k: fix invalid m3 buffer address Wen Gong
2021-11-16  4:15   ` Wen Gong
2021-11-16  4:15 ` [PATCH v4 3/4] ath11k: add support for device recovery for QCA6390 Wen Gong
2021-11-16  4:15   ` Wen Gong
2021-11-16  8:15   ` Sven Eckelmann
2021-11-16  8:15     ` Sven Eckelmann
2021-11-17  8:12     ` Kalle Valo
2021-11-17  8:12       ` Kalle Valo
2021-11-17  8:46       ` Sven Eckelmann [this message]
2021-11-17  8:46         ` Sven Eckelmann
2021-11-19 15:17         ` ath11k: using boolean bitfields? Kalle Valo
2021-11-19 15:17           ` Kalle Valo
2021-11-16  4:15 ` [PATCH v4 4/4] ath11k: add synchronization operation between reconfigure of mac80211 and ath11k_base Wen Gong
2021-11-16  4:15   ` Wen Gong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2357945.8AGfkZV0UB@sven-desktop \
    --to=sven@narfation.org \
    --cc=ath11k@lists.infradead.org \
    --cc=corbet@lwn.net \
    --cc=jgg@ziepe.ca \
    --cc=joe@perches.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=quic_wgong@quicinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.