On Tue, Jan 14, 2020 at 03:21:14PM +0800, Anand Jain wrote: > On 14/1/20 2:54 PM, Qu Wenruo wrote: > > On 2020/1/14 下午2:09, Anand Jain wrote: > >> The first argument to btrfs_printk() wrappers such as > >> btrfs_warn_in_rcu(), btrfs_info_in_rcu(), etc.. is fs_info, but in some > >> context like scan and assembling of the volumes there isn't fs_info yet, > >> so those code generally don't use the btrfs_printk() wrappers and it > >> could could still use NULL but then it would become hard to distinguish > >> whether fs_info is NULL for genuine reason or a bug. > >> > >> So introduce a define NO_FS_INFO to be used instead of NULL so that we > >> know the code where fs_info isn't initialized and also we have a > >> consistent logging functions. Thanks. > > > > I'm not sure why this is needed. > > > > Could you give me an example in which NULL is not clear enough? > > The first argument in btrfs_info_in_rcu() can be NULL like for example.. > btrfs_info_in_rcu(NULL, ..) which then it shall print the prefix.. > > BTRFS info (device <unknown>): > > Lets say due to some bug local copy of the variable fs_info wasn't > initialized then we end up printing the same unknown <unknown>. > > So in the context of device_list_add() as there is no fs_info > genuinely and be different from unknown we use > btrfs_info_in_rcu(NO_FS_INFO, ..) to get prefix something like.. > > BTRFS info (device ...): With the fixup to set fs_info to NULL on a device that's unmounted, do we still need the NO_FS_INFO stub? The only difference I can see is a to print "..." instead of "<unknown>" that I don't find too useful or improving the output. My idea about the stub fs info was to avoid any access to fs_info inside device_list_add in case we can't reliably close the race where scan can read device::fs_info during mount that sets it up, but as I'm told it's not a problem anymore.
