Re: [GIT PULL] bcachefs

Arnaldo Carvalho de Melo
2023-09-06T21:55:45Z

Em Wed, Sep 06, 2023 at 01:20:59PM -0700, Linus Torvalds escreveu:
> On Wed, 6 Sept 2023 at 13:02, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > And guess what happens when you have (unsigned char)-1? It does *not*
> > cast back to -1.
>
> Side note: again, this may be one of those "it works in practice",
> because if we have -fshort-enums, I think 'enum
> btree_node_locked_type' in turn ends up being represented as a 'signed
> char', because that's the smallest simple type that can fit all those
> values.

> I don't think gcc ever uses less than that (ie while a six_lock_type
> could fit in two bits, it's still going to be considered at least a
> 8-bit value in practice).

There are some cases where people stuff the enum into a bitfield, but
no, no simple type.

⬢[acme@toolbox perf-tools-next]$ pahole | grep -w enum | grep :
enum btrfs_rsv_type type:8; /* 28:16 4 */
enum btrfs_delayed_item_type type:8; /* 100: 0 4 */
enum kernel_pkey_operation op:8; /* 40: 0 4 */
enum integrity_status ima_file_status:4; /* 96: 0 4 */
enum integrity_status ima_mmap_status:4; /* 96: 4 4 */
enum integrity_status ima_bprm_status:4; /* 96: 8 4 */
enum integrity_status ima_read_status:4; /* 96:12 4 */
enum integrity_status ima_creds_status:4; /* 96:16 4 */
enum integrity_status evm_status:4; /* 96:20 4 */
enum fs_context_purpose purpose:8; /* 152: 0 4 */
enum fs_context_phase phase:8; /* 152: 8 4 */
enum fs_value_type type:8; /* 8: 0 4 */
enum sgx_page_type type:16; /* 8: 8 4 */
enum nf_hook_ops_type hook_ops_type:8; /* 24: 8 4 */
enum resctrl_event_id evtid:8; /* 0:10 4 */
enum _cache_type type:5; /* 0: 0 4 */
⬢[acme@toolbox perf-tools-next]$ pahole _cache_type
enum _cache_type {
CTYPE_NULL = 0,
CTYPE_DATA = 1,
CTYPE_INST = 2,
CTYPE_UNIFIED = 3,
};

⬢[acme@toolbox perf-tools-next]$ pahole integrity_status
enum integrity_status {
INTEGRITY_PASS = 0,
INTEGRITY_PASS_IMMUTABLE = 1,
INTEGRITY_FAIL = 2,
INTEGRITY_FAIL_IMMUTABLE = 3,
INTEGRITY_NOLABEL = 4,
INTEGRITY_NOXATTRS = 5,
INTEGRITY_UNKNOWN = 6,
};

⬢[acme@toolbox perf-tools-next]
>
> So we may have 'enum six_lock_type' essentially being 'unsigned char',
> and when the code does
>
> mark_btree_node_locked(trans, path, 0, BTREE_NODE_UNLOCKED);
>
> that BTREE_NODE_UNLOCKED value might actually be 255.
>
> And then when it's cast to 'enum btree_node_locked_type' in the inline
> function, the 255 will be cast to 'signed char', and we'll end up
> compatible with '(enum btree_node_locked_type)-1' again.
>
> So it's one of those things that are seriously wrong to do, but might
> generate the expected code anyway.
>
> Unless the compiler adds any other sanity checks, like UBSAN or
> something, that actually uses the exact range of the enums.
>
> It could happen even without UBSAN, if the compiler ends up going "I
> can see that the original value came from a 'enum six_lock_type', so I
> know the original value can't be signed, so any comparison with
> BTREE_NODE_UNLOCKED can never be true.
>
> But again, I suspect that in practice this all just happens to work.
> That doesn't make it right.
>
> Linus

--

- Arnaldo