Re: [PATCH 1/4] btrfs: add NO_FS_INFO to btrfs_printk

[David Sterba <dsterba@suse.cz>]

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.