Re: [PATCH] Btrfs: fix comp_oper to get right order

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 13, 2015 at 6:27 PM, Filipe David Manana <fdmanana@xxxxxxxxx> wrote:
> On Fri, Mar 13, 2015 at 12:37 PM, David Sterba <dsterba@xxxxxxx> wrote:
>> On Fri, Mar 13, 2015 at 02:24:38PM +0800, Liu Bo wrote:
>>> Case (oper1->seq > oper2->seq) should differ with case (oper1->seq < oper2->seq).
>>>
>>> Signed-off-by: Liu Bo <bo.li.liu@xxxxxxxxxx>
>>
>> Reviewed-by: David Sterba <dsterba@xxxxxxx>
> Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>
>
>>
>> and should go to stable@ if not to 4.1-rc as it's a bug in the core of
>> qgroups. Waiting for the next merge window would not buy us anything.
>
> Well, unless I missed something, it doesn't seem to cause any real
> harm as the only other search operation on the rbtree uses
> comp_oper_exist(), which totally ignores the seq field.

So that actually makes qgroup_oper_exists() wrong (unrelated to this
patch), which ignores the seq field. In other words it can miss
elements of the tree with the desired bytenr and ref root, since the
second criteria for insertion is the seq value and comp_oper_exist()
only uses bytenr and ref_root for comparing nodes / navigate the tree,
so if the tree has multiple nodes with the same bytenr but different
seqs, it can miss the fact that an operation for the given bytenr and
ref root exists.

>
> Of course it should be fixed anyway as it's logic error likely due to
> copy-paste, but unlikely to fix any real user visible problem.
>
>>
>>> ---
>>> This typo was hiden in a big patch, so we need to work harder on review.
>>
>> Of course, lack of reviews that come from non-maintainers is a perpetual
>> problem. There are never too many and each is highly appreciated.
>> --
>> 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
>
>
>
> --
> 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."



-- 
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




[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux