Re: [PATCH 2/2 v10] Btrfs: be aware of btree inode write errors to avoid fs corruption

[David Sterba <dsterba@suse.cz>]

On Thu, Sep 25, 2014 at 05:51:45PM +0100, Filipe Manana wrote:
> +	switch (eb->log_index) {
> +	case -1:
> +		set_bit(BTRFS_INODE_BTREE_ERR, &btree_ino->runtime_flags);
> +		break;
> +	case 0:
> +		set_bit(BTRFS_INODE_BTREE_LOG1_ERR, &btree_ino->runtime_flags);
> +		break;
> +	case 1:
> +		set_bit(BTRFS_INODE_BTREE_LOG2_ERR, &btree_ino->runtime_flags);
> +		break;
> +	default:
> +		ASSERT(0); /* unexpected, logic error */

I think we can afford a BUG() here, preceded by a message. The logic is
pretty strict about the possible values so if we get here, something is
really bad and we don't want to make it silent.

This could happen if the memory modules are faulty and tripping over
random BUG_ONs is usually a sign of problems. I've seen that in the
past with certain CPU types during stresstests and it definetely was
worth knowing. (The BUG_ONs were in memory management, it's not a crime
to use them, only in place of proper error hanling.)

> --- a/fs/btrfs/extent_io.h
> +++ b/fs/btrfs/extent_io.h
> @@ -141,7 +142,9 @@ struct extent_buffer {
>  	atomic_t blocking_readers;
>  	atomic_t spinning_readers;
>  	atomic_t spinning_writers;
> -	int lock_nested;
> +	unsigned int lock_nested:1;

Please do not use bitfields next to atomic_t or spin_lock (or kref_t),
this can lead to all sorts of funny problems on architectures that are
not able to do atomic bit operations and have to do read-modify-write
cycle. This leads to problems when it's on the same 8-byte word with an
atomic/... and can corrupt it if the timing is right.