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