Messages in this thread | | | Date | Thu, 24 Feb 2022 15:09:08 -0500 | Subject | Re: [PATCH v4] btrfs: add fs state details to error messages. | From | Sweet Tea Dorminy <> |
| |
>> All the other interactions with info->fs_state are test/set/clear_bit, >> which treat the argument as volatile and are therefore safe to do from >> multiple threads. Without the READ_ONCE (reading it as a volatile), >> the compiler or cpu could turn the reads of info->fs_state in >> for_each_set_bit() into writes of random stuff into info->fs_state, >> potentially clearing the state bits or filling them with garbage. > I'm not sure I'm missing something, but I find the above hard to > believe. Concurrent access to a variable from multiple threads may not > produce consistent results, but random writes should not happen when > we're just reading.
Maybe I've been reading too many articles about the things compilers are technically allowed to do. But as per the following link, the C standard does permit compilers inventing writes except to atomics and volatiles: https://lwn.net/Articles/793253/#Invented%20Stores
> >> Even if this is right, it'd be rare, but it would be exceeding weird >> for a message to be logged listing an error and then future messages >> be logged without any such state, or with a random collection of >> garbage states. > How would that happen? The volatile keyword is only a compiler hint not > to do optimizations on the variable, what actually happens on the CPU > level depends if the instruction is locked or not, so different threads > may read different bits. > You seem to imply that once a variable is not used with volatile > semantics, even just for read, the result could lead to random writes > because it's otherwise undefined.
Pretty much; once a variable is read without READ_ONCE, it's unsafe to write a new value on another thread that depends on the old value. Imagine a compiler which invents stores; then if you are both reading and setting a variable 'a' on different threads, the following could happen:
thread 1 (reads) thread 2 (modifies)
reads a into tmp
stores junk into a
reads junk from a
stores tmp into a
writes junk | 2 to a
Now a contains junk indefinitely.
But if it's too theoretical, I'm happy to drop it and amend my paranoia level.
(Thanks for fixing the !CONFIG_PRINTK warning that btrfs_state_to_string was unused; sorry I missed it.)
Sweet Tea
| |