On Tue, Jun 30, 2015 at 5:17 AM, Davide Italiano <dccitaliano@xxxxxxxxx> wrote:
> On Mon, Jun 29, 2015 at 4:59 AM, Filipe David Manana <fdmanana@xxxxxxxxx> wrote:
>> On Sun, Jun 28, 2015 at 10:47 PM, Davide C. C. Italiano
>> <dccitaliano@xxxxxxxxx> wrote:
>>> From: Davide Italiano <dccitaliano@xxxxxxxxx>
>>>
>>> btrfs_insert_inode_ref() may fail and we want to make sure
>>> the transaction is aborted before calling btrfs_end_transaction(),
>>> as it already happens everywhere else in this function in case
>>> of error.
>>>
>>> Signed-off-by: Davide Italiano <dccitaliano@xxxxxxxxx>
>>> ---
>>> fs/btrfs/inode.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>>> index 8bb0136..59c475c 100644
>>> --- a/fs/btrfs/inode.c
>>> +++ b/fs/btrfs/inode.c
>>> @@ -9114,8 +9114,11 @@ static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry,
>>> new_dentry->d_name.len,
>>> old_ino,
>>> btrfs_ino(new_dir), index);
>>> - if (ret)
>>> + if (ret) {
>>> + btrfs_abort_transaction(trans, root, ret);
>>> goto out_fail;
>>> + }
>>> +
>>
>> Hi,
>>
>> I don't think we need a transaction abortion here. The reason it's not
>> being done is likely because at that point the trees are in a
>> consistent state (i.e. we haven't touched any of them yet) and not
>> because it was forgotten. So an abortion there is
>> unnecessary/excessive.
>>
>> thanks
>>
>
> Thank you for the comment -- I updated the other patch and I have
> mixed feeling about this one.
> I can either withdrawn the review or provide a new patch where I add a
> comment to clarify why this is not needed, for the future.
> Which one do you like better?
Hi,
I don't think it's needed. We do this pattern in many places and it's
quite obvious if one reads the code flow.
thanks
>
> --
> Davide
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
