| Message ID | 4a76fdcba3b1707714fa6b08f28f955c35cae0ee.1744063163.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | btrfs: prepare compreesion paths for large data folios | expand |
On Mon, Apr 7, 2025 at 11:05 PM Qu Wenruo <wqu@suse.com> wrote: > > All compression algorithms inside btrfs are not supporting > large folios due to the following points: > > - btrfs_calc_input_length() is assuming page sized folio > > - kmap_local_folio() usages are using offset_in_page() > > Prepare them to support large data folios by: > > - Add a folio parameter to btrfs_calc_input_length() > And use that folio parameter to calculate the correct length. > > Since we're here, also add extra ASSERT()s to make sure the parameter > @cur is inside the folio range. > > This affects only zlib and zstd. Lzo compresses at most one block one > time, thus not affected. "one block one time" -> "one block at a time" > > - Use offset_in_folio() to calculate the kmap_local_folio() offset > This affects all 3 algorithms. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > This is exposed by the fstests run btrfs/138 with large data folios > enabled, which verify the contents of compressed extents and found out > that the compressed data is all duplication of the first page. > > That's the only new regression exposed during my large data folios runs. > The final enablement patch will be sent when all dependent patches are > mreged. > --- > fs/btrfs/compression.h | 12 +++++++++--- > fs/btrfs/lzo.c | 5 ++--- > fs/btrfs/zlib.c | 7 +++---- > fs/btrfs/zstd.c | 8 ++++---- > 4 files changed, 18 insertions(+), 14 deletions(-) > > diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h > index df198623cc08..d3a61d5fb425 100644 > --- a/fs/btrfs/compression.h > +++ b/fs/btrfs/compression.h > @@ -11,7 +11,9 @@ > #include <linux/list.h> > #include <linux/workqueue.h> > #include <linux/wait.h> > +#include <linux/pagemap.h> > #include "bio.h" > +#include "messages.h" > > struct address_space; > struct page; > @@ -73,11 +75,15 @@ struct compressed_bio { > }; > > /* @range_end must be exclusive. */ > -static inline u32 btrfs_calc_input_length(u64 range_end, u64 cur) > +static inline u32 btrfs_calc_input_length(struct folio *folio, > + u64 range_end, u64 cur) > { > - u64 page_end = round_down(cur, PAGE_SIZE) + PAGE_SIZE; > + u64 folio_end = folio_pos(folio) + folio_size(folio); Can be made const. Otherwise it looks fine. Reviewed-by: Filipe Manana <fdmanana@suse.com> Thanks. > > - return min(range_end, page_end) - cur; > + /* @cur must be inside the folio. */ > + ASSERT(folio_pos(folio) <= cur); > + ASSERT(cur < folio_end); > + return min(range_end, folio_end) - cur; > } > > int __init btrfs_init_compress(void); > diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c > index a45bc11f8665..d403641889ca 100644 > --- a/fs/btrfs/lzo.c > +++ b/fs/btrfs/lzo.c > @@ -252,9 +252,8 @@ int lzo_compress_folios(struct list_head *ws, struct address_space *mapping, > /* Compress at most one sector of data each time */ > in_len = min_t(u32, start + len - cur_in, sectorsize - sector_off); > ASSERT(in_len); > - data_in = kmap_local_folio(folio_in, 0); > - ret = lzo1x_1_compress(data_in + > - offset_in_page(cur_in), in_len, > + data_in = kmap_local_folio(folio_in, offset_in_folio(folio_in, cur_in)); > + ret = lzo1x_1_compress(data_in, in_len, > workspace->cbuf, &out_len, > workspace->mem); > kunmap_local(data_in); > diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c > index b32aa05b288e..f8cd9d6d7e37 100644 > --- a/fs/btrfs/zlib.c > +++ b/fs/btrfs/zlib.c > @@ -203,7 +203,6 @@ int zlib_compress_folios(struct list_head *ws, struct address_space *mapping, > workspace->strm.next_in = workspace->buf; > workspace->strm.avail_in = copy_length; > } else { > - unsigned int pg_off; > unsigned int cur_len; > > if (data_in) { > @@ -215,9 +214,9 @@ int zlib_compress_folios(struct list_head *ws, struct address_space *mapping, > start, &in_folio); > if (ret < 0) > goto out; > - pg_off = offset_in_page(start); > - cur_len = btrfs_calc_input_length(orig_end, start); > - data_in = kmap_local_folio(in_folio, pg_off); > + cur_len = btrfs_calc_input_length(in_folio, orig_end, start); > + data_in = kmap_local_folio(in_folio, > + offset_in_folio(in_folio, start)); > start += cur_len; > workspace->strm.next_in = data_in; > workspace->strm.avail_in = cur_len; > diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c > index cd5f38d6fbaa..b20aeaf12424 100644 > --- a/fs/btrfs/zstd.c > +++ b/fs/btrfs/zstd.c > @@ -426,8 +426,8 @@ int zstd_compress_folios(struct list_head *ws, struct address_space *mapping, > ret = btrfs_compress_filemap_get_folio(mapping, start, &in_folio); > if (ret < 0) > goto out; > - cur_len = btrfs_calc_input_length(orig_end, start); > - workspace->in_buf.src = kmap_local_folio(in_folio, offset_in_page(start)); > + cur_len = btrfs_calc_input_length(in_folio, orig_end, start); > + workspace->in_buf.src = kmap_local_folio(in_folio, offset_in_folio(in_folio, start)); > workspace->in_buf.pos = 0; > workspace->in_buf.size = cur_len; > > @@ -511,9 +511,9 @@ int zstd_compress_folios(struct list_head *ws, struct address_space *mapping, > ret = btrfs_compress_filemap_get_folio(mapping, start, &in_folio); > if (ret < 0) > goto out; > - cur_len = btrfs_calc_input_length(orig_end, start); > + cur_len = btrfs_calc_input_length(in_folio, orig_end, start); > workspace->in_buf.src = kmap_local_folio(in_folio, > - offset_in_page(start)); > + offset_in_folio(in_folio, start)); > workspace->in_buf.pos = 0; > workspace->in_buf.size = cur_len; > } > -- > 2.49.0 > >
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h index df198623cc08..d3a61d5fb425 100644 --- a/fs/btrfs/compression.h +++ b/fs/btrfs/compression.h @@ -11,7 +11,9 @@ #include <linux/list.h> #include <linux/workqueue.h> #include <linux/wait.h> +#include <linux/pagemap.h> #include "bio.h" +#include "messages.h" struct address_space; struct page; @@ -73,11 +75,15 @@ struct compressed_bio { }; /* @range_end must be exclusive. */ -static inline u32 btrfs_calc_input_length(u64 range_end, u64 cur) +static inline u32 btrfs_calc_input_length(struct folio *folio, + u64 range_end, u64 cur) { - u64 page_end = round_down(cur, PAGE_SIZE) + PAGE_SIZE; + u64 folio_end = folio_pos(folio) + folio_size(folio); - return min(range_end, page_end) - cur; + /* @cur must be inside the folio. */ + ASSERT(folio_pos(folio) <= cur); + ASSERT(cur < folio_end); + return min(range_end, folio_end) - cur; } int __init btrfs_init_compress(void); diff --git a/fs/btrfs/lzo.c b/fs/btrfs/lzo.c index a45bc11f8665..d403641889ca 100644 --- a/fs/btrfs/lzo.c +++ b/fs/btrfs/lzo.c @@ -252,9 +252,8 @@ int lzo_compress_folios(struct list_head *ws, struct address_space *mapping, /* Compress at most one sector of data each time */ in_len = min_t(u32, start + len - cur_in, sectorsize - sector_off); ASSERT(in_len); - data_in = kmap_local_folio(folio_in, 0); - ret = lzo1x_1_compress(data_in + - offset_in_page(cur_in), in_len, + data_in = kmap_local_folio(folio_in, offset_in_folio(folio_in, cur_in)); + ret = lzo1x_1_compress(data_in, in_len, workspace->cbuf, &out_len, workspace->mem); kunmap_local(data_in); diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c index b32aa05b288e..f8cd9d6d7e37 100644 --- a/fs/btrfs/zlib.c +++ b/fs/btrfs/zlib.c @@ -203,7 +203,6 @@ int zlib_compress_folios(struct list_head *ws, struct address_space *mapping, workspace->strm.next_in = workspace->buf; workspace->strm.avail_in = copy_length; } else { - unsigned int pg_off; unsigned int cur_len; if (data_in) { @@ -215,9 +214,9 @@ int zlib_compress_folios(struct list_head *ws, struct address_space *mapping, start, &in_folio); if (ret < 0) goto out; - pg_off = offset_in_page(start); - cur_len = btrfs_calc_input_length(orig_end, start); - data_in = kmap_local_folio(in_folio, pg_off); + cur_len = btrfs_calc_input_length(in_folio, orig_end, start); + data_in = kmap_local_folio(in_folio, + offset_in_folio(in_folio, start)); start += cur_len; workspace->strm.next_in = data_in; workspace->strm.avail_in = cur_len; diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c index cd5f38d6fbaa..b20aeaf12424 100644 --- a/fs/btrfs/zstd.c +++ b/fs/btrfs/zstd.c @@ -426,8 +426,8 @@ int zstd_compress_folios(struct list_head *ws, struct address_space *mapping, ret = btrfs_compress_filemap_get_folio(mapping, start, &in_folio); if (ret < 0) goto out; - cur_len = btrfs_calc_input_length(orig_end, start); - workspace->in_buf.src = kmap_local_folio(in_folio, offset_in_page(start)); + cur_len = btrfs_calc_input_length(in_folio, orig_end, start); + workspace->in_buf.src = kmap_local_folio(in_folio, offset_in_folio(in_folio, start)); workspace->in_buf.pos = 0; workspace->in_buf.size = cur_len; @@ -511,9 +511,9 @@ int zstd_compress_folios(struct list_head *ws, struct address_space *mapping, ret = btrfs_compress_filemap_get_folio(mapping, start, &in_folio); if (ret < 0) goto out; - cur_len = btrfs_calc_input_length(orig_end, start); + cur_len = btrfs_calc_input_length(in_folio, orig_end, start); workspace->in_buf.src = kmap_local_folio(in_folio, - offset_in_page(start)); + offset_in_folio(in_folio, start)); workspace->in_buf.pos = 0; workspace->in_buf.size = cur_len; }
All compression algorithms inside btrfs are not supporting large folios due to the following points: - btrfs_calc_input_length() is assuming page sized folio - kmap_local_folio() usages are using offset_in_page() Prepare them to support large data folios by: - Add a folio parameter to btrfs_calc_input_length() And use that folio parameter to calculate the correct length. Since we're here, also add extra ASSERT()s to make sure the parameter @cur is inside the folio range. This affects only zlib and zstd. Lzo compresses at most one block one time, thus not affected. - Use offset_in_folio() to calculate the kmap_local_folio() offset This affects all 3 algorithms. Signed-off-by: Qu Wenruo <wqu@suse.com> --- This is exposed by the fstests run btrfs/138 with large data folios enabled, which verify the contents of compressed extents and found out that the compressed data is all duplication of the first page. That's the only new regression exposed during my large data folios runs. The final enablement patch will be sent when all dependent patches are mreged. --- fs/btrfs/compression.h | 12 +++++++++--- fs/btrfs/lzo.c | 5 ++--- fs/btrfs/zlib.c | 7 +++---- fs/btrfs/zstd.c | 8 ++++---- 4 files changed, 18 insertions(+), 14 deletions(-)