lkml.org 
[lkml]   [2022]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v4] btrfs: add fs state details to error messages.
From

> Added to misc-next with some minor updates, thanks.
>
Awesome, thank you. I realized this morning that it might be technically
slightly racy actually and would propose something like the following

static void btrfs_state_to_string(const struct btrfs_fs_info *info, char *buf)
{
unsigned int bit;
+ unsigned long fs_state = READ_ONCE(info->fs_state);
unsigned int states_printed = 0;
char *curr = buf;

memcpy(curr, STATE_STRING_PREFACE, sizeof(STATE_STRING_PREFACE));
curr += sizeof(STATE_STRING_PREFACE) - 1;

- for_each_set_bit(bit, &info->fs_state, sizeof(info->fs_state)) {
+ for_each_set_bit(bit, fs_state, sizeof(fs_state)) {


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.

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.

I can send another patch if this explanation seems reasonable and you'd like it separate; or maybe this is too unlikely to worry about. Thanks again for the review!

Sweet Tea

\
 
 \ /
  Last update: 2022-02-24 17:14    [W:0.182 / U:0.364 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site