lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <29ec66bd-27a0-443e-b19b-fb759a847dcb@suse.com>
Date: Wed, 5 Mar 2025 18:14:16 +1030
From: Qu Wenruo <wqu@...e.com>
To: Daniel Vacek <neelx@...e.com>, Qu Wenruo <quwenruo.btrfs@....com>
Cc: Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
 David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] btrfs/defrag: implement compression levels



在 2025/3/5 17:32, Daniel Vacek 写道:
> On Tue, 4 Mar 2025 at 22:31, Qu Wenruo <quwenruo.btrfs@....com> wrote:
[...]
>>
>> I am not familiar with the compress level, but
>> btrfs_compress_set_level() does extra clamping, maybe we also want to do
>> that too?
> 
> This is intentionally left to be limited later. There's no need to do
> it at this point and the code is simpler. It's also compression
> type/method agnostic.

You're right, the level checks are done in the compression path already.

[...]
>>>        /* spare for later */
>>>        __u32 unused[4];
>>
>> We have enough space left here, although u32 is overkilled for
>> compress_type, using the unused space for a new s8/s16/s32 member should
>> be fine.
> 
> That is what I did originally, but discussing with Dave he suggested
> this solution.

Normally I would be fine with the union, to save some memory.

Maybe I'm a little paranoid, but the defrag ioctl flag check is only 
introduced last year by commit 173431b274a9 ("btrfs: defrag: reject 
unknown flags of btrfs_ioctl_defrag_range_args").

So it's possible that some older kernels don't have that commit, and may 
incorrectly continue by ignoring the flag.
Thankfully that should fail with -EINVAL (type always in the higher 
bits, thus always tricking the NR_COMPRESS_TYPES check.

If that layout (type in higher bits, level in lower bits) is 
intentionally, I'd say it's very clever.

Anyway either solution looks fine to me now.

With that commit message fixed:

Reviewed-by: Qu Wenruo <wqu@...e.com>

Thanks,
Qu
> 
>>
>> Thanks,
>> Qu
> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ