On Fri, Jan 12, 2018 at 04:21:05PM +0200, Nikolay Borisov wrote:
> @@ -6062,19 +6062,19 @@ int btrfs_delalloc_reserve_metadata(struct 
> btrfs_inode *inode, u64 num_bytes)
>        * If we have a transaction open (can happen if we call truncate_block
>        * from truncate), then we need FLUSH_LIMIT so we don't deadlock.
>        */
> +
>       if (btrfs_is_free_space_inode(inode)) {
>               flush = BTRFS_RESERVE_NO_FLUSH;
>               delalloc_lock = false;
> -     } else if (current->journal_info) {
> -             flush = BTRFS_RESERVE_FLUSH_LIMIT;
> -     }
> +     } else {
> +             if (current->journal_info)
> +                     flush = BTRFS_RESERVE_FLUSH_LIMIT;
>  
> -     if (flush != BTRFS_RESERVE_NO_FLUSH &&
> -         btrfs_transaction_in_commit(fs_info))
> -             schedule_timeout(1);
> +             if (btrfs_transaction_in_commit(fs_info))
> +                     schedule_timeout(1);
>  
> -     if (delalloc_lock)
>               mutex_lock(&inode->delalloc_mutex);
> +     }

Squeezing the condition branches makes the code more readable, I have
only one objection and it's the mutex_lock. It IMHO looks better when
it's a separate branch as it pairs with the unlock:

if (delalloc_lock)
        mutex_lock(...);

...

if (delalloc_lock)
        mutex_unlock(...);

In your version it's implied by the first if that checks
btrfs_is_free_space_inode and delalloc_lock is hidden there.

>  
>       num_bytes = ALIGN(num_bytes, fs_info->sectorsize);
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to