Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 23 Feb 2022 16:24:05 +0100
From:   David Sterba <dsterba@suse.cz>
To:     Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
Cc:     Chris Mason <clm@fb.com>, Josef Bacik <josef@toxicpanda.com>,
        David Sterba <dsterba@suse.com>, linux-btrfs@vger.kernel.org,
        linux-kernel@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH v3] btrfs: add fs state details to error messages.
Message-ID: <20220223152405.GO12643@twin.jikos.cz>
Reply-To: dsterba@suse.cz
Mail-Followup-To: dsterba@suse.cz,
        Sweet Tea Dorminy <sweettea-kernel@dorminy.me>,
        Chris Mason <clm@fb.com>, Josef Bacik <josef@toxicpanda.com>,
        David Sterba <dsterba@suse.com>, linux-btrfs@vger.kernel.org,
        linux-kernel@vger.kernel.org, kernel-team@fb.com
References: <8a2a73ab4b48a4e73d24cf7f10cc0fe245d50a84.1645562216.git.sweettea-kernel@dorminy.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <8a2a73ab4b48a4e73d24cf7f10cc0fe245d50a84.1645562216.git.sweettea-kernel@dorminy.me>
User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12)
Precedence: bulk

On Tue, Feb 22, 2022 at 03:42:28PM -0500, Sweet Tea Dorminy wrote:
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -66,6 +66,46 @@ static struct file_system_type btrfs_root_fs_type;
>  
>  static int btrfs_remount(struct super_block *sb, int *flags, char *data);
>  
> +#define STATE_STRING_PREFACE ": state "
> +#define STATE_STRING_BUF_LEN \
> +	(sizeof(STATE_STRING_PREFACE) + BTRFS_FS_STATE_COUNT)
> +
> +/* Characters to print to indicate error conditions. RO is not an error. */
> +static const char * const fs_state_strings[] = {
> +	[BTRFS_FS_STATE_ERROR]			= "E",
> +	[BTRFS_FS_STATE_REMOUNTING]		= "M",
> +	[BTRFS_FS_STATE_RO]			= NULL,
> +	[BTRFS_FS_STATE_TRANS_ABORTED]		= "A",
> +	[BTRFS_FS_STATE_DEV_REPLACING]		= "P",
> +	[BTRFS_FS_STATE_DUMMY_FS_INFO]		= NULL,
> +	[BTRFS_FS_STATE_NO_CSUMS]		= NULL,
> +	[BTRFS_FS_STATE_LOG_CLEANUP_ERROR]	= "L",

Yeah that's the idea with the table, but I think you don't need to use
strings, it should be sufficient to use chars, and 0 works for the empty
ones.  The way you did it consumes more memory and has indirection with
the pointers to the actual single letter strings.

I'm not sure if we want the non-error states like remounting or
replacing, but actually why not, even if it's a transient state it's
another piece of information that could be useful eventually.