From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH 1/2] btrfs: fix off-by-one end related to inode_need_compress()
Date: Thu, 10 Sep 2026 11:35:52 +0930 [thread overview]
Message-ID: <a617618bc5199f045759e47022e25ee053fd15ec.1789003160.git.wqu@suse.com> (raw)
In-Reply-To: <cover.1789003160.git.wqu@suse.com>
In most cases btrfs uses @end as the inclusive end bytenr for a range,
and this applies to inode_need_compress().
However we have several sites not following the inclusive bytenr:
- run_delalloc_inline()
Which assigned @blocksize as @end for inode_need_compress()
This makes inode_need_compress() always skip the disk_i_size check.
- heuristic_collect_sample()
Which assigned "start + BTRFS_MAX_UNCOMPRESSED" to @end, which is
the exclusive bytenr.
Neither is really causing any real problem, as
heuristic_collect_sample() has proper checks to avoid reading anything
beyond @end, and the sampling read size is 16 bytes, so it has enough
headroom to handle that off-by-one problem.
But still I do not like anything out of the common scheme, so fix the
off-by-one @end for both call sites, and add extra ASSERT()s to catch
such unaligned parameters.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/compression.c | 15 +++++++--------
fs/btrfs/inode.c | 5 ++++-
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 20169d028961..0134b9253702 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -1488,11 +1488,14 @@ static bool sample_repeated_patterns(struct heuristic_ws *ws)
static void heuristic_collect_sample(struct inode *inode, u64 start, u64 end,
struct heuristic_ws *ws)
{
+ const u32 blocksize = BTRFS_I(inode)->root->fs_info->sectorsize;
struct folio *folio;
pgoff_t index, index_end;
u32 i, curr_sample_pos;
u8 *in_data;
+ ASSERT(IS_ALIGNED(start, blocksize) && IS_ALIGNED(end + 1, blocksize));
+
/*
* Compression handles the input data by chunks of 128KiB
* (defined by BTRFS_MAX_UNCOMPRESSED)
@@ -1502,18 +1505,14 @@ static void heuristic_collect_sample(struct inode *inode, u64 start, u64 end,
* MAX_SAMPLE_SIZE - calculated under assumption that heuristic will
* process no more than BTRFS_MAX_UNCOMPRESSED at a time.
*/
- if (end - start > BTRFS_MAX_UNCOMPRESSED)
- end = start + BTRFS_MAX_UNCOMPRESSED;
+ if (end + 1 - start > BTRFS_MAX_UNCOMPRESSED)
+ end = start + BTRFS_MAX_UNCOMPRESSED - 1;
index = start >> PAGE_SHIFT;
index_end = end >> PAGE_SHIFT;
- /* Don't miss unaligned end */
- if (!PAGE_ALIGNED(end))
- index_end++;
-
curr_sample_pos = 0;
- while (index < index_end) {
+ while (index <= index_end) {
folio = filemap_get_folio(inode->i_mapping, index);
ASSERT(!IS_ERR(folio));
in_data = kmap_local_folio(folio,
@@ -1522,7 +1521,7 @@ static void heuristic_collect_sample(struct inode *inode, u64 start, u64 end,
i = start % PAGE_SIZE;
while (i < PAGE_SIZE - SAMPLING_READ_SIZE) {
/* Don't sample any garbage from the last page */
- if (start > end - SAMPLING_READ_SIZE)
+ if (start > end + 1 - SAMPLING_READ_SIZE)
break;
memcpy(&ws->sample[curr_sample_pos], &in_data[i],
SAMPLING_READ_SIZE);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index a85a7c561cf8..79f2181dc631 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -730,6 +730,9 @@ static inline int inode_need_compress(struct btrfs_inode *inode, u64 start,
u64 end, bool check_inline)
{
struct btrfs_fs_info *fs_info = inode->root->fs_info;
+ const u32 blocksize = fs_info->sectorsize;
+
+ ASSERT(IS_ALIGNED(start, blocksize) && IS_ALIGNED(end + 1, blocksize));
if (unlikely(!btrfs_inode_can_compress(inode))) {
DEBUG_WARN("BTRFS: unexpected compression for ino %llu", btrfs_ino(inode));
@@ -2331,7 +2334,7 @@ static int run_delalloc_inline(struct btrfs_inode *inode, struct folio *locked_f
btrfs_check_folio_write_protected(locked_folio);
if (btrfs_inode_can_compress(inode) &&
- inode_need_compress(inode, 0, blocksize, true)) {
+ inode_need_compress(inode, 0, blocksize - 1, true)) {
if (inode->defrag_compress > 0 &&
inode->defrag_compress < BTRFS_NR_COMPRESS_TYPES) {
compress_type = inode->defrag_compress;
--
2.55.0
next prev parent reply other threads:[~2026-09-10 2:06 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-10 2:05 [PATCH 0/2] btrfs: fix and simplify heuristic_collect_sample() Qu Wenruo
2026-09-10 2:05 ` Qu Wenruo [this message]
2026-09-10 2:05 ` [PATCH 2/2] btrfs: simplify heuristic_collect_sample() to handle large folios better Qu Wenruo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a617618bc5199f045759e47022e25ee053fd15ec.1789003160.git.wqu@suse.com \
--to=wqu@suse.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.