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

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2317 bytes --]



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?

Thanks,
Qu

> 
> Suggested-by: David Sterba <dsterba@suse.com>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/ctree.h |  5 +++++
>  fs/btrfs/super.c | 14 +++++++++++---
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 569931dd0ce5..625c7eee3d0f 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -110,6 +110,11 @@ struct btrfs_ref;
>  #define BTRFS_STAT_CURR		0
>  #define BTRFS_STAT_PREV		1
>  
> +/*
> + * Used when we know that fs_info is not yet initialized.
> + */
> +#define	NO_FS_INFO	((void *)0x1)
> +
>  /*
>   * Count how many BTRFS_MAX_EXTENT_SIZE cover the @size
>   */
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index a906315efd19..5bd8a889fed0 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -216,9 +216,17 @@ void __cold btrfs_printk(const struct btrfs_fs_info *fs_info, const char *fmt, .
>  	vaf.fmt = fmt;
>  	vaf.va = &args;
>  
> -	if (__ratelimit(ratelimit))
> -		printk("%sBTRFS %s (device %s): %pV\n", lvl, type,
> -			fs_info ? fs_info->sb->s_id : "<unknown>", &vaf);
> +	if (__ratelimit(ratelimit)) {
> +		if (fs_info == NULL)
> +			printk("%sBTRFS %s (device %s): %pV\n", lvl, type,
> +				"<unknown>", &vaf);
> +		else if (fs_info == NO_FS_INFO)
> +			printk("%sBTRFS %s (device %s): %pV\n", lvl, type,
> +				"...", &vaf);
> +		else
> +			printk("%sBTRFS %s (device %s): %pV\n", lvl, type,
> +				fs_info->sb->s_id, &vaf);
> +	}
>  
>  	va_end(args);
>  }
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]