btrfs: used compressed_bio structure for read and write
Subsystems (2)
v5 cover btrfs: used compressed_bio structure for read and write
[CHANGELOG]
v5:
- Implement an internal compress_bio_last_folio()
Upstream is removing bio_last_bvec_all(), and the recent stable page bouncing for direct IO is going to make that function no longer reliable.
Thus we need to implement it locally, with some extra ASSERT()s to make sure the compressed bio have all folio sizes fixed to min_folio_size, so that we can use offset_in_folio(folio, bi_size) to calculate the last position.
- Fix several spelling errors
jut -> just in patch 5 and chaning -> changing in patch 7.
v4:
- Fix a missing kunmap_local() introduced in encoded write changes Which only affects 32bit systems.
- Fix an incorrect unlikely() usage introduced in encoded write changes Which will not cause any observable problem but completely flips the intention.
- Fix an allocation loop bug for bs > ps cases in encoded write changes The encoded write has migrated to use btrfs_alloc_compr_folio() which respects the minimal folio size.
This means the old PAGE_SIZE based loop condition must also be changed.
v3:
- Rebased to the latest for-next branch
There is a minor conflict with the format of parameter list of get_current_folio().
- Call btrfs_free_compr_folio() during cleanup_compressed_bio()
- Make it more consistent regarding using btrfs_alloc/free_compr_folio() The old compressed read path is using btrfs_alloc_folio_array() but freeing them using btrfs_free_compr_folio().
This is fine but never consistent, unify it to use btrfs_free_compr_folio() for all compressed_bio.
v2:
- Fix an error in error path of of compress_file_range()
If btrfs_compress_bio() returned an error, we should reset @cb to NULL before doing error handling, or cleanup_compressed_bio() can be called on an error pointer.
- Fix several bugs in zstd_compress_bio() which causes compression failure
There are several different bugs in the mostly copy-n-pasted code:
* Uncommon @start and @len usage
The old code allows @start and @len to be modified, which is against the more common practice nowadays.
* Fix a incorrect input buffer check
Which cause us to incorrectly end the compression early and fallback to uncompressed write.
I was never a huge fan of the current btrfs_compress_folios() interface:
- Complex and duplicated parameter list
* A folio array to hold all folios
Which means extra error handling.
* A @nr_folios pointer
That pointer is both input and output, representing the number of max folios, but also the number of compressed folios.
The number of input folios is not really necessary, it's always no larger than DIV_ROUND_UP(len, PAGE_SIZE) in the first place.
* A @total_in pointer
Again an pointer as both input and output, representing the filemap range length, and how many bytes are compressed in this run.
However if we failed to compress the full range, all supported algorithms will return an error, thus fallback to uncompressed path.
Thus there is no need to use it as an output pointer.
* A @total_compressed point
Again an pointer as both input and output, representing the max number of compressed size, and the final compressed size.
However we do not need it as an input at all, we always error out if the compressed size is larger than the original size.
- Extra error cleanup handling
We need to cleanup the compressed_folios[] array during error handling.
Replace the old btrfs_compress_folios() interface with btrfs_compress_bio(), which has the following benefits:
- Simplified parameter list
* inode
* start
* len
* compress_type
* compress_level
* write_flags
No parameter is sharing input and output members, and all are very straightforward (except the last write_flags, which is just an extra bio flag).
- Directly return a compressed_bio structure
With minor modifications, that pointer can be passed to btrfs_submit_bio().
The caller still needs to do proper round up and fill the proper disk_bytenr/num_bytes before submission.
And for error handling, simply call cleanup_compressed_bio() then everything is cleaned up properly (at least I hope so).
- No more extra folios array passing and error handling
Qu Wenruo (9):
btrfs: introduce lzo_compress_bio() helper
btrfs: introduce zstd_compress_bio() helper
btrfs: introduce zlib_compress_bio() helper
btrfs: introduce btrfs_compress_bio() helper
btrfs: switch to btrfs_compress_bio() interface for compressed writes
btrfs: remove the old btrfs_compress_folios() infrastructures
btrfs: get rid of compressed_folios[] usage for compressed read
btrfs: get rid of compressed_folios[] usage for encoded writes
btrfs: get rid of compressed_bio::compressed_folios[]
fs/btrfs/compression.c | 208 ++++++++++++++++------------------ fs/btrfs/compression.h | 40 ++++--- fs/btrfs/inode.c | 251 ++++++++++++++++++++++------------------- fs/btrfs/lzo.c | 223 ++++++++++++++++++++++++------------ fs/btrfs/zlib.c | 64 ++++++----- fs/btrfs/zstd.c | 98 ++++++++-------- 6 files changed, 482 insertions(+), 402 deletions(-)
Signature · 1 line
2.52.0
PATCH v5 1/9 btrfs: introduce lzo_compress_bio() helper
The new helper has the following enhancements against the existing lzo_compress_folios()
- Much smaller parameter list
No more shared IN/OUT members, no need to pre-allocate a compressed_folios[] array.
Just a workspace list header and a compressed_bio pointer.
Everything else can be fetched from that @cb pointer.
- Read-to-be-submitted compressed bio
Although the caller still needs to do some common works like rounding up and zeroing the tailing part of the last fs block.
Some workloads are specific to lzo that is not needed with other multi-run compression interfaces:
- Need to write a LZO header or segment header
Use the new write_and_queue_folio() helper to do the bio_add_folio() call and folio switching.
- Need to update the LZO header after compression is done
Use bio_first_folio_all() to grab the first folio and update the header.
- Extra corner case of error handling
This can happen when we have queued part of a folio and hit an error. In that case those folios will be released by the bio. Thus we can only release the folio that has no queued part.
--- fs/btrfs/compression.h | 1 + fs/btrfs/lzo.c | 261 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 262 insertions(+)
fs/btrfs/compression.h +1 -0
@@ -161,6 +161,7 @@ struct list_head *zlib_get_workspace(struct btrfs_fs_info *fs_info, unsigned intint lzo_compress_folios (struct list_head *ws,struct btrfs_inode *inode,u64 start,struct folio **folios,unsigned long *out_folios,unsigned long *total_in,unsigned long *total_out); +int lzo_compress_bio (struct list_head *ws,struct compressed_bio *cb);int lzo_decompress_bio (struct list_head *ws,struct compressed_bio *cb); int lzo_decompress(struct list_head *ws, const u8 *data_in, struct folio *dest_folio, unsigned long dest_pgoff, size_t srclen,
fs/btrfs/lzo.c +261 -0
273 lines not on this page · raw message
Signature · 1 line
2.52.0
PATCH v5 2/9 btrfs: introduce zstd_compress_bio() helper
In reply to Qu WenruoThe new helper has the following enhancements against the existing zstd_compress_folios()
- Much smaller parameter list
No more shared IN/OUT members, no need to pre-allocate a compressed_folios[] array.
Just a workspace and compressed_bio pointer, everything we need can be extracted from that @cb pointer.
- Ready-to-be-submitted compressed bio
Although the caller still needs to do some common works like rounding up and zeroing the tailing part of the last fs block.
Overall the workflow is the same as zstd_compress_folios(), but with some minor changes:
- @start/@len is now constant
For the current input file offset, use @start + @tot_in instead.
The original change of @start and @len makes it pretty hard to know what value we're really comparing to.
- No more @cur_len
It's only utilized when switching input buffer.
Directly use btrfs_calc_input_length() instead.
--- fs/btrfs/compression.h | 1 + fs/btrfs/zstd.c | 185 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+)
fs/btrfs/compression.h +1 -0
@@ -172,6 +172,7 @@ void lzo_free_workspace(struct list_head *ws);int zstd_compress_folios (struct list_head *ws,struct btrfs_inode *inode,u64 start,struct folio **folios,unsigned long *out_folios,unsigned long *total_in,unsigned long *total_out); +int zstd_compress_bio (struct list_head *ws,struct compressed_bio *cb);int zstd_decompress_bio (struct list_head *ws,struct compressed_bio *cb); int zstd_decompress(struct list_head *ws, const u8 *data_in, struct folio *dest_folio, unsigned long dest_pgoff, size_t srclen,
fs/btrfs/zstd.c +185 -0
191 lines not on this page · raw message
Signature · 1 line
2.52.0
PATCH v5 3/9 btrfs: introduce zlib_compress_bio() helper
In reply to Qu WenruoThe new helper has the following enhancements against the existing zlib_compress_folios()
- Much smaller parameter list
No more shared IN/OUT members, no need to pre-allocate a compressed_folios[] array.
Just a workspace and compressed_bio pointer, everything we need can be extracted from that @cb pointer.
- Ready-to-be-submitted compressed bio
Although the caller still needs to do some common works like rounding up and zeroing the tailing part of the last fs block.
--- fs/btrfs/compression.h | 1 + fs/btrfs/zlib.c | 193 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+)
fs/btrfs/compression.h +1 -0
@@ -150,6 +150,7 @@ int btrfs_compress_filemap_get_folio(struct address_space *mapping, u64 start,int zlib_compress_folios (struct list_head *ws,struct btrfs_inode *inode,u64 start,struct folio **folios,unsigned long *out_folios,unsigned long *total_in,unsigned long *total_out); +int zlib_compress_bio (struct list_head *ws,struct compressed_bio *cb);int zlib_decompress_bio (struct list_head *ws,struct compressed_bio *cb); int zlib_decompress(struct list_head *ws, const u8 *data_in, struct folio *dest_folio, unsigned long dest_pgoff, size_t srclen,
fs/btrfs/zlib.c +193 -0
199 lines not on this page · raw message
Signature · 1 line
2.52.0
PATCH v5 4/9 btrfs: introduce btrfs_compress_bio() helper
In reply to Qu WenruoThe helper will allocate a new compressed_bio, do the compression, and return it to the caller.
This greatly simplifies the compression path, as we no longer need to allocate a folio array thus no extra error path, furthermore the compressed bio structure can be utilized for submission with very minor modifications (like rounding up the bi_size and populate the bi_sector).
--- fs/btrfs/compression.c | 68 ++++++++++++++++++++++++++++++++++++++++++ fs/btrfs/compression.h | 13 ++++++++ 2 files changed, 81 insertions(+)
fs/btrfs/compression.c +68 -0
@@ -1064,6 +1064,74 @@ int btrfs_compress_folios(unsigned int type, int level, struct btrfs_inode *inodreturn ret; } +/* +* Given an address space and start and length, compress the page cache +* contents into @cb. +* +* @type_level is encoded algorithm and level, where level 0 means whatever +* default the algorithm chooses and is opaque here; +* - compression algo are 0-3 +* - the level are bits 4-7 +* +* @cb->bbio.bio.bi_iter.bi_size will indicate the compressed data size. +* The bi_size may not be sectorsize aligned, thus the caller still need +* to do the round up before submission. +* +* This function will allocate compressed folios with btrfs_alloc_compr_folio(), +* thus callers must make sure the endio function and error handling are using +* btrfs_free_compr_folio() to release those folios. +* This is already done in end_bbio_compressed_write() and cleanup_compressed_bio(). +*/ +struct compressed_bio *btrfs_compress_bio (struct btrfs_inode *inode, +u64 start,u32 len,unsigned int type, +int level,blk_opf_t write_flags) +{ +struct btrfs_fs_info *fs_info = inode->root->fs_info; +struct list_head *workspace; +struct compressed_bio *cb; +int ret; + + cb =alloc_compressed_bio (inode, start,REQ_OP_WRITE | write_flags, + end_bbio_compressed_write); + cb->start = start; + cb->len = len; + cb->writeback = true; + cb->compress_type = type; + + level =btrfs_compress_set_level (type, level); + workspace =get_workspace (fs_info, type, level); +switch (type) { +case BTRFS_COMPRESS_ZLIB : + ret =zlib_compress_bio (workspace, cb); +break ; +case BTRFS_COMPRESS_LZO : + ret =lzo_compress_bio (workspace, cb); +break ; +case BTRFS_COMPRESS_ZSTD : + ret =zstd_compress_bio (workspace, cb); +break ; +case BTRFS_COMPRESS_NONE : +default : +/* +* This can happen when compression races with remount setting +* it to 'no compress', while caller doesn't call +* inode_need_compress() to check if we really need to +* compress. +* +* Not a big deal, just need to inform caller that we +* haven't allocated any pages yet. +*/ + ret = -E2BIG ; + } + +put_workspace (fs_info, type, workspace); +if (ret <0 ) { +cleanup_compressed_bio (cb); +return ERR_PTR (ret); + } +return cb; +} +static int btrfs_decompress_bio (struct compressed_bio *cb) {struct btrfs_fs_info *fs_info =cb_to_fs_info (cb);
fs/btrfs/compression.h +13 -0
@@ -146,6 +146,19 @@ int btrfs_compress_heuristic(struct btrfs_inode *inode, u64 start, u64 end);int btrfs_compress_filemap_get_folio (struct address_space *mapping,u64 start,struct folio **in_folio_ret); +struct compressed_bio *btrfs_compress_bio (struct btrfs_inode *inode, +u64 start,u32 len,unsigned int type, +int level,blk_opf_t write_flags); + +static inline void cleanup_compressed_bio (struct compressed_bio *cb) +{ +struct bio *bio = &cb->bbio.bio; +struct folio_iter fi; + +bio_for_each_folio_all (fi, bio) +btrfs_free_compr_folio (fi.folio); +bio_put (bio); +} int zlib_compress_folios(struct list_head *ws, struct btrfs_inode *inode, u64 start, struct folio **folios, unsigned long *out_folios,
Signature · 1 line
2.52.0
PATCH v5 5/9 btrfs: switch to btrfs_compress_bio() interface for compressed writes
In reply to Qu WenruoThis switch has the following benefits:
- A single structure to handle all compression
No more extra members like compressed_folios[] nor compress_type, all those members.
This means the structure of async_extent is much smaller.
- Simpler error handling
A single cleanup_compressed_bio() will handle everything, no extra compressed_folios[] array to bother.
Some special notes:
- Compressed folios releasing
Now we go bio_for_each_folio_all() loop to release the folios of the bio. This will work for both the old compressed_folios[] array and the new pure bio method.
For old compressed_folios[], all folios of that array is queued into the bio, thus releasing the folios from the bio is the same as releasing each folio of that array. We just need to be sure no double releasing from the array and bio.
For the new pure bio method, that array is NULL, just usual folio releasing of the bio.
The only extra note is for end_bbio_compressed_read(), as the folios are allocated using btrfs_alloc_folio_array(), thus the folios should only be released by regular folio_put(), not btrfs_free_compr_folio().
- Rounding up the bio to block size
We can not simply increase bi_size, as that will not increase the length of the last bvec.
Thus we have to properly add the last part into the bio. This will be done by the helper, round_up_last_block().
The reason we do not round those bios up at compression time is to get the unaligned compressed size, so that they can be utilized for inline extents.
If we round the bios up at *_compress_bio(), then every compressed bio will be larger than or equal to one fs block, resulting no inline compressed extent.
--- fs/btrfs/compression.c | 16 ++-- fs/btrfs/inode.c | 186 ++++++++++++++++++++++------------------- 2 files changed, 108 insertions(+), 94 deletions(-)
fs/btrfs/compression.c +7 -9
@@ -155,13 +155,6 @@ static int compression_decompress(int type, struct list_head *ws, } } -static void btrfs_free_compressed_folios (struct compressed_bio *cb) -{ -for (unsigned int i =0 ; i < cb->nr_folios; i++) -btrfs_free_compr_folio (cb->compressed_folios[i]); -kfree (cb->compressed_folios); -} -static int btrfs_decompress_bio (struct compressed_bio *cb);/*
@@ -270,12 +263,14 @@ static void end_bbio_compressed_read(struct btrfs_bio *bbio) {struct compressed_bio *cb =to_compressed_bio (bbio);blk_status_t status = bbio->bio.bi_status; +struct folio_iter fi;if (!status) status =errno_to_blk_status (btrfs_decompress_bio (cb)); -btrfs_free_compressed_folios (cb);btrfs_bio_end_io (cb->orig_bbio, status); +bio_for_each_folio_all (fi, &bbio->bio) +folio_put (fi.folio);bio_put (&bbio->bio); }
@@ -326,6 +321,7 @@ static noinline void end_compressed_writeback(const struct compressed_bio *cb)static void end_bbio_compressed_write (struct btrfs_bio *bbio) {struct compressed_bio *cb =to_compressed_bio (bbio); +struct folio_iter fi;btrfs_finish_ordered_extent (cb->bbio.ordered,NULL , cb->start, cb->len, cb->bbio.bio.bi_status ==BLK_STS_OK );
@@ -333,7 +329,9 @@ static void end_bbio_compressed_write(struct btrfs_bio *bbio)if (cb->writeback)end_compressed_writeback (cb);/* Note, our inode could be gone now. */ -btrfs_free_compressed_folios (cb); +bio_for_each_folio_all (fi, &bbio->bio) +btrfs_free_compr_folio (fi.folio); +kfree (cb->compressed_folios);bio_put (&cb->bbio.bio); }
fs/btrfs/inode.c +101 -85
342 lines not on this page · raw message
Signature · 1 line
2.52.0
PATCH v5 6/9 btrfs: remove the old btrfs_compress_folios() infrastructures
In reply to Qu WenruoSince it's replaced by btrfs_compress_bio(), remove all involved functions.
--- fs/btrfs/compression.c | 70 --------------- fs/btrfs/compression.h | 12 --- fs/btrfs/inode.c | 2 +- fs/btrfs/lzo.c | 188 ---------------------------------------- fs/btrfs/zlib.c | 189 ----------------------------------------- fs/btrfs/zstd.c | 189 ----------------------------------------- 6 files changed, 1 insertion(+), 649 deletions(-)
fs/btrfs/compression.c +0 -70
@@ -86,37 +86,6 @@ bool btrfs_compress_is_valid_type(const char *str, size_t len)return false; } -static int compression_compress_pages (int type,struct list_head *ws, -struct btrfs_inode *inode,u64 start, -struct folio **folios,unsigned long *out_folios, -unsigned long *total_in,unsigned long *total_out) -{ -switch (type) { -case BTRFS_COMPRESS_ZLIB : -return zlib_compress_folios (ws, inode, start, folios, - out_folios, total_in, total_out); -case BTRFS_COMPRESS_LZO : -return lzo_compress_folios (ws, inode, start, folios, - out_folios, total_in, total_out); -case BTRFS_COMPRESS_ZSTD : -return zstd_compress_folios (ws, inode, start, folios, - out_folios, total_in, total_out); -case BTRFS_COMPRESS_NONE : -default : -/* -* This can happen when compression races with remount setting -* it to 'no compress', while caller doesn't call -* inode_need_compress() to check if we really need to -* compress. -* -* Not a big deal, just need to inform caller that we -* haven't allocated any pages yet. -*/ - *out_folios =0 ; -return -E2BIG ; - } -} -static int compression_decompress_bio (struct list_head *ws,struct compressed_bio *cb) {
@@ -1023,45 +992,6 @@ int btrfs_compress_filemap_get_folio(struct address_space *mapping, u64 start,return 0 ; } -/* -* Given an address space and start and length, compress the bytes into @pages -* that are allocated on demand. -* -* @type_level is encoded algorithm and level, where level 0 means whatever -* default the algorithm chooses and is opaque here; -* - compression algo are 0-3 -* - the level are bits 4-7 -* -* @out_folios is an in/out parameter, holds maximum number of folios to allocate -* and returns number of actually allocated folios -* -* @total_in is used to return the number of bytes actually read. It -* may be smaller than the input length if we had to exit early because we -* ran out of room in the folios array or because we cross the -* max_out threshold. -* -* @total_out is an in/out parameter, must be set to the input length and will -* be also used to return the total number of compressed bytes -*/ -int btrfs_compress_folios (unsigned int type,int level,struct btrfs_inode *inode, -u64 start,struct folio **folios,unsigned long *out_folios, -unsigned long *total_in,unsigned long *total_out) -{ -struct btrfs_fs_info *fs_info = inode->root->fs_info; -const unsigned long orig_len = *total_out; -struct list_head *workspace; -int ret; - - level =btrfs_compress_set_level (type, level); - workspace =get_workspace (fs_info, type, level); - ret =compression_compress_pages (type, workspace, inode, start, folios, - out_folios, total_in, total_out); -/* The total read-in bytes should be no larger than the input. */ -ASSERT (*total_in <= orig_len); -put_workspace (fs_info, type, workspace); -return ret; -} -/* * Given an address space and start and length, compress the page cache * contents into @cb.
fs/btrfs/compression.h +0 -12
@@ -91,9 +91,6 @@ int __init btrfs_init_compress(void);void __cold btrfs_exit_compress (void );bool btrfs_compress_level_valid (unsigned int type,int level); -int btrfs_compress_folios (unsigned int type,int level,struct btrfs_inode *inode, -u64 start,struct folio **folios,unsigned long *out_folios, -unsigned long *total_in,unsigned long *total_out);int btrfs_decompress (int type,const u8 *data_in,struct folio *dest_folio,unsigned long dest_pgoff,size_t srclen,size_t destlen); int btrfs_decompress_buf2page(const char *buf, u32 buf_len,
@@ -160,9 +157,6 @@ static inline void cleanup_compressed_bio(struct compressed_bio *cb)bio_put (bio); } -int zlib_compress_folios (struct list_head *ws,struct btrfs_inode *inode, -u64 start,struct folio **folios,unsigned long *out_folios, -unsigned long *total_in,unsigned long *total_out);int zlib_compress_bio (struct list_head *ws,struct compressed_bio *cb);int zlib_decompress_bio (struct list_head *ws,struct compressed_bio *cb); int zlib_decompress(struct list_head *ws, const u8 *data_in,
@@ -172,9 +166,6 @@ struct list_head *zlib_alloc_workspace(struct btrfs_fs_info *fs_info, unsigned ivoid zlib_free_workspace (struct list_head *ws);struct list_head *zlib_get_workspace (struct btrfs_fs_info *fs_info,unsigned int level); -int lzo_compress_folios (struct list_head *ws,struct btrfs_inode *inode, -u64 start,struct folio **folios,unsigned long *out_folios, -unsigned long *total_in,unsigned long *total_out);int lzo_compress_bio (struct list_head *ws,struct compressed_bio *cb);int lzo_decompress_bio (struct list_head *ws,struct compressed_bio *cb); int lzo_decompress(struct list_head *ws, const u8 *data_in,
@@ -183,9 +174,6 @@ int lzo_decompress(struct list_head *ws, const u8 *data_in,struct list_head *lzo_alloc_workspace (struct btrfs_fs_info *fs_info);void lzo_free_workspace (struct list_head *ws); -int zstd_compress_folios (struct list_head *ws,struct btrfs_inode *inode, -u64 start,struct folio **folios,unsigned long *out_folios, -unsigned long *total_in,unsigned long *total_out);int zstd_compress_bio (struct list_head *ws,struct compressed_bio *cb);int zstd_decompress_bio (struct list_head *ws,struct compressed_bio *cb); int zstd_decompress(struct list_head *ws, const u8 *data_in,
fs/btrfs/inode.c +1 -1
@@ -967,7 +967,7 @@ static void compress_file_range(struct btrfs_work *work)/* * All the folios should have been locked thus no failure. * -* And even if some folios are missing, btrfs_compress_folios() +* And even if some folios are missing, btrfs_compress_bio() * would handle them correctly, so here just do an ASSERT() check for * early logic errors. */
fs/btrfs/lzo.c +0 -188
200 lines not on this page · raw message
fs/btrfs/zlib.c +0 -189
195 lines not on this page · raw message
fs/btrfs/zstd.c +0 -189
195 lines not on this page · raw message
Signature · 1 line
2.52.0
PATCH v5 7/9 btrfs: get rid of compressed_folios[] usage for compressed read
In reply to Qu WenruoCurrently btrfs_submit_compressed_read() still uses compressed_bio::compressed_folios[] array.
Change it to allocate each folio and queue them into the compressed bio so that we do not need to allocate that array.
Considering how small each compressed read bio is (less than 128KiB), we do not benefit that much from btrfs_alloc_folio_array() anyway, meanwhile we may benefit more from btrfs_alloc_compr_folio() by using the global folio pool.
So changing from btrfs_alloc_folio_array() to btrfs_alloc_compr_folio() in a loop should still be fine.
This removes one error path, and paves the way to completely remove compressed_folios[] array.
--- fs/btrfs/compression.c | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-)
fs/btrfs/compression.c +22 -20
@@ -239,7 +239,7 @@ static void end_bbio_compressed_read(struct btrfs_bio *bbio)btrfs_bio_end_io (cb->orig_bbio, status);bio_for_each_folio_all (fi, &bbio->bio) -folio_put (fi.folio); +btrfs_free_compr_folio (fi.folio);bio_put (&bbio->bio); }
@@ -537,13 +537,13 @@ void btrfs_submit_compressed_read(struct btrfs_bio *bbio)struct extent_map_tree *em_tree = &inode->extent_tree;struct compressed_bio *cb;unsigned int compressed_len; +const u32 min_folio_size =btrfs_min_folio_size (fs_info);u64 file_offset = bbio->file_offset;u64 em_len;u64 em_start;struct extent_map *em;unsigned long pflags;int memstall =0 ; -blk_status_t status;int ret;/* we need the actual starting offset of this extent in the file */
@@ -551,7 +551,7 @@ void btrfs_submit_compressed_read(struct btrfs_bio *bbio) em =btrfs_lookup_extent_mapping (em_tree, file_offset, fs_info->sectorsize);read_unlock (&em_tree->lock);if (!em) { - status =BLK_STS_IOERR ; + ret = -EIO ; goto out; }
@@ -573,27 +573,31 @@ void btrfs_submit_compressed_read(struct btrfs_bio *bbio)btrfs_free_extent_map (em); - cb->nr_folios =DIV_ROUND_UP (compressed_len,btrfs_min_folio_size (fs_info)); - cb->compressed_folios =kcalloc (cb->nr_folios,sizeof (struct folio *),GFP_NOFS ); -if (!cb->compressed_folios) { - status =BLK_STS_RESOURCE ; - goto out_free_bio; - } +for (int i =0 ; i * min_folio_size < compressed_len; i++) { +struct folio *folio; +u32 cur_len =min (compressed_len - i * min_folio_size, + min_folio_size); - ret =btrfs_alloc_folio_array (cb->nr_folios, fs_info->block_min_order, - cb->compressed_folios); -if (ret) { - status =BLK_STS_RESOURCE ; - goto out_free_compressed_pages; + folio =btrfs_alloc_compr_folio (fs_info); +if (!folio) { + ret = -ENOMEM ; + goto out_free_bio; + } + + ret =bio_add_folio (&cb->bbio.bio, folio, cur_len,0 ); +if (unlikely (!ret)) { +folio_put (folio); + ret = -EINVAL ; + goto out_free_bio; + } } +ASSERT (cb->bbio.bio.bi_iter.bi_size == compressed_len);add_ra_bio_pages (&inode->vfs_inode, em_start + em_len, cb, &memstall, &pflags); -/* include any pages we added in add_ra-bio_pages */ cb->len = bbio->bio.bi_iter.bi_size; cb->bbio.bio.bi_iter.bi_sector = bbio->bio.bi_iter.bi_sector; -btrfs_add_compressed_bio_folios (cb);if (memstall)psi_memstall_leave (&pflags);
@@ -601,12 +605,10 @@ void btrfs_submit_compressed_read(struct btrfs_bio *bbio)btrfs_submit_bbio (&cb->bbio,0 );return ; -out_free_compressed_pages: -kfree (cb->compressed_folios); out_free_bio: -bio_put (&cb->bbio.bio); +cleanup_compressed_bio (cb); out: -btrfs_bio_end_io (bbio, status); +btrfs_bio_end_io (bbio,errno_to_blk_status (ret)); }/*
Signature · 1 line
2.52.0
PATCH v5 8/9 btrfs: get rid of compressed_folios[] usage for encoded writes
In reply to Qu WenruoCurrently only encoded writes utilized btrfs_submit_compressed_write(), which utilized compressed_bio::compressed_folios[] array.
Change the only call site to call the new helper, btrfs_alloc_compressed_write(), to allocate a compressed bio, then queue needed folios into that bio, and finally call btrfs_submit_compressed_write() to submit the compreseed bio.
This change has one hidden benefit, previously we use btrfs_alloc_folio_array() for the folios of btrfs_submit_compressed_read(), which doesn't utilize the compression page pool for bs == ps cases.
Now we call btrfs_alloc_compr_folio() which will benefit from page pool.
The other obvious benefit is that we no longer need to allocate an array to hold all those folios, thus one less error path.
--- fs/btrfs/compression.c | 53 +++++++++++++++-------------------- fs/btrfs/compression.h | 6 ++-- fs/btrfs/inode.c | 63 +++++++++++++++++++++++------------------- 3 files changed, 59 insertions(+), 63 deletions(-)
fs/btrfs/compression.c +22 -31
@@ -304,25 +304,6 @@ static void end_bbio_compressed_write(struct btrfs_bio *bbio)bio_put (&cb->bbio.bio); } -static void btrfs_add_compressed_bio_folios (struct compressed_bio *cb) -{ -struct bio *bio = &cb->bbio.bio; -u32 offset =0 ; -unsigned int findex =0 ; - -while (offset < cb->compressed_len) { -struct folio *folio = cb->compressed_folios[findex]; -u32 len =min_t (u32, cb->compressed_len - offset,folio_size (folio)); -int ret; - -/* Maximum compressed extent is smaller than bio size limit. */ - ret =bio_add_folio (bio, folio, len,0 ); -ASSERT (ret); - offset += len; - findex++; - } -} -/* * worker function to build and submit bios for previously compressed pages. * The corresponding pages in the inode should be marked for writeback
@@ -333,34 +314,44 @@ static void btrfs_add_compressed_bio_folios(struct compressed_bio *cb)* the end io hooks. */ void btrfs_submit_compressed_write (struct btrfs_ordered_extent *ordered, -struct folio **compressed_folios, -unsigned int nr_folios, -blk_opf_t write_flags, -bool writeback) +struct compressed_bio *cb) {struct btrfs_inode *inode = ordered->inode;struct btrfs_fs_info *fs_info = inode->root->fs_info; -struct compressed_bio *cb;ASSERT (IS_ALIGNED (ordered->file_offset, fs_info->sectorsize));ASSERT (IS_ALIGNED (ordered->num_bytes, fs_info->sectorsize)); +ASSERT (cb->writeback); - cb =alloc_compressed_bio (inode, ordered->file_offset, -REQ_OP_WRITE | write_flags, - end_bbio_compressed_write); cb->start = ordered->file_offset; cb->len = ordered->num_bytes; - cb->compressed_folios = compressed_folios; cb->compressed_len = ordered->disk_num_bytes; - cb->writeback = writeback; - cb->nr_folios = nr_folios; cb->bbio.bio.bi_iter.bi_sector = ordered->disk_bytenr >>SECTOR_SHIFT ; cb->bbio.ordered = ordered; -btrfs_add_compressed_bio_folios (cb);btrfs_submit_bbio (&cb->bbio,0 ); } +/* +* Allocate a compressed write bio for @inode file offset @start length @len. +* +* The caller still needs to properly queue all folios and populate involved +* members. +*/ +struct compressed_bio *btrfs_alloc_compressed_write (struct btrfs_inode *inode, +u64 start,u64 len) +{ +struct compressed_bio *cb; + + cb =alloc_compressed_bio (inode, start,REQ_OP_WRITE , + end_bbio_compressed_write); + cb->start = start; + cb->len = len; + cb->writeback = true; + +return cb; +} +/* * Add extra pages in the same compressed file extent so that we don't need to * re-read the same extent again and again.
fs/btrfs/compression.h +3 -3
@@ -96,10 +96,10 @@ int btrfs_decompress(int type, const u8 *data_in, struct folio *dest_folio,int btrfs_decompress_buf2page (const char *buf,u32 buf_len,struct compressed_bio *cb,u32 decompressed); +struct compressed_bio *btrfs_alloc_compressed_write (struct btrfs_inode *inode, +u64 start,u64 len);void btrfs_submit_compressed_write (struct btrfs_ordered_extent *ordered, -struct folio **compressed_folios, -unsigned int nr_folios,blk_opf_t write_flags, -bool writeback); +struct compressed_bio *cb);void btrfs_submit_compressed_read (struct btrfs_bio *bbio);int btrfs_compress_str2level (unsigned int type,const char *str,int *level_ret);
fs/btrfs/inode.c +34 -29
111 lines not on this page · raw message
Signature · 1 line
2.52.0
PATCH v5 9/9 btrfs: get rid of compressed_bio::compressed_folios[]
In reply to Qu WenruoNow there is no one utilizing that member, we can safely remove it along with compressed_bio::nr_folios member.
--- fs/btrfs/compression.c | 1 - fs/btrfs/compression.h | 6 ------ 2 files changed, 7 deletions(-)
fs/btrfs/compression.c +0 -1
@@ -300,7 +300,6 @@ static void end_bbio_compressed_write(struct btrfs_bio *bbio)/* Note, our inode could be gone now. */ bio_for_each_folio_all (fi, &bbio->bio)btrfs_free_compr_folio (fi.folio); -kfree (cb->compressed_folios);bio_put (&cb->bbio.bio); }
fs/btrfs/compression.h +0 -6
@@ -42,12 +42,6 @@ static_assert((BTRFS_MAX_COMPRESSED % PAGE_SIZE) == 0);#define BTRFS_ZLIB_DEFAULT_LEVEL 3struct compressed_bio { -/* Number of compressed folios in the array. */ -unsigned int nr_folios; - -/* The folios with the compressed data on them. */ -struct folio **compressed_folios; -/* starting offset in the inode for our pages */ u64 start;
Signature · 1 line
2.52.0
Quoted 7 lines · Qu Wenruo
[CHANGELOG]
v5:
- Implement an internalcompress_bio_last_folio()
Upstream is removingbio_last_bvec_all(), and the recent stable page bouncing for direct IO is going to make that function no longer reliable.
Quoted 7 lines
Thus we need to implement it locally, with some extra ASSERT()s to make sure the compressed bio have all folio sizes fixed to
min_folio_size, so that we can useoffset_in_folio(folio, bi_size)to calculate the last position.- Fix several spelling errors
jut -> just in patch 5 and chaning -> changing in patch 7.
I've moved the series to for-next, with some fixups here and there. The tests for all three compression algorithms and we're short on time before the 6.20/7.0 pull so any fixups please send as separate ptches. Thanks.