From: Omar Sandoval <redacted>
This series has three parts: new Btrfs ioctls for reading/writing
compressed data, support for sending compressed data via Btrfs send, and
btrfs-progs support for sending/receiving compressed data and writing it
with the new ioctl.
The Btrfs ioctls for reading compressed data from a file without
decompressing it and for writing compressed data directly to a file are
adapted from my previous attempt to do this as an extension to
preadv2/pwritev2 [1]. We weren't able to come up with a generic
interface that everyone was happy with, so we're going to do this
ourselves in Btrfs. If another user comes along, we can generalize it
then. Test cases are here [2]
Patches 1 and 2 are VFS changes exporting a couple of helpers for checks
needed by reads and writes. Patches 3-7 are preparatory Btrfs changes
for compressed reads and writes. Patch 8 adds the compressed read ioctl
and patch 9 adds the compressed write ioctl.
The main use-case for this interface is Btrfs send/receive. Currently,
when sending data from one compressed filesystem to another, the sending
side decompresses the data and the receiving side recompresses it before
writing it out. This is wasteful and can be avoided if we can just send
and write compressed extents.
Patches 10-14 add the Btrfs send support. See the previous posting for
more details and benchmarks [3]. Patches 10-12 prepare some protocol
changes for send stream v2. Patch 13 implements compressed send. Patch
14 enables send stream v2 and compressed send in the send ioctl when
requested.
These patches are based on Dave Sterba's Btrfs misc-next branch [4],
which is in turn currently based on v5.14-rc6.
1: https://lore.kernel.org/linux-fsdevel/cover.1623972518.git.osandov@fb.com/
2: https://github.com/osandov/xfstests/tree/btrfs-encoded-io
3: https://lore.kernel.org/linux-btrfs/cover.1615922753.git.osandov@fb.com/
4: https://github.com/kdave/btrfs-devel/tree/misc-next
Omar Sandoval (14):
fs: export rw_verify_area()
fs: export variant of generic_write_checks without iov_iter
btrfs: don't advance offset for compressed bios in
btrfs_csum_one_bio()
btrfs: add ram_bytes and offset to btrfs_ordered_extent
btrfs: support different disk extent size for delalloc
btrfs: optionally extend i_size in cow_file_range_inline()
btrfs: add definitions + documentation for encoded I/O ioctls
btrfs: add BTRFS_IOC_ENCODED_READ
btrfs: add BTRFS_IOC_ENCODED_WRITE
btrfs: add send stream v2 definitions
btrfs: send: write larger chunks when using stream v2
btrfs: send: allocate send buffer with alloc_page() and vmap() for v2
btrfs: send: send compressed extents with encoded writes
btrfs: send: enable support for stream v2 and compressed writes
fs/btrfs/compression.c | 12 +-
fs/btrfs/compression.h | 6 +-
fs/btrfs/ctree.h | 17 +-
fs/btrfs/delalloc-space.c | 18 +-
fs/btrfs/file-item.c | 35 +-
fs/btrfs/file.c | 68 ++-
fs/btrfs/inode.c | 911 +++++++++++++++++++++++++++++++++----
fs/btrfs/ioctl.c | 213 +++++++++
fs/btrfs/ordered-data.c | 124 ++---
fs/btrfs/ordered-data.h | 25 +-
fs/btrfs/relocation.c | 2 +-
fs/btrfs/send.c | 307 +++++++++++--
fs/btrfs/send.h | 32 +-
fs/internal.h | 5 -
fs/read_write.c | 41 +-
include/linux/fs.h | 2 +
include/uapi/linux/btrfs.h | 149 +++++-
17 files changed, 1690 insertions(+), 277 deletions(-)
The btrfs-progs patches were written by Boris Burkov with some updates
from me. Patches 1-4 are preparation. Patch 5 implements encoded writes.
Patch 6 implements the fallback to decompressing. Patches 7 and 8
implement the other commands. Patch 9 adds the new `btrfs send` options.
Patch 10 adds a test case.
Boris Burkov (10):
btrfs-progs: receive: support v2 send stream larger tlv_len
btrfs-progs: receive: dynamically allocate sctx->read_buf
btrfs-progs: receive: support v2 send stream DATA tlv format
btrfs-progs: receive: add send stream v2 cmds and attrs to send.h
btrfs-progs: receive: process encoded_write commands
btrfs-progs: receive: encoded_write fallback to explicit decode and
write
btrfs-progs: receive: process fallocate commands
btrfs-progs: receive: process setflags ioctl commands
btrfs-progs: send: stream v2 ioctl flags
btrfs-progs: receive: add tests for basic encoded_write send/receive
Documentation/btrfs-receive.asciidoc | 4 +
Documentation/btrfs-send.asciidoc | 16 +-
cmds/receive-dump.c | 31 +-
cmds/receive.c | 347 +++++++++++++++++-
cmds/send.c | 54 ++-
common/send-stream.c | 157 ++++++--
common/send-stream.h | 7 +
ioctl.h | 149 +++++++-
libbtrfsutil/btrfs.h | 17 +-
send.h | 19 +-
.../049-receive-write-encoded/test.sh | 114 ++++++
11 files changed, 871 insertions(+), 44 deletions(-)
create mode 100755 tests/misc-tests/049-receive-write-encoded/test.sh
--
2.32.0
From: Omar Sandoval <redacted>
Encoded I/O in Btrfs needs to check a write with a given logical size
without an iov_iter that matches that size (because the iov_iter we have
is for the compressed data). So, factor out the parts of
generic_write_check() that expect an iov_iter into a new
__generic_write_checks() function and export that.
Signed-off-by: Omar Sandoval <redacted>
---
fs/read_write.c | 40 ++++++++++++++++++++++------------------
include/linux/fs.h | 1 +
2 files changed, 23 insertions(+), 18 deletions(-)
@@ -1633,6 +1633,26 @@ int generic_write_check_limits(struct file *file, loff_t pos, loff_t *count)return0;}+/* Like generic_write_checks(), but takes size of write instead of iter. */+int__generic_write_checks(structkiocb*iocb,loff_t*count)+{+structfile*file=iocb->ki_filp;+structinode*inode=file->f_mapping->host;++if(IS_SWAPFILE(inode))+return-ETXTBSY;++/* FIXME: this is for backwards compatibility with 2.4 */+if(iocb->ki_flags&IOCB_APPEND)+iocb->ki_pos=i_size_read(inode);++if((iocb->ki_flags&IOCB_NOWAIT)&&!(iocb->ki_flags&IOCB_DIRECT))+return-EINVAL;++returngeneric_write_check_limits(iocb->ki_filp,iocb->ki_pos,count);+}+EXPORT_SYMBOL(__generic_write_checks);+/**Performsnecessarychecksbeforedoingawrite*
@@ -1642,26 +1662,10 @@ int generic_write_check_limits(struct file *file, loff_t pos, loff_t *count)*/ssize_tgeneric_write_checks(structkiocb*iocb,structiov_iter*from){-structfile*file=iocb->ki_filp;-structinode*inode=file->f_mapping->host;-loff_tcount;+loff_tcount=iov_iter_count(from);intret;-if(IS_SWAPFILE(inode))-return-ETXTBSY;--if(!iov_iter_count(from))-return0;--/* FIXME: this is for backwards compatibility with 2.4 */-if(iocb->ki_flags&IOCB_APPEND)-iocb->ki_pos=i_size_read(inode);--if((iocb->ki_flags&IOCB_NOWAIT)&&!(iocb->ki_flags&IOCB_DIRECT))-return-EINVAL;--count=iov_iter_count(from);-ret=generic_write_check_limits(file,iocb->ki_pos,&count);+ret=__generic_write_checks(iocb,&count);if(ret)returnret;
From: Omar Sandoval <redacted>
btrfs_csum_one_bio() loops over each filesystem block in the bio while
keeping a cursor of its current logical position in the file in order to
look up the ordered extent to add the checksums to. However, this
doesn't make much sense for compressed extents, as a sector on disk does
not correspond to a sector of decompressed file data. It happens to work
because 1) the compressed bio always covers one ordered extent and 2)
the size of the bio is always less than the size of the ordered extent.
However, the second point will not always be true for encoded writes.
Let's add a boolean parameter to btrfs_csum_one_bio() to indicate that
it can assume that the bio only covers one ordered extent. Since we're
already changing the signature, let's get rid of the contig parameter
and make it implied by the offset parameter, similar to the change we
recently made to btrfs_lookup_bio_sums(). Additionally, let's rename
nr_sectors to blockcount to make it clear that it's the number of
filesystem blocks, not the number of 512-byte sectors.
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/compression.c | 5 +++--
fs/btrfs/ctree.h | 2 +-
fs/btrfs/file-item.c | 35 ++++++++++++++++-------------------
fs/btrfs/inode.c | 8 ++++----
4 files changed, 24 insertions(+), 26 deletions(-)
From: Omar Sandoval <redacted>
Currently, we only create ordered extents when ram_bytes == num_bytes
and offset == 0. However, RWF_ENCODED writes may create extents which
only refer to a subset of the full unencoded extent, so we need to plumb
these fields through the ordered extent infrastructure and pass them
down to insert_reserved_file_extent().
Since we're changing the btrfs_add_ordered_extent* signature, let's get
rid of the trivial wrappers and add a kernel-doc.
Reviewed-by: Nikolay Borisov <redacted>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/inode.c | 56 +++++++++++---------
fs/btrfs/ordered-data.c | 112 +++++++++++-----------------------------
fs/btrfs/ordered-data.h | 22 ++++----
3 files changed, 76 insertions(+), 114 deletions(-)
@@ -1180,9 +1180,9 @@ static noinline int cow_file_range(struct btrfs_inode *inode,}free_extent_map(em);-ret=btrfs_add_ordered_extent(inode,start,ins.objectid,-ram_size,cur_alloc_size,-BTRFS_ORDERED_REGULAR);+ret=btrfs_add_ordered_extent(inode,start,ram_size,ram_size,+ins.objectid,cur_alloc_size,0,+0,BTRFS_COMPRESS_NONE);if(ret)gotoout_drop_extent_cache;
@@ -1843,10 +1843,11 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,gotoerror;}free_extent_map(em);-ret=btrfs_add_ordered_extent(inode,cur_offset,-disk_bytenr,num_bytes,-num_bytes,-BTRFS_ORDERED_PREALLOC);+ret=btrfs_add_ordered_extent(inode,+cur_offset,num_bytes,num_bytes,+disk_bytenr,num_bytes,0,+1<<BTRFS_ORDERED_PREALLOC,+BTRFS_COMPRESS_NONE);if(ret){btrfs_drop_extent_cache(inode,cur_offset,cur_offset+num_bytes-1,
@@ -1855,9 +1856,11 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,}}else{ret=btrfs_add_ordered_extent(inode,cur_offset,+num_bytes,num_bytes,disk_bytenr,num_bytes,-num_bytes,-BTRFS_ORDERED_NOCOW);+0,+1<<BTRFS_ORDERED_NOCOW,+BTRFS_COMPRESS_NONE);if(ret)gotoerror;}
@@ -2842,6 +2845,7 @@ static int insert_reserved_file_extent(struct btrfs_trans_handle *trans,structbtrfs_keyins;u64disk_num_bytes=btrfs_stack_file_extent_disk_num_bytes(stack_fi);u64disk_bytenr=btrfs_stack_file_extent_disk_bytenr(stack_fi);+u64offset=btrfs_stack_file_extent_offset(stack_fi);u64num_bytes=btrfs_stack_file_extent_num_bytes(stack_fi);u64ram_bytes=btrfs_stack_file_extent_ram_bytes(stack_fi);structbtrfs_drop_extents_argsdrop_args={0};
@@ -2916,7 +2920,8 @@ static int insert_reserved_file_extent(struct btrfs_trans_handle *trans,gotoout;ret=btrfs_alloc_reserved_file_extent(trans,root,btrfs_ino(inode),-file_pos,qgroup_reserved,&ins);+file_pos-offset,+qgroup_reserved,&ins);out:btrfs_free_path(path);
@@ -2942,20 +2947,20 @@ static int insert_ordered_extent_file_extent(struct btrfs_trans_handle *trans,structbtrfs_ordered_extent*oe){structbtrfs_file_extent_itemstack_fi;-u64logical_len;boolupdate_inode_bytes;+u64num_bytes=oe->num_bytes;+u64ram_bytes=oe->ram_bytes;memset(&stack_fi,0,sizeof(stack_fi));btrfs_set_stack_file_extent_type(&stack_fi,BTRFS_FILE_EXTENT_REG);btrfs_set_stack_file_extent_disk_bytenr(&stack_fi,oe->disk_bytenr);btrfs_set_stack_file_extent_disk_num_bytes(&stack_fi,oe->disk_num_bytes);+btrfs_set_stack_file_extent_offset(&stack_fi,oe->offset);if(test_bit(BTRFS_ORDERED_TRUNCATED,&oe->flags))-logical_len=oe->truncated_len;-else-logical_len=oe->num_bytes;-btrfs_set_stack_file_extent_num_bytes(&stack_fi,logical_len);-btrfs_set_stack_file_extent_ram_bytes(&stack_fi,logical_len);+num_bytes=ram_bytes=oe->truncated_len;+btrfs_set_stack_file_extent_num_bytes(&stack_fi,num_bytes);+btrfs_set_stack_file_extent_ram_bytes(&stack_fi,ram_bytes);btrfs_set_stack_file_extent_compression(&stack_fi,oe->compress_type);/* Encryption and other encoding is reserved and all 0 */
@@ -161,7 +172,8 @@ static int __btrfs_add_ordered_extent(struct btrfs_inode *inode, u64 file_offsetstructbtrfs_ordered_extent*entry;intret;-if(type==BTRFS_ORDERED_NOCOW||type==BTRFS_ORDERED_PREALLOC){+if(flags&+((1<<BTRFS_ORDERED_NOCOW)|(1<<BTRFS_ORDERED_PREALLOC))){/* For nocow write, we can release the qgroup rsv right now */ret=btrfs_qgroup_free_data(inode,NULL,file_offset,num_bytes);if(ret<0)
@@ -191,18 +205,12 @@ static int __btrfs_add_ordered_extent(struct btrfs_inode *inode, u64 file_offsetentry->qgroup_rsv=ret;entry->physical=(u64)-1;-ASSERT(type==BTRFS_ORDERED_REGULAR||-type==BTRFS_ORDERED_NOCOW||-type==BTRFS_ORDERED_PREALLOC||-type==BTRFS_ORDERED_COMPRESSED);-set_bit(type,&entry->flags);+ASSERT((flags&~BTRFS_ORDERED_TYPE_FLAGS)==0);+entry->flags=flags;percpu_counter_add_batch(&fs_info->ordered_bytes,num_bytes,fs_info->delalloc_batch);-if(dio)-set_bit(BTRFS_ORDERED_DIRECT,&entry->flags);-/* one ref for the tree */refcount_set(&entry->refs,1);init_waitqueue_head(&entry->wait);
@@ -76,6 +76,13 @@ enum {BTRFS_ORDERED_PENDING,};+/* BTRFS_ORDERED_* flags that specify the type of the extent. */+#define BTRFS_ORDERED_TYPE_FLAGS ((1UL << BTRFS_ORDERED_REGULAR) | \+(1UL<<BTRFS_ORDERED_NOCOW)|\+(1UL<<BTRFS_ORDERED_PREALLOC)|\+(1UL<<BTRFS_ORDERED_COMPRESSED)|\+(1UL<<BTRFS_ORDERED_DIRECT))+structbtrfs_ordered_extent{/* logical offset in the file */u64file_offset;
@@ -84,9 +91,11 @@ struct btrfs_ordered_extent {*Thesefieldsdirectlycorrespondtothesamefieldsin*btrfs_file_extent_item.*/-u64disk_bytenr;u64num_bytes;+u64ram_bytes;+u64disk_bytenr;u64disk_num_bytes;+u64offset;/* number of bytes that still need writing */u64bytes_left;
From: Omar Sandoval <redacted>
Currently, we always reserve the same extent size in the file and extent
size on disk for delalloc because the former is the worst case for the
latter. For RWF_ENCODED writes, we know the exact size of the extent on
disk, which may be less than or greater than (for bookends) the size in
the file. Add a disk_num_bytes parameter to
btrfs_delalloc_reserve_metadata() so that we can reserve the correct
amount of csum bytes. No functional change.
Reviewed-by: Nikolay Borisov <redacted>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/ctree.h | 3 ++-
fs/btrfs/delalloc-space.c | 18 ++++++++++--------
fs/btrfs/file.c | 3 ++-
fs/btrfs/inode.c | 2 +-
fs/btrfs/relocation.c | 2 +-
5 files changed, 16 insertions(+), 12 deletions(-)
@@ -3016,7 +3016,7 @@ static int relocate_one_page(struct inode *inode, struct file_ra_state *ra,/* Reserve metadata for this range */ret=btrfs_delalloc_reserve_metadata(BTRFS_I(inode),-clamped_len);+clamped_len,clamped_len);if(ret)gotorelease_page;
From: Omar Sandoval <redacted>
Currently, an inline extent is always created after i_size is extended
from btrfs_dirty_pages(). However, for encoded writes, we only want to
update i_size after we successfully created the inline extent. Add an
update_i_size parameter to cow_file_range_inline() and
insert_inline_extent() and pass in the size of the extent rather than
determining it from i_size. Since the start parameter is always passed
as 0, get rid of it and simplify the logic in these two functions. While
we're here, let's document the requirements for creating an inline
extent.
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/inode.c | 100 +++++++++++++++++++++++------------------------
1 file changed, 48 insertions(+), 52 deletions(-)
@@ -695,14 +687,15 @@ static noinline int compress_file_range(struct async_chunk *async_chunk)/* we didn't compress the entire range, try*tomakeanuncompressedinlineextent.*/-ret=cow_file_range_inline(BTRFS_I(inode),start,end,+ret=cow_file_range_inline(BTRFS_I(inode),actual_end,0,BTRFS_COMPRESS_NONE,-NULL);+NULL,false);}else{/* try making a compressed inline extent */-ret=cow_file_range_inline(BTRFS_I(inode),start,end,+ret=cow_file_range_inline(BTRFS_I(inode),actual_end,total_compressed,-compress_type,pages);+compress_type,pages,+false);}if(ret<=0){unsignedlongclear_flags=EXTENT_DELALLOC|
@@ -1098,9 +1091,12 @@ static noinline int cow_file_range(struct btrfs_inode *inode,*Sohereweskipinlineextentcreationcompletely.*/if(start==0&&fs_info->sectorsize==PAGE_SIZE){+u64actual_end=min_t(u64,i_size_read(&inode->vfs_inode),+end+1);+/* lets try to make an inline extent */-ret=cow_file_range_inline(inode,start,end,0,-BTRFS_COMPRESS_NONE,NULL);+ret=cow_file_range_inline(inode,actual_end,0,+BTRFS_COMPRESS_NONE,NULL,false);if(ret==0){/**WeuseDO_ACCOUNTINGherebecauseweneedthe
From: Omar Sandoval <redacted>
In order to allow sending and receiving compressed data without
decompressing it, we need an interface to write pre-compressed data
directly to the filesystem and the matching interface to read compressed
data without decompressing it. This adds the definitions for ioctls to
do that and detailed explanations of how to use them.
Signed-off-by: Omar Sandoval <redacted>
---
include/uapi/linux/btrfs.h | 132 +++++++++++++++++++++++++++++++++++++
1 file changed, 132 insertions(+)
@@ -861,6 +861,134 @@ struct btrfs_ioctl_get_subvol_rootref_args {__u8align[7];};+/*+*Dataandmetadataforanencodedreadorwrite.+*+*EncodedI/Obypassesanyencodingautomaticallydonebythefilesystem(e.g.,+*compression).Thiscanbeusedtoreadthecompressedcontentsofafileor+*writepre-compresseddatadirectlytoafile.+*+*BTRFS_IOC_ENCODED_READandBTRFS_IOC_ENCODED_WRITEareessentially+*preadv/pwritevwithadditionalmetadataabouthowthedataisencodedandthe+*sizeoftheunencodeddata.+*+*BTRFS_IOC_ENCODED_READfillsthegiveniovecswiththeencodeddata,fills+*themetadatafields,andreturnsthesizeoftheencodeddata.Itreadsone+*extentpercall.Itcanalsoreaddatawhichisnotencoded.+*+*BTRFS_IOC_ENCODED_WRITEusesthemetadatafields,writestheencodeddata+*fromtheiovecs,andreturnsthesizeoftheencodeddata.Notethatthe+*encodeddataisnotvalidatedwhenitiswritten;ifitisnotvalid(e.g.,+*itcannotbedecompressed),thenasubsequentreadmayreturnanerror.+*+*Sincethefilesystempagecachecontainsdecodeddata,encodedI/Obypasses+*thepagecache.EncodedI/OrequiresCAP_SYS_ADMIN.+*/+structbtrfs_ioctl_encoded_io_args{+/* Input parameters for both reads and writes. */++/*+*iovecscontainingencodeddata.+*+*Forreads,ifthesizeoftheencodeddataislargerthanthesumof+*iov[n].iov_lenfor0<=n<iovcnt,thentheioctlfailswith+*ENOBUFS.+*+*Forwrites,thesizeoftheencodeddataisthesumofiov[n].iov_len+*for0<=n<iovcnt.Thismustbelessthan128KiB(thislimitmay+*increaseinthefuture).Thismustalsobelessthanorequalto+*unencoded_len.+*/+conststructiovec__user*iov;+/* Number of iovecs. */+unsignedlongiovcnt;+/*+*Offsetinfile.+*+*Forwrites,mustbealignedtothesectorsizeofthefilesystem.+*/+__s64offset;+/* Currently must be zero. */+__u64flags;++/*+*Forreads,thefollowingmembersarefilledinwiththemetadatafor+*theencodeddata.+*Forwrites,thefollowingmembersmustbesettothemetadataforthe+*encodeddata.+*/++/*+*Lengthofthedatainthefile.+*+*Mustbelessthanorequaltounencoded_len-unencoded_offset.For+*writes,mustbealignedtothesectorsizeofthefilesystemunless+*thedataendsatorbeyondthecurrentendofthefile.+*/+__u64len;+/*+*Lengthoftheunencoded(i.e.,decryptedanddecompressed)data.+*+*Forwrites,mustbenomorethan128KiB(thislimitmayincreasein+*thefuture).Iftheunencodeddataisactuallylongerthan+*unencoded_len,thenitistruncated;ifitisshorter,thenitis+*extendedwithzeroes.+*/+__u64unencoded_len;+/*+*Offsetfromthefirstbyteoftheunencodeddatatothefirstbyteof+*logicaldatainthefile.+*+*Mustbelessthanunencoded_len.+*/+__u64unencoded_offset;+/*+*BTRFS_ENCODED_IO_COMPRESSION_*type.+*+*Forwrites,mustnotbeBTRFS_ENCODED_IO_COMPRESSION_NONE.+*/+__u32compression;+/* Currently always BTRFS_ENCODED_IO_ENCRYPTION_NONE. */+__u32encryption;+/*+*Reservedforfutureexpansion.+*+*Forreads,alwaysreturnedaszero.Usersshouldcheckfornon-zero+*bytes.Ifthereareany,thenthekernelhasanewerversionofthis+*structurewithadditionalinformationthattheuserdefinitionis+*missing.+*+*Forwrites,mustbezeroed.+*/+__u8reserved[32];+};++/* Data is not compressed. */+#define BTRFS_ENCODED_IO_COMPRESSION_NONE 0+/* Data is compressed as a single zlib stream. */+#define BTRFS_ENCODED_IO_COMPRESSION_ZLIB 1+/*+*DataiscompressedasasinglezstdframewiththewindowLogcompression+*parametersettonomorethan17.+*/+#define BTRFS_ENCODED_IO_COMPRESSION_ZSTD 2+/*+*Dataiscompressedpagebypage(usingthepagesizeindicatedbythenameof+*theconstant)withLZO1Xandwrappedintheformatdocumentedin+*fs/btrfs/lzo.c.Forwrites,thecompressionpagesizemustmatchthe+*filesystempagesize.+*/+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_4K 3+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_8K 4+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_16K 5+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_32K 6+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_64K 7+#define BTRFS_ENCODED_IO_COMPRESSION_TYPES 8++/* Data is not encrypted. */+#define BTRFS_ENCODED_IO_ENCRYPTION_NONE 0+#define BTRFS_ENCODED_IO_ENCRYPTION_TYPES 1+/* Error codes as returned by the kernel */enumbtrfs_err_code{BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET=1,
From: Omar Sandoval <redacted>
There are 4 main cases:
1. Inline extents: we copy the data straight out of the extent buffer.
2. Hole/preallocated extents: we fill in zeroes.
3. Regular, uncompressed extents: we read the sectors we need directly
from disk.
4. Regular, compressed extents: we read the entire compressed extent
from disk and indicate what subset of the decompressed extent is in
the file.
This initial implementation simplifies a few things that can be improved
in the future:
- We hold the inode lock during the operation.
- Cases 1, 3, and 4 allocate temporary memory to read into before
copying out to userspace.
- We don't do read repair, because it turns out that read repair is
currently broken for compressed data.
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/ctree.h | 4 +
fs/btrfs/inode.c | 489 +++++++++++++++++++++++++++++++++++++++++++++++
fs/btrfs/ioctl.c | 111 +++++++++++
3 files changed, 604 insertions(+)
@@ -86,6 +87,22 @@ struct btrfs_ioctl_send_args_32 {#define BTRFS_IOC_SEND_32 _IOW(BTRFS_IOCTL_MAGIC, 38, \structbtrfs_ioctl_send_args_32)++structbtrfs_ioctl_encoded_io_args_32{+compat_uptr_tiov;+compat_ulong_tiovcnt;+__s64offset;+__u64flags;+__u64len;+__u64unencoded_len;+__u64unencoded_offset;+__u32compression;+__u32encryption;+__u32reserved[8];+};++#define BTRFS_IOC_ENCODED_READ_32 _IOR(BTRFS_IOCTL_MAGIC, 64, \+structbtrfs_ioctl_encoded_io_args_32)#endif/* Mask out flags that are inappropriate for the given type of inode. */
From: Omar Sandoval <redacted>
The length field of the send stream TLV header is 16 bits. This means
that the maximum amount of data that can be sent for one write is 64k
minus one. However, encoded writes must be able to send the maximum
compressed extent (128k) in one command. To support this, send stream
version 2 encodes the DATA attribute differently: it has no length
field, and the length is implicitly up to the end of containing command
(which has a 32-bit length field). Although this is necessary for
encoded writes, normal writes can benefit from it, too.
For v2, let's bump up the send buffer to the maximum compressed extent
size plus 16k for the other metadata (144k total). Since this will most
likely be vmalloc'd (and always will be after the next commit), we round
it up to the next page since we might as well use the rest of the page
on systems with >16k pages.
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/send.c | 34 ++++++++++++++++++++++++++--------
1 file changed, 26 insertions(+), 8 deletions(-)
From: Omar Sandoval <redacted>
The implementation resembles direct I/O: we have to flush any ordered
extents, invalidate the page cache, and do the io tree/delalloc/extent
map/ordered extent dance. From there, we can reuse the compression code
with a minor modification to distinguish the write from writeback. This
also creates inline extents when possible.
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/compression.c | 7 +-
fs/btrfs/compression.h | 6 +-
fs/btrfs/ctree.h | 4 +
fs/btrfs/file.c | 65 ++++++++--
fs/btrfs/inode.c | 256 +++++++++++++++++++++++++++++++++++++++-
fs/btrfs/ioctl.c | 102 ++++++++++++++++
fs/btrfs/ordered-data.c | 12 +-
fs/btrfs/ordered-data.h | 5 +-
8 files changed, 437 insertions(+), 20 deletions(-)
@@ -353,7 +353,8 @@ static void end_compressed_bio_write(struct bio *bio)cb->start,cb->start+cb->len-1,!cb->errors);-end_compressed_writeback(inode,cb);+if(cb->writeback)+end_compressed_writeback(inode,cb);/* note, our inode could be gone now *//*
@@ -52,6 +52,9 @@ struct compressed_bio {/* The compression algorithm for this bio */u8compress_type;+/* Whether this is a write for writeback. */+boolwriteback;+/* IO errors */u8errors;intmirror_num;
@@ -2967,6 +2967,7 @@ static int insert_ordered_extent_file_extent(struct btrfs_trans_handle *trans,*exceptiftheorderedextentwastruncated.*/update_inode_bytes=test_bit(BTRFS_ORDERED_DIRECT,&oe->flags)||+test_bit(BTRFS_ORDERED_ENCODED,&oe->flags)||test_bit(BTRFS_ORDERED_TRUNCATED,&oe->flags);returninsert_reserved_file_extent(trans,BTRFS_I(oe->inode),
@@ -3001,7 +3002,8 @@ static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)if(!test_bit(BTRFS_ORDERED_NOCOW,&ordered_extent->flags)&&!test_bit(BTRFS_ORDERED_PREALLOC,&ordered_extent->flags)&&-!test_bit(BTRFS_ORDERED_DIRECT,&ordered_extent->flags))+!test_bit(BTRFS_ORDERED_DIRECT,&ordered_extent->flags)&&+!test_bit(BTRFS_ORDERED_ENCODED,&ordered_extent->flags))clear_bits|=EXTENT_DELALLOC_NEW;freespace_inode=btrfs_is_free_space_inode(inode);
@@ -10985,6 +10987,256 @@ ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter,returnret;}+ssize_tbtrfs_do_encoded_write(structkiocb*iocb,structiov_iter*from,+conststructbtrfs_ioctl_encoded_io_args*encoded)+{+structinode*inode=file_inode(iocb->ki_filp);+structbtrfs_fs_info*fs_info=btrfs_sb(inode->i_sb);+structbtrfs_root*root=BTRFS_I(inode)->root;+structextent_io_tree*io_tree=&BTRFS_I(inode)->io_tree;+structextent_changeset*data_reserved=NULL;+structextent_state*cached_state=NULL;+intcompression;+size_torig_count;+u64start,end;+u64num_bytes,ram_bytes,disk_num_bytes;+unsignedlongnr_pages,i;+structpage**pages;+structbtrfs_keyins;+boolextent_reserved=false;+structextent_map*em;+ssize_tret;++switch(encoded->compression){+caseBTRFS_ENCODED_IO_COMPRESSION_ZLIB:+compression=BTRFS_COMPRESS_ZLIB;+break;+caseBTRFS_ENCODED_IO_COMPRESSION_ZSTD:+compression=BTRFS_COMPRESS_ZSTD;+break;+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_4K:+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_8K:+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_16K:+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_32K:+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_64K:+/* The page size must match for LZO. */+if(encoded->compression-+BTRFS_ENCODED_IO_COMPRESSION_LZO_4K+12!=PAGE_SHIFT)+return-EINVAL;+compression=BTRFS_COMPRESS_LZO;+break;+default:+return-EINVAL;+}+if(encoded->encryption!=BTRFS_ENCODED_IO_ENCRYPTION_NONE)+return-EINVAL;++orig_count=iov_iter_count(from);++/* The extent size must be sane. */+if(encoded->unencoded_len>BTRFS_MAX_UNCOMPRESSED||+orig_count>BTRFS_MAX_COMPRESSED||orig_count==0)+return-EINVAL;++/*+*Thecompresseddatamustbesmallerthanthedecompresseddata.+*+*It'sofcoursepossiblefordatatocompresstolargerorthesame+*size,butthebufferedI/Opathfallsbacktonocompressionforsuch+*data,andwedon'twanttobreakanyassumptionsbycreatingthese+*extents.+*+*Notethatthisislessstrictthanthecurrentcheckwehavethatthe+*compresseddatamustbeatleastonesectorsmallerthanthe+*decompresseddata.Weonlywanttoenforcetheweakerrequirement+*fromoldkernelsthatitisatleastonebytesmaller.+*/+if(orig_count>=encoded->unencoded_len)+return-EINVAL;++/* The extent must start on a sector boundary. */+start=iocb->ki_pos;+if(!IS_ALIGNED(start,fs_info->sectorsize))+return-EINVAL;++/*+*Theextentmustendonasectorboundary.However,weallowawrite+*whichendsatorextendsi_sizetohaveanunalignedlength;weround+*uptheextentsizeandseti_sizetotheunalignedend.+*/+if(start+encoded->len<inode->i_size&&+!IS_ALIGNED(start+encoded->len,fs_info->sectorsize))+return-EINVAL;++/* Finally, the offset in the unencoded data must be sector-aligned. */+if(!IS_ALIGNED(encoded->unencoded_offset,fs_info->sectorsize))+return-EINVAL;++num_bytes=ALIGN(encoded->len,fs_info->sectorsize);+ram_bytes=ALIGN(encoded->unencoded_len,fs_info->sectorsize);+end=start+num_bytes-1;++/*+*Iftheextentcannotbeinline,thecompresseddataondiskmustbe+*sector-aligned.Forconvenience,weextenditwithzeroesifit+*isn't.+*/+disk_num_bytes=ALIGN(orig_count,fs_info->sectorsize);+nr_pages=DIV_ROUND_UP(disk_num_bytes,PAGE_SIZE);+pages=kvcalloc(nr_pages,sizeof(structpage*),GFP_KERNEL_ACCOUNT);+if(!pages)+return-ENOMEM;+for(i=0;i<nr_pages;i++){+size_tbytes=min_t(size_t,PAGE_SIZE,iov_iter_count(from));+char*kaddr;++pages[i]=alloc_page(GFP_KERNEL_ACCOUNT|__GFP_HIGHMEM);+if(!pages[i]){+ret=-ENOMEM;+gotoout_pages;+}+kaddr=kmap(pages[i]);+if(copy_from_iter(kaddr,bytes,from)!=bytes){+kunmap(pages[i]);+ret=-EFAULT;+gotoout_pages;+}+if(bytes<PAGE_SIZE)+memset(kaddr+bytes,0,PAGE_SIZE-bytes);+kunmap(pages[i]);+}++for(;;){+structbtrfs_ordered_extent*ordered;++ret=btrfs_wait_ordered_range(inode,start,num_bytes);+if(ret)+gotoout_pages;+ret=invalidate_inode_pages2_range(inode->i_mapping,+start>>PAGE_SHIFT,+end>>PAGE_SHIFT);+if(ret)+gotoout_pages;+lock_extent_bits(io_tree,start,end,&cached_state);+ordered=btrfs_lookup_ordered_range(BTRFS_I(inode),start,+num_bytes);+if(!ordered&&+!filemap_range_has_page(inode->i_mapping,start,end))+break;+if(ordered)+btrfs_put_ordered_extent(ordered);+unlock_extent_cached(io_tree,start,end,&cached_state);+cond_resched();+}++/*+*Wedon'tusethehigher-leveldelallocspacefunctionsbecauseour+*num_bytesanddisk_num_bytesaredifferent.+*/+ret=btrfs_alloc_data_chunk_ondemand(BTRFS_I(inode),disk_num_bytes);+if(ret)+gotoout_unlock;+ret=btrfs_qgroup_reserve_data(BTRFS_I(inode),&data_reserved,start,+num_bytes);+if(ret)+gotoout_free_data_space;+ret=btrfs_delalloc_reserve_metadata(BTRFS_I(inode),num_bytes,+disk_num_bytes);+if(ret)+gotoout_qgroup_free_data;++/* Try an inline extent first. */+if(start==0&&encoded->unencoded_len==encoded->len&&+encoded->unencoded_offset==0){+ret=cow_file_range_inline(BTRFS_I(inode),encoded->len,+orig_count,compression,pages,+true);+if(ret<=0){+if(ret==0)+ret=orig_count;+gotoout_delalloc_release;+}+}++ret=btrfs_reserve_extent(root,disk_num_bytes,disk_num_bytes,+disk_num_bytes,0,0,&ins,1,1);+if(ret)+gotoout_delalloc_release;+extent_reserved=true;++em=create_io_em(BTRFS_I(inode),start,num_bytes,+start-encoded->unencoded_offset,ins.objectid,+ins.offset,ins.offset,ram_bytes,compression,+BTRFS_ORDERED_COMPRESSED);+if(IS_ERR(em)){+ret=PTR_ERR(em);+gotoout_free_reserved;+}+free_extent_map(em);++ret=btrfs_add_ordered_extent(BTRFS_I(inode),start,num_bytes,+ram_bytes,ins.objectid,ins.offset,+encoded->unencoded_offset,+(1<<BTRFS_ORDERED_ENCODED)|+(1<<BTRFS_ORDERED_COMPRESSED),+compression);+if(ret){+btrfs_drop_extent_cache(BTRFS_I(inode),start,end,0);+gotoout_free_reserved;+}+btrfs_dec_block_group_reservations(fs_info,ins.objectid);++if(start+encoded->len>inode->i_size)+i_size_write(inode,start+encoded->len);++unlock_extent_cached(io_tree,start,end,&cached_state);++btrfs_delalloc_release_extents(BTRFS_I(inode),num_bytes);++if(btrfs_submit_compressed_write(BTRFS_I(inode),start,num_bytes,+ins.objectid,ins.offset,pages,+nr_pages,0,NULL,false)){+btrfs_writepage_endio_finish_ordered(BTRFS_I(inode),pages[0],+start,end,0);+ret=-EIO;+gotoout_pages;+}+ret=orig_count;+gotoout;++out_free_reserved:+btrfs_dec_block_group_reservations(fs_info,ins.objectid);+btrfs_free_reserved_extent(fs_info,ins.objectid,ins.offset,1);+out_delalloc_release:+btrfs_delalloc_release_extents(BTRFS_I(inode),num_bytes);+btrfs_delalloc_release_metadata(BTRFS_I(inode),disk_num_bytes,+ret<0);+out_qgroup_free_data:+if(ret<0){+btrfs_qgroup_free_data(BTRFS_I(inode),data_reserved,start,+num_bytes);+}+out_free_data_space:+/*+*Ifbtrfs_reserve_extent()succeeded,thenwealreadydecremented+*bytes_may_use.+*/+if(!extent_reserved)+btrfs_free_reserved_data_space_noquota(fs_info,disk_num_bytes);+out_unlock:+unlock_extent_cached(io_tree,start,end,&cached_state);+out_pages:+for(i=0;i<nr_pages;i++){+if(pages[i])+__free_page(pages[i]);+}+kvfree(pages);+out:+if(ret>=0)+iocb->ki_pos+=encoded->len;+returnret;+}+#ifdef CONFIG_SWAP/**Addanentryindicatingablockgroupordevicewhichispinnedbya
@@ -103,6 +103,8 @@ struct btrfs_ioctl_encoded_io_args_32 {#define BTRFS_IOC_ENCODED_READ_32 _IOR(BTRFS_IOCTL_MAGIC, 64, \structbtrfs_ioctl_encoded_io_args_32)+#define BTRFS_IOC_ENCODED_WRITE_32 _IOW(BTRFS_IOCTL_MAGIC, 64, \+structbtrfs_ioctl_encoded_io_args_32)#endif/* Mask out flags that are inappropriate for the given type of inode. */
@@ -74,6 +74,8 @@ enum {BTRFS_ORDERED_LOGGED_CSUM,/* We wait for this extent to complete in the current transaction */BTRFS_ORDERED_PENDING,+/* RWF_ENCODED I/O */+BTRFS_ORDERED_ENCODED,};/* BTRFS_ORDERED_* flags that specify the type of the extent. */
@@ -81,7 +83,8 @@ enum {(1UL<<BTRFS_ORDERED_NOCOW)|\(1UL<<BTRFS_ORDERED_PREALLOC)|\(1UL<<BTRFS_ORDERED_COMPRESSED)|\-(1UL<<BTRFS_ORDERED_DIRECT))+(1UL<<BTRFS_ORDERED_DIRECT)|\+(1UL<<BTRFS_ORDERED_ENCODED))structbtrfs_ordered_extent{/* logical offset in the file */
From: Omar Sandoval <redacted>
For encoded writes, we need the raw pages for reading compressed data
directly via a bio. So, replace kvmalloc() with vmap() so we have access
to the raw pages. 144k is large enough that it usually gets allocated
with vmalloc(), anyways.
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/send.c | 33 +++++++++++++++++++++++++++++++--
1 file changed, 31 insertions(+), 2 deletions(-)
@@ -81,6 +81,7 @@ struct send_ctx {char*send_buf;u32send_size;u32send_max_size;+structpage**send_buf_pages;u64total_send_size;u64cmd_send_size[BTRFS_SEND_C_MAX+1];u64flags;/* 'flags' member of btrfs_ioctl_send_args is u64 */
From: Omar Sandoval <redacted>
This adds the definitions of the new commands for send stream version 2
and their respective attributes: fallocate, FS_IOC_SETFLAGS (a.k.a.
chattr), and encoded writes. It also documents two changes to the send
stream format in v2: the receiver shouldn't assume a maximum command
size, and the DATA attribute is encoded differently to allow for writes
larger than 64k. These will be implemented in subsequent changes, and
then the ioctl will accept the new flags.
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/send.c | 2 +-
fs/btrfs/send.h | 30 +++++++++++++++++++++++++++++-
include/uapi/linux/btrfs.h | 13 +++++++++++++
3 files changed, 43 insertions(+), 2 deletions(-)
@@ -114,6 +130,18 @@ enum {BTRFS_SEND_A_CLONE_OFFSET,BTRFS_SEND_A_CLONE_LEN,+/* The following attributes were added in send stream v2. */++BTRFS_SEND_A_FALLOCATE_MODE,++BTRFS_SEND_A_SETFLAGS_FLAGS,++BTRFS_SEND_A_UNENCODED_FILE_LEN,+BTRFS_SEND_A_UNENCODED_LEN,+BTRFS_SEND_A_UNENCODED_OFFSET,+BTRFS_SEND_A_COMPRESSION,+BTRFS_SEND_A_ENCRYPTION,+__BTRFS_SEND_A_MAX,};#define BTRFS_SEND_A_MAX (__BTRFS_SEND_A_MAX - 1)
From: Omar Sandoval <redacted>
Now that the new support is implemented, allow the ioctl to accept the
flags and update the version in sysfs.
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/send.c | 10 +++++++++-
fs/btrfs/send.h | 2 +-
include/uapi/linux/btrfs.h | 4 +++-
3 files changed, 13 insertions(+), 3 deletions(-)
From: Omar Sandoval <redacted>
Now that all of the pieces are in place, we can use the ENCODED_WRITE
command to send compressed extents when appropriate.
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/ctree.h | 4 +
fs/btrfs/inode.c | 6 +-
fs/btrfs/send.c | 230 +++++++++++++++++++++++++++++++++++++++++++----
3 files changed, 220 insertions(+), 20 deletions(-)
From: Boris Burkov <redacted>
In send stream v2, write commands can now be an arbitrary size. For that
reason, we can no longer allocate a fixed array in sctx for read_cmd.
Instead, read_cmd dynamically allocates sctx->read_buf. To avoid
needless reallocations, we reuse read_buf between read_cmd calls by also
keeping track of the size of the allocated buffer in sctx->read_buf_sz.
We do the first allocation of the old default size at the start of
processing the stream, and we only reallocate if we encounter a command
that needs a larger buffer.
Signed-off-by: Boris Burkov <redacted>
---
common/send-stream.c | 55 ++++++++++++++++++++++++++++----------------
send.h | 2 +-
2 files changed, 36 insertions(+), 21 deletions(-)
@@ -111,11 +111,12 @@ static int read_cmd(struct btrfs_send_stream *sctx)u32pos;u32crc;u32crc2;+structbtrfs_cmd_header*cmd_hdr;+size_tbuf_len;memset(sctx->cmd_attrs,0,sizeof(sctx->cmd_attrs));-ASSERT(sizeof(*sctx->cmd_hdr)<=sizeof(sctx->read_buf));-ret=read_buf(sctx,sctx->read_buf,sizeof(*sctx->cmd_hdr));+ret=read_buf(sctx,sctx->read_buf,sizeof(*cmd_hdr));if(ret<0)gotoout;if(ret){
@@ -124,18 +125,22 @@ static int read_cmd(struct btrfs_send_stream *sctx)gotoout;}-sctx->cmd_hdr=(structbtrfs_cmd_header*)sctx->read_buf;-cmd=le16_to_cpu(sctx->cmd_hdr->cmd);-cmd_len=le32_to_cpu(sctx->cmd_hdr->len);--if(cmd_len+sizeof(*sctx->cmd_hdr)>=sizeof(sctx->read_buf)){-ret=-EINVAL;-error("command length %u too big for buffer %zu",-cmd_len,sizeof(sctx->read_buf));-gotoout;+cmd_hdr=(structbtrfs_cmd_header*)sctx->read_buf;+cmd_len=le32_to_cpu(cmd_hdr->len);+cmd=le16_to_cpu(cmd_hdr->cmd);+buf_len=sizeof(*cmd_hdr)+cmd_len;+if(sctx->read_buf_sz<buf_len){+sctx->read_buf=realloc(sctx->read_buf,buf_len);+if(!sctx->read_buf){+ret=-ENOMEM;+error("failed to reallocate read buffer for cmd");+gotoout;+}+sctx->read_buf_sz=buf_len;+/* We need to reset cmd_hdr after realloc of sctx->read_buf */+cmd_hdr=(structbtrfs_cmd_header*)sctx->read_buf;}--data=sctx->read_buf+sizeof(*sctx->cmd_hdr);+data=sctx->read_buf+sizeof(*cmd_hdr);ret=read_buf(sctx,data,cmd_len);if(ret<0)gotoout;
@@ -145,11 +150,12 @@ static int read_cmd(struct btrfs_send_stream *sctx)gotoout;}-crc=le32_to_cpu(sctx->cmd_hdr->crc);-sctx->cmd_hdr->crc=0;+crc=le32_to_cpu(cmd_hdr->crc);+/* in send, crc is computed with header crc = 0, replicate that */+cmd_hdr->crc=0;crc2=crc32c(0,(unsignedchar*)sctx->read_buf,-sizeof(*sctx->cmd_hdr)+cmd_len);+sizeof(*cmd_hdr)+cmd_len);if(crc!=crc2){ret=-EINVAL;
@@ -524,19 +530,28 @@ int btrfs_read_and_process_send_stream(int fd,gotoout;}+sctx.read_buf=malloc(BTRFS_SEND_BUF_SIZE_V1);+if(!sctx.read_buf){+ret=-ENOMEM;+error("unable to allocate send stream read buffer");+gotoout;+}+sctx.read_buf_sz=BTRFS_SEND_BUF_SIZE_V1;+while(1){ret=read_and_process_cmd(&sctx);if(ret<0){last_err=ret;errors++;if(max_errors>0&&errors>=max_errors)-gotoout;+break;}elseif(ret>0){if(!honor_end_cmd)ret=0;-gotoout;+break;}}+free(sctx.read_buf);out:if(last_err&&!ret)
From: Boris Burkov <redacted>
An encoded extent can be up to 128K in length, which exceeds the largest
value expressible by the current send stream format's 16 bit tlv_len
field. Since encoded writes cannot be split into multiple writes by
btrfs send, the send stream format must change to accommodate encoded
writes.
Supporting this changed format requires retooling how we store the
commands we have processed. Since we can no longer use btrfs_tlv_header
to describe every attribute, we define a new struct btrfs_send_attribute
which has a 32 bit length field, and use that to store the attribute
information needed for receive processing. This is transparent to users
of the various TLV_GET macros.
Signed-off-by: Boris Burkov <redacted>
---
common/send-stream.c | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
@@ -188,15 +204,15 @@ static int tlv_get(struct btrfs_send_stream *sctx, int attr, void **data, int *lgotoout;}-hdr=sctx->cmd_attrs[attr];-if(!hdr){+send_attr=&sctx->cmd_attrs[attr];+if(!send_attr->data){error("attribute %d requested but not present",attr);ret=-ENOENT;gotoout;}-*len=le16_to_cpu(hdr->tlv_len);-*data=hdr+1;+*len=send_attr->tlv_len;+*data=send_attr->data;ret=0;
From: Boris Burkov <redacted>
The new format privileges the BTRFS_SEND_A_DATA attribute by
guaranteeing it will always be the last attribute in any command that
needs it, and by implicitly encoding the data length as the difference
between the total command length in the command header and the sizes of
the rest of the attributes (and of course the tlv_type identifying the
DATA attribute). To parse the new stream, we must read the tlv_type and
if it is not DATA, we proceed normally, but if it is DATA, we don't
parse a tlv_len but simply compute the length.
In addition, we add some bounds checking when parsing each chunk of
data, as well as for the tlv_len itself.
Signed-off-by: Boris Burkov <redacted>
---
common/send-stream.c | 36 ++++++++++++++++++++++++++----------
1 file changed, 26 insertions(+), 10 deletions(-)
From: Boris Burkov <redacted>
Send stream v2 adds three commands and several attributes associated to
those commands. Before we implement processing them, add all the
commands and attributes. This avoids leaving the enums in an
intermediate state that doesn't correspond to any version of send
stream.
Signed-off-by: Boris Burkov <redacted>
---
send.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)
From: Boris Burkov <redacted>
Add a new btrfs_send_op and support for both dumping and proper receive
processing which does actual encoded writes.
Encoded writes are only allowed on a file descriptor opened with an
extra flag that allows encoded writes, so we also add support for this
flag when opening or reusing a file for writing.
Signed-off-by: Boris Burkov <redacted>
---
cmds/receive-dump.c | 16 +++++-
cmds/receive.c | 47 +++++++++++++++
common/send-stream.c | 22 ++++++++
common/send-stream.h | 4 ++
ioctl.h | 132 +++++++++++++++++++++++++++++++++++++++++++
5 files changed, 220 insertions(+), 1 deletion(-)
@@ -775,6 +775,134 @@ struct btrfs_ioctl_get_subvol_rootref_args {};BUILD_ASSERT(sizeof(structbtrfs_ioctl_get_subvol_rootref_args)==4096);+/*+*Dataandmetadataforanencodedreadorwrite.+*+*EncodedI/Obypassesanyencodingautomaticallydonebythefilesystem(e.g.,+*compression).Thiscanbeusedtoreadthecompressedcontentsofafileor+*writepre-compresseddatadirectlytoafile.+*+*BTRFS_IOC_ENCODED_READandBTRFS_IOC_ENCODED_WRITEareessentially+*preadv/pwritevwithadditionalmetadataabouthowthedataisencodedandthe+*sizeoftheunencodeddata.+*+*BTRFS_IOC_ENCODED_READfillsthegiveniovecswiththeencodeddata,fills+*themetadatafields,andreturnsthesizeoftheencodeddata.Itreadsone+*extentpercall.Itcanalsoreaddatawhichisnotencoded.+*+*BTRFS_IOC_ENCODED_WRITEusesthemetadatafields,writestheencodeddata+*fromtheiovecs,andreturnsthesizeoftheencodeddata.Notethatthe+*encodeddataisnotvalidatedwhenitiswritten;ifitisnotvalid(e.g.,+*itcannotbedecompressed),thenasubsequentreadmayreturnanerror.+*+*Sincethefilesystempagecachecontainsdecodeddata,encodedI/Obypasses+*thepagecache.EncodedI/OrequiresCAP_SYS_ADMIN.+*/+structbtrfs_ioctl_encoded_io_args{+/* Input parameters for both reads and writes. */++/*+*iovecscontainingencodeddata.+*+*Forreads,ifthesizeoftheencodeddataislargerthanthesumof+*iov[n].iov_lenfor0<=n<iovcnt,thentheioctlfailswith+*ENOBUFS.+*+*Forwrites,thesizeoftheencodeddataisthesumofiov[n].iov_len+*for0<=n<iovcnt.Thismustbelessthan128KiB(thislimitmay+*increaseinthefuture).Thismustalsobelessthanorequalto+*unencoded_len.+*/+conststructiovec*iov;+/* Number of iovecs. */+unsignedlongiovcnt;+/*+*Offsetinfile.+*+*Forwrites,mustbealignedtothesectorsizeofthefilesystem.+*/+__s64offset;+/* Currently must be zero. */+__u64flags;++/*+*Forreads,thefollowingmembersarefilledinwiththemetadatafor+*theencodeddata.+*Forwrites,thefollowingmembersmustbesettothemetadataforthe+*encodeddata.+*/++/*+*Lengthofthedatainthefile.+*+*Mustbelessthanorequaltounencoded_len-unencoded_offset.For+*writes,mustbealignedtothesectorsizeofthefilesystemunless+*thedataendsatorbeyondthecurrentendofthefile.+*/+__u64len;+/*+*Lengthoftheunencoded(i.e.,decryptedanddecompressed)data.+*+*Forwrites,mustbenomorethan128KiB(thislimitmayincreasein+*thefuture).Iftheunencodeddataisactuallylongerthan+*unencoded_len,thenitistruncated;ifitisshorter,thenitis+*extendedwithzeroes.+*/+__u64unencoded_len;+/*+*Offsetfromthefirstbyteoftheunencodeddatatothefirstbyteof+*logicaldatainthefile.+*+*Mustbelessthanunencoded_len.+*/+__u64unencoded_offset;+/*+*BTRFS_ENCODED_IO_COMPRESSION_*type.+*+*Forwrites,mustnotbeBTRFS_ENCODED_IO_COMPRESSION_NONE.+*/+__u32compression;+/* Currently always BTRFS_ENCODED_IO_ENCRYPTION_NONE. */+__u32encryption;+/*+*Reservedforfutureexpansion.+*+*Forreads,alwaysreturnedaszero.Usersshouldcheckfornon-zero+*bytes.Ifthereareany,thenthekernelhasanewerversionofthis+*structurewithadditionalinformationthattheuserdefinitionis+*missing.+*+*Forwrites,mustbezeroed.+*/+__u8reserved[32];+};++/* Data is not compressed. */+#define BTRFS_ENCODED_IO_COMPRESSION_NONE 0+/* Data is compressed as a single zlib stream. */+#define BTRFS_ENCODED_IO_COMPRESSION_ZLIB 1+/*+*DataiscompressedasasinglezstdframewiththewindowLogcompression+*parametersettonomorethan17.+*/+#define BTRFS_ENCODED_IO_COMPRESSION_ZSTD 2+/*+*Dataiscompressedpagebypage(usingthepagesizeindicatedbythenameof+*theconstant)withLZO1Xandwrappedintheformatdocumentedin+*fs/btrfs/lzo.c.Forwrites,thecompressionpagesizemustmatchthe+*filesystempagesize.+*/+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_4K 3+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_8K 4+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_16K 5+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_32K 6+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_64K 7+#define BTRFS_ENCODED_IO_COMPRESSION_TYPES 8++/* Data is not encrypted. */+#define BTRFS_ENCODED_IO_ENCRYPTION_NONE 0+#define BTRFS_ENCODED_IO_ENCRYPTION_TYPES 1+/* Error codes as returned by the kernel */enumbtrfs_err_code{notused,
From: Boris Burkov <redacted>
Send stream v2 can emit fallocate commands, so receive must support them
as well. The implementation simply passes along the arguments to the
syscall. Note that mode is encoded as a u32 in send stream but fallocate
takes an int, so there is a unsigned->signed conversion there.
Signed-off-by: Boris Burkov <redacted>
---
cmds/receive-dump.c | 9 +++++++++
cmds/receive.c | 25 +++++++++++++++++++++++++
common/send-stream.c | 9 +++++++++
common/send-stream.h | 2 ++
4 files changed, 45 insertions(+)
From: Boris Burkov <redacted>
In send stream v2, send can emit a command for setting inode flags via
the setflags ioctl. Pass the flags attribute through to the ioctl call
in receive.
Signed-off-by: Boris Burkov <redacted>
---
cmds/receive-dump.c | 6 ++++++
cmds/receive.c | 25 +++++++++++++++++++++++++
common/send-stream.c | 7 +++++++
common/send-stream.h | 1 +
4 files changed, 39 insertions(+)
From: Boris Burkov <redacted>
An encoded_write can fail if the file system it is being applied to does
not support encoded writes or if it can't find enough contiguous space
to accommodate the encoded extent. In those cases, we can likely still
process an encoded_write by explicitly decoding the data and doing a
normal write.
Add the necessary fallback path for decoding data compressed with zlib,
lzo, or zstd. zlib and zstd have reusable decoding context data
structures which we cache in the receive context so that we don't have
to recreate them on every encoded_write.
Finally, add a command line flag for force-decompress which causes
receive to always use the fallback path rather than first attempting the
encoded write.
Signed-off-by: Boris Burkov <redacted>
---
Documentation/btrfs-receive.asciidoc | 4 +
cmds/receive.c | 266 ++++++++++++++++++++++++++-
2 files changed, 261 insertions(+), 9 deletions(-)
@@ -60,6 +60,10 @@ By default the mountpoint is searched in '/proc/self/mounts'. If '/proc' is not accessible, eg. in a chroot environment, use this option to tell us where this filesystem is mounted.+--force-decompress::+if the stream contains compressed data (see '--compressed-data' in+`btrfs-send`(8)), always decompress it instead of writing it with encoded I/O.+ --dump:: dump the stream metadata, one line per operation +
@@ -989,9 +999,222 @@ static int process_update_extent(const char *path, u64 offset, u64 len,return0;}+staticintdecompress_zlib(structbtrfs_receive*rctx,constchar*encoded_data,+u64encoded_len,char*unencoded_data,+u64unencoded_len)+{+boolinit=false;+intret;++if(!rctx->zlib_stream){+init=true;+rctx->zlib_stream=malloc(sizeof(z_stream));+if(!rctx->zlib_stream){+error("failed to allocate zlib stream %m");+return-ENOMEM;+}+}+rctx->zlib_stream->next_in=(void*)encoded_data;+rctx->zlib_stream->avail_in=encoded_len;+rctx->zlib_stream->next_out=(void*)unencoded_data;+rctx->zlib_stream->avail_out=unencoded_len;++if(init){+rctx->zlib_stream->zalloc=Z_NULL;+rctx->zlib_stream->zfree=Z_NULL;+rctx->zlib_stream->opaque=Z_NULL;+ret=inflateInit(rctx->zlib_stream);+}else{+ret=inflateReset(rctx->zlib_stream);+}+if(ret!=Z_OK){+error("zlib inflate init failed: %d",ret);+return-EIO;+}++while(rctx->zlib_stream->avail_in>0&&+rctx->zlib_stream->avail_out>0){+ret=inflate(rctx->zlib_stream,Z_FINISH);+if(ret==Z_STREAM_END){+break;+}elseif(ret!=Z_OK){+error("zlib inflate failed: %d",ret);+return-EIO;+}+}+return0;+}++staticintdecompress_zstd(structbtrfs_receive*rctx,constchar*encoded_buf,+u64encoded_len,char*unencoded_buf,+u64unencoded_len)+{+ZSTD_inBufferin_buf={+.src=encoded_buf,+.size=encoded_len+};+ZSTD_outBufferout_buf={+.dst=unencoded_buf,+.size=unencoded_len+};+size_tret;++if(!rctx->zstd_dstream){+rctx->zstd_dstream=ZSTD_createDStream();+if(!rctx->zstd_dstream){+error("failed to create zstd dstream");+return-ENOMEM;+}+}+ret=ZSTD_initDStream(rctx->zstd_dstream);+if(ZSTD_isError(ret)){+error("failed to init zstd stream: %s",ZSTD_getErrorName(ret));+return-EIO;+}+while(in_buf.pos<in_buf.size&&out_buf.pos<out_buf.size){+ret=ZSTD_decompressStream(rctx->zstd_dstream,&out_buf,&in_buf);+if(ret==0){+break;+}elseif(ZSTD_isError(ret)){+error("failed to decompress zstd stream: %s",+ZSTD_getErrorName(ret));+return-EIO;+}+}+return0;+}++staticintdecompress_lzo(constchar*encoded_data,u64encoded_len,+char*unencoded_data,u64unencoded_len,+unsignedintpage_size)+{+uint32_ttotal_len;+size_tin_pos,out_pos;++if(encoded_len<4){+error("lzo header is truncated");+return-EIO;+}+memcpy(&total_len,encoded_data,4);+total_len=le32toh(total_len);+if(total_len>encoded_len){+error("lzo header is invalid");+return-EIO;+}++in_pos=4;+out_pos=0;+while(in_pos<total_len&&out_pos<unencoded_len){+size_tpage_remaining;+uint32_tsrc_len;+lzo_uintdst_len;+intret;++page_remaining=-in_pos%page_size;+if(page_remaining<4){+if(total_len-in_pos<=page_remaining)+break;+in_pos+=page_remaining;+}++if(total_len-in_pos<4){+error("lzo segment header is truncated");+return-EIO;+}++memcpy(&src_len,encoded_data+in_pos,4);+src_len=le32toh(src_len);+in_pos+=4;+if(src_len>total_len-in_pos){+error("lzo segment header is invalid");+return-EIO;+}++dst_len=page_size;+ret=lzo1x_decompress_safe((void*)(encoded_data+in_pos),+src_len,+(void*)(unencoded_data+out_pos),+&dst_len,NULL);+if(ret!=LZO_E_OK){+error("lzo1x_decompress_safe failed: %d",ret);+return-EIO;+}++in_pos+=src_len;+out_pos+=dst_len;+}+return0;+}++staticintdecompress_and_write(structbtrfs_receive*rctx,+constchar*encoded_data,u64offset,+u64encoded_len,u64unencoded_file_len,+u64unencoded_len,u64unencoded_offset,+u32compression)+{+intret=0;+size_tpos;+ssize_tw;+char*unencoded_data;+intpage_shift;++unencoded_data=calloc(unencoded_len,1);+if(!unencoded_data){+error("allocating space for unencoded data failed: %m");+return-errno;+}++switch(compression){+caseBTRFS_ENCODED_IO_COMPRESSION_ZLIB:+ret=decompress_zlib(rctx,encoded_data,encoded_len,+unencoded_data,unencoded_len);+if(ret)+gotoout;+break;+caseBTRFS_ENCODED_IO_COMPRESSION_ZSTD:+ret=decompress_zstd(rctx,encoded_data,encoded_len,+unencoded_data,unencoded_len);+if(ret)+gotoout;+break;+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_4K:+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_8K:+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_16K:+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_32K:+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_64K:+page_shift=compression-BTRFS_ENCODED_IO_COMPRESSION_LZO_4K+12;+ret=decompress_lzo(encoded_data,encoded_len,unencoded_data,+unencoded_len,1U<<page_shift);+if(ret)+gotoout;+break;+default:+error("unknown compression: %d",compression);+ret=-EOPNOTSUPP;+gotoout;+}++pos=unencoded_offset;+while(pos<unencoded_file_len){+w=pwrite(rctx->write_fd,unencoded_data+pos,+unencoded_file_len-pos,offset);+if(w<0){+ret=-errno;+error("writing unencoded data failed: %m");+gotoout;+}+pos+=w;+offset+=w;+}+out:+free(unencoded_data);+returnret;+}+staticintprocess_encoded_write(constchar*path,constvoid*data,u64offset,-u64len,u64unencoded_file_len,u64unencoded_len,-u64unencoded_offset,u32compression,u32encryption,void*user)+u64len,u64unencoded_file_len,+u64unencoded_len,u64unencoded_offset,+u32compression,u32encryption,void*user){intret;structbtrfs_receive*rctx=user;
@@ -1007,6 +1230,7 @@ static int process_encoded_write(const char *path, const void *data, u64 offset,.compression=compression,.encryption=encryption,};+boolencoded_write=!rctx->force_decompress;if(encryption){error("encoded_write: encryption not supported");
@@ -1023,13 +1247,21 @@ static int process_encoded_write(const char *path, const void *data, u64 offset,if(ret<0)returnret;-ret=ioctl(rctx->write_fd,BTRFS_IOC_ENCODED_WRITE,&encoded);-if(ret<0){-ret=-errno;-error("encoded_write: writing to %s failed: %m",path);-returnret;+if(encoded_write){+ret=ioctl(rctx->write_fd,BTRFS_IOC_ENCODED_WRITE,&encoded);+if(ret>=0)+return0;+/* Fall back for these errors, fail hard for anything else. */+if(errno!=ENOSPC&&errno!=EOPNOTSUPP&&errno!=EINVAL){+ret=-errno;+error("encoded_write: writing to %s failed: %m",path);+returnret;+}}-return0;++returndecompress_and_write(rctx,data,offset,len,unencoded_file_len,+unencoded_len,unencoded_offset,+compression);}staticstructbtrfs_send_opssend_ops={
@@ -1239,6 +1477,9 @@ static const char * const cmd_receive_usage[] = {"-m ROOTMOUNT the root mount point of the destination filesystem."," If /proc is not accessible, use this to tell us where"," this file system is mounted.",+"--force-decompress",+" if the stream contains compressed data, always",+" decompress it instead of writing it with encoded I/O","--dump dump stream metadata, one line per operation,"," does not require the MOUNT parameter","-v deprecated, alias for global -v option",
@@ -1282,12 +1523,16 @@ static int cmd_receive(const struct cmd_struct *cmd, int argc, char **argv)optind=0;while(1){intc;-enum{GETOPT_VAL_DUMP=257};+enum{+GETOPT_VAL_DUMP=257,+GETOPT_VAL_FORCE_DECOMPRESS,+};staticconststructoptionlong_opts[]={{"max-errors",required_argument,NULL,'E'},{"chroot",no_argument,NULL,'C'},{"dump",no_argument,NULL,GETOPT_VAL_DUMP},{"quiet",no_argument,NULL,'q'},+{"force-decompress",no_argument,NULL,GETOPT_VAL_FORCE_DECOMPRESS},{NULL,0,NULL,0}};
@@ -1330,6 +1575,9 @@ static int cmd_receive(const struct cmd_struct *cmd, int argc, char **argv)caseGETOPT_VAL_DUMP:dump=1;break;+caseGETOPT_VAL_FORCE_DECOMPRESS:+rctx.force_decompress=1;+break;default:usage_unknown_option(cmd,argv);}
From: Boris Burkov <redacted>
To make the btrfs send ioctl use the stream v2 format requires passing
BTRFS_SEND_FLAG_STREAM_V2 in flags. Further, to cause the ioctl to emit
encoded_write commands for encoded extents, we must set that flag as
well as BTRFS_SEND_FLAG_COMPRESSED. Finally, we bump up the version in
send.h as well, since we are now fully compatible with v2.
Add two command line arguments to btrfs send: --stream-version and
--compressed-data. --stream-version requires an argument which it parses
as an integer and sets STREAM_V2 if the argument is 2. --compressed-data
does not require an argument and automatically implies STREAM_V2 as well
(COMPRESSED alone causes the ioctl to error out).
Some examples to illustrate edge cases:
// v1, old format and no encoded_writes
btrfs send subvol
btrfs send --stream-version 1 subvol
// v2 and compressed, we will see encoded_writes
btrfs send --compressed-data subvol
btrfs send --compressed-data --stream-version 2 subvol
// v2 only, new format but no encoded_writes
btrfs send --stream-version 2 subvol
// error: compressed needs version >= 2
btrfs send --compressed-data --stream-version 1 subvol
// error: invalid version (not 1 or 2)
btrfs send --stream-version 3 subvol
btrfs send --compressed-data --stream-version 0 subvol
btrfs send --compressed-data --stream-version 10 subvol
Signed-off-by: Boris Burkov <redacted>
---
Documentation/btrfs-send.asciidoc | 16 ++++++++-
cmds/send.c | 54 ++++++++++++++++++++++++++++++-
ioctl.h | 17 +++++++++-
libbtrfsutil/btrfs.h | 17 +++++++++-
send.h | 2 +-
5 files changed, 101 insertions(+), 5 deletions(-)
@@ -55,7 +55,21 @@ send in 'NO_FILE_DATA' mode The output stream does not contain any file data and thus cannot be used to transfer changes. This mode is faster and is useful to show the differences in metadata.--q|--quiet::::++--stream-version <1|2>::+Use the given send stream version. The default is 1. Version 2 encodes file+data slightly more efficiently; it is also required for sending compressed data+directly (see '--compressed-data'). Version 2 requires at least btrfs-progs+5.12 on both the sender and receiver and at least Linux 5.12 on the sender.++--compressed-data::+Send data that is compressed on the filesystem directly without decompressing+it. If the receiver supports encoded I/O (see `encoded_io`(7)), it can also+write it directly without decompressing it. Otherwise, the receiver will fall+back to decompressing it and writing it normally. This implies+'--stream-version 2'.++-q|--quiet:: (deprecated) alias for global '-q' option -v|--verbose:: (deprecated) alias for global '-v' option
@@ -452,6 +452,21 @@ static const char * const cmd_send_usage[] = {" does not contain any file data and thus cannot be used"," to transfer changes. This mode is faster and useful to"," show the differences in metadata.",+"--stream-version <1|2>",+" Use the given send stream version. The default is",+" 1. Version 2 encodes file data slightly more",+" efficiently; it is also required for sending",+" compressed data directly (see --compressed-data).",+" Version 2 requires at least btrfs-progs 5.12 on both",+" the sender and receiver and at least Linux 5.12 on the",+" sender.",+"--compressed-data",+" Send data that is compressed on the filesystem",+" directly without decompressing it. If the receiver",+" supports encoded I/O, it can also write it directly",+" without decompressing it. Otherwise, the receiver will",+" fall back to decompressing it and writing it normally.",+" This implies --stream-version 2.","-v|--verbose deprecated, alias for global -v option","-q|--quiet deprecated, alias for global -q option",HELPINFO_INSERT_GLOBALS,
From: Boris Burkov <redacted>
Adapt the existing send/receive tests by passing '-o --force-compress'
to the mount commands in a new test. After writing a few files in the
various compression formats, send/receive them with and without
--force-decompress to test both the encoded_write path and the
fallback to decode+write.
Signed-off-by: Boris Burkov <redacted>
---
.../049-receive-write-encoded/test.sh | 114 ++++++++++++++++++
1 file changed, 114 insertions(+)
create mode 100755 tests/misc-tests/049-receive-write-encoded/test.sh
@@ -0,0 +1,114 @@+#!/bin/bash+#+# test that we can send and receive encoded writes for three modes of+# transparent compression: zlib, lzo, and zstd.++source"$TEST_TOP/common"++check_prereqmkfs.btrfs+check_prereqbtrfs++setup_root_helper+prepare_test_dev++here=`pwd`++# assumes the filesystem exists, and does mount, write, snapshot, send, unmount+# for the specified encoding option+send_one(){+localstr+localsubv+localsnap++algorithm="$1"+shift+str="$1"+shift++subv="subv-$algorithm"+snap="snap-$algorithm"++run_check_mount_test_dev"-o""compress-force=$algorithm"+cd"$TEST_MNT"||_fail"cannot chdir to TEST_MNT"++run_check$SUDO_HELPER"$TOP/btrfs"subvolumecreate"$subv"+run_check$SUDO_HELPERddif=/dev/zeroof="$subv/file1"bs=1Mcount=1+run_check$SUDO_HELPERddif=/dev/zeroof="$subv/file2"bs=500Kcount=1+run_check$SUDO_HELPER"$TOP/btrfs"subvolumesnapshot-r"$subv""$snap"+run_check$SUDO_HELPER"$TOP/btrfs"send-f"$str""$snap""$@"++cd"$here"||_fail"cannot chdir back to test directory"+run_check_umount_test_dev+}++receive_one(){+localstr+str="$1"+shift++run_check_mkfs_test_dev+run_check_mount_test_dev+run_check$SUDO_HELPER"$TOP/btrfs"receive"$@"-v-f"$str""$TEST_MNT"+run_check_umount_test_dev+run_checkrm-f--"$str"+}++test_one_write_encoded(){+localstr+localalgorithm+algorithm="$1"+shift+str="$here/stream-$algorithm.stream"++run_check_mkfs_test_dev+send_one"$algorithm""$str"--compressed-data+receive_one"$str""$@"+}++test_one_stream_v1(){+localstr+localalgorithm+algorithm="$1"+shift+str="$here/stream-$algorithm.stream"++run_check_mkfs_test_dev+send_one"$algorithm""$str"--stream-version1+receive_one"$str""$@"+}++test_mix_write_encoded(){+localstrzlib+localstrlzo+localstrzstd+strzlib="$here/stream-zlib.stream"+strlzo="$here/stream-lzo.stream"+strzstd="$here/stream-zstd.stream"++run_check_mkfs_test_dev++send_one"zlib""$strzlib"--compressed-data+send_one"lzo""$strlzo"--compressed-data+send_one"zstd""$strzstd"--compressed-data++receive_one"$strzlib"+receive_one"$strlzo"+receive_one"$strzstd"+}++test_one_write_encoded"zlib"+test_one_write_encoded"lzo"+test_one_write_encoded"zstd"++# with decompression forced+test_one_write_encoded"zlib""--force-decompress"+test_one_write_encoded"lzo""--force-decompress"+test_one_write_encoded"zstd""--force-decompress"++# send stream v1+test_one_stream_v1"zlib"+test_one_stream_v1"lzo"+test_one_stream_v1"zstd"++# files use a mix of compression algorithms+test_mix_write_encoded
On Tue, Aug 17, 2021 at 02:06:52PM -0700, Omar Sandoval wrote:
quoted hunk
From: Boris Burkov <redacted>
An encoded_write can fail if the file system it is being applied to does
not support encoded writes or if it can't find enough contiguous space
to accommodate the encoded extent. In those cases, we can likely still
process an encoded_write by explicitly decoding the data and doing a
normal write.
Add the necessary fallback path for decoding data compressed with zlib,
lzo, or zstd. zlib and zstd have reusable decoding context data
structures which we cache in the receive context so that we don't have
to recreate them on every encoded_write.
Finally, add a command line flag for force-decompress which causes
receive to always use the fallback path rather than first attempting the
encoded write.
Signed-off-by: Boris Burkov <redacted>
---
Documentation/btrfs-receive.asciidoc | 4 +
cmds/receive.c | 266 ++++++++++++++++++++++++++-
2 files changed, 261 insertions(+), 9 deletions(-)
@@ -60,6 +60,10 @@ By default the mountpoint is searched in '/proc/self/mounts'. If '/proc' is not accessible, eg. in a chroot environment, use this option to tell us where this filesystem is mounted.+--force-decompress::+if the stream contains compressed data (see '--compressed-data' in+`btrfs-send`(8)), always decompress it instead of writing it with encoded I/O.+ --dump:: dump the stream metadata, one line per operation +
@@ -989,9 +999,222 @@ static int process_update_extent(const char *path, u64 offset, u64 len,return0;}+staticintdecompress_zlib(structbtrfs_receive*rctx,constchar*encoded_data,+u64encoded_len,char*unencoded_data,+u64unencoded_len)+{+boolinit=false;+intret;++if(!rctx->zlib_stream){+init=true;+rctx->zlib_stream=malloc(sizeof(z_stream));+if(!rctx->zlib_stream){+error("failed to allocate zlib stream %m");+return-ENOMEM;+}+}+rctx->zlib_stream->next_in=(void*)encoded_data;+rctx->zlib_stream->avail_in=encoded_len;+rctx->zlib_stream->next_out=(void*)unencoded_data;+rctx->zlib_stream->avail_out=unencoded_len;++if(init){+rctx->zlib_stream->zalloc=Z_NULL;+rctx->zlib_stream->zfree=Z_NULL;+rctx->zlib_stream->opaque=Z_NULL;+ret=inflateInit(rctx->zlib_stream);+}else{+ret=inflateReset(rctx->zlib_stream);+}+if(ret!=Z_OK){+error("zlib inflate init failed: %d",ret);+return-EIO;+}++while(rctx->zlib_stream->avail_in>0&&+rctx->zlib_stream->avail_out>0){+ret=inflate(rctx->zlib_stream,Z_FINISH);+if(ret==Z_STREAM_END){+break;+}elseif(ret!=Z_OK){+error("zlib inflate failed: %d",ret);+return-EIO;+}+}+return0;+}++staticintdecompress_zstd(structbtrfs_receive*rctx,constchar*encoded_buf,+u64encoded_len,char*unencoded_buf,+u64unencoded_len)+{+ZSTD_inBufferin_buf={+.src=encoded_buf,+.size=encoded_len+};+ZSTD_outBufferout_buf={+.dst=unencoded_buf,+.size=unencoded_len+};+size_tret;++if(!rctx->zstd_dstream){+rctx->zstd_dstream=ZSTD_createDStream();+if(!rctx->zstd_dstream){+error("failed to create zstd dstream");+return-ENOMEM;+}+}+ret=ZSTD_initDStream(rctx->zstd_dstream);+if(ZSTD_isError(ret)){+error("failed to init zstd stream: %s",ZSTD_getErrorName(ret));+return-EIO;+}+while(in_buf.pos<in_buf.size&&out_buf.pos<out_buf.size){+ret=ZSTD_decompressStream(rctx->zstd_dstream,&out_buf,&in_buf);+if(ret==0){+break;+}elseif(ZSTD_isError(ret)){+error("failed to decompress zstd stream: %s",+ZSTD_getErrorName(ret));+return-EIO;+}+}+return0;+}++staticintdecompress_lzo(constchar*encoded_data,u64encoded_len,+char*unencoded_data,u64unencoded_len,+unsignedintpage_size)+{+uint32_ttotal_len;+size_tin_pos,out_pos;++if(encoded_len<4){+error("lzo header is truncated");+return-EIO;+}+memcpy(&total_len,encoded_data,4);+total_len=le32toh(total_len);+if(total_len>encoded_len){+error("lzo header is invalid");+return-EIO;+}++in_pos=4;+out_pos=0;+while(in_pos<total_len&&out_pos<unencoded_len){+size_tpage_remaining;+uint32_tsrc_len;+lzo_uintdst_len;+intret;++page_remaining=-in_pos%page_size;+if(page_remaining<4){+if(total_len-in_pos<=page_remaining)+break;+in_pos+=page_remaining;+}++if(total_len-in_pos<4){+error("lzo segment header is truncated");+return-EIO;+}++memcpy(&src_len,encoded_data+in_pos,4);+src_len=le32toh(src_len);+in_pos+=4;+if(src_len>total_len-in_pos){+error("lzo segment header is invalid");+return-EIO;+}++dst_len=page_size;+ret=lzo1x_decompress_safe((void*)(encoded_data+in_pos),+src_len,+(void*)(unencoded_data+out_pos),+&dst_len,NULL);+if(ret!=LZO_E_OK){+error("lzo1x_decompress_safe failed: %d",ret);+return-EIO;+}++in_pos+=src_len;+out_pos+=dst_len;+}+return0;+}++staticintdecompress_and_write(structbtrfs_receive*rctx,+constchar*encoded_data,u64offset,+u64encoded_len,u64unencoded_file_len,+u64unencoded_len,u64unencoded_offset,+u32compression)+{+intret=0;+size_tpos;+ssize_tw;+char*unencoded_data;+intpage_shift;++unencoded_data=calloc(unencoded_len,1);+if(!unencoded_data){+error("allocating space for unencoded data failed: %m");+return-errno;+}++switch(compression){+caseBTRFS_ENCODED_IO_COMPRESSION_ZLIB:+ret=decompress_zlib(rctx,encoded_data,encoded_len,+unencoded_data,unencoded_len);+if(ret)+gotoout;+break;+caseBTRFS_ENCODED_IO_COMPRESSION_ZSTD:+ret=decompress_zstd(rctx,encoded_data,encoded_len,+unencoded_data,unencoded_len);+if(ret)+gotoout;+break;+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_4K:+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_8K:+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_16K:+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_32K:+caseBTRFS_ENCODED_IO_COMPRESSION_LZO_64K:+page_shift=compression-BTRFS_ENCODED_IO_COMPRESSION_LZO_4K+12;+ret=decompress_lzo(encoded_data,encoded_len,unencoded_data,+unencoded_len,1U<<page_shift);+if(ret)+gotoout;+break;+default:+error("unknown compression: %d",compression);+ret=-EOPNOTSUPP;+gotoout;+}++pos=unencoded_offset;+while(pos<unencoded_file_len){+w=pwrite(rctx->write_fd,unencoded_data+pos,+unencoded_file_len-pos,offset);+if(w<0){+ret=-errno;+error("writing unencoded data failed: %m");+gotoout;+}+pos+=w;+offset+=w;+}+out:+free(unencoded_data);+returnret;+}+staticintprocess_encoded_write(constchar*path,constvoid*data,u64offset,-u64len,u64unencoded_file_len,u64unencoded_len,-u64unencoded_offset,u32compression,u32encryption,void*user)+u64len,u64unencoded_file_len,+u64unencoded_len,u64unencoded_offset,+u32compression,u32encryption,void*user){intret;structbtrfs_receive*rctx=user;
@@ -1007,6 +1230,7 @@ static int process_encoded_write(const char *path, const void *data, u64 offset,.compression=compression,.encryption=encryption,};+boolencoded_write=!rctx->force_decompress;if(encryption){error("encoded_write: encryption not supported");
@@ -1023,13 +1247,21 @@ static int process_encoded_write(const char *path, const void *data, u64 offset,if(ret<0)returnret;-ret=ioctl(rctx->write_fd,BTRFS_IOC_ENCODED_WRITE,&encoded);-if(ret<0){-ret=-errno;-error("encoded_write: writing to %s failed: %m",path);-returnret;+if(encoded_write){+ret=ioctl(rctx->write_fd,BTRFS_IOC_ENCODED_WRITE,&encoded);+if(ret>=0)+return0;+/* Fall back for these errors, fail hard for anything else. */+if(errno!=ENOSPC&&errno!=EOPNOTSUPP&&errno!=EINVAL){
Just caught something that I missed in the conversion, this needs to be
ENOTTY instead of EOPNOTSUPP.
From: Nikolay Borisov <hidden> Date: 2021-08-20 07:59:38
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted hunk
From: Omar Sandoval <redacted>
Encoded I/O in Btrfs needs to check a write with a given logical size
without an iov_iter that matches that size (because the iov_iter we have
is for the compressed data). So, factor out the parts of
generic_write_check() that expect an iov_iter into a new
__generic_write_checks() function and export that.
Signed-off-by: Omar Sandoval <redacted>
---
fs/read_write.c | 40 ++++++++++++++++++++++------------------
include/linux/fs.h | 1 +
2 files changed, 23 insertions(+), 18 deletions(-)
@@ -1633,6 +1633,26 @@ int generic_write_check_limits(struct file *file, loff_t pos, loff_t *count)return0;}+/* Like generic_write_checks(), but takes size of write instead of iter. */+int__generic_write_checks(structkiocb*iocb,loff_t*count)+{+structfile*file=iocb->ki_filp;+structinode*inode=file->f_mapping->host;++if(IS_SWAPFILE(inode))+return-ETXTBSY;
Missing 'if(!count) return 0' from original code ?
<snip>
From: Nikolay Borisov <hidden> Date: 2021-08-20 08:08:49
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted hunk
From: Omar Sandoval <redacted>
btrfs_csum_one_bio() loops over each filesystem block in the bio while
keeping a cursor of its current logical position in the file in order to
look up the ordered extent to add the checksums to. However, this
doesn't make much sense for compressed extents, as a sector on disk does
not correspond to a sector of decompressed file data. It happens to work
because 1) the compressed bio always covers one ordered extent and 2)
the size of the bio is always less than the size of the ordered extent.
However, the second point will not always be true for encoded writes.
Let's add a boolean parameter to btrfs_csum_one_bio() to indicate that
it can assume that the bio only covers one ordered extent. Since we're
already changing the signature, let's get rid of the contig parameter
and make it implied by the offset parameter, similar to the change we
recently made to btrfs_lookup_bio_sums(). Additionally, let's rename
nr_sectors to blockcount to make it clear that it's the number of
filesystem blocks, not the number of 512-byte sectors.
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/compression.c | 5 +++--
fs/btrfs/ctree.h | 2 +-
fs/btrfs/file-item.c | 35 ++++++++++++++++-------------------
fs/btrfs/inode.c | 8 ++++----
4 files changed, 24 insertions(+), 26 deletions(-)
nit: I don't have strong preference but my gut feeling tells me
"single_ordered" might be more explicit/informative. But unless someone
else thinks the same one_ordered would also make do
nit: Again, instead of page_offsets perhaps use_page_offsets, somewhat
more explicit/informative.
quoted hunk
char *data; struct bvec_iter iter; struct bio_vec bvec; int index;- int nr_sectors;+ int blockcount; unsigned long total_bytes = 0; unsigned long this_sum_bytes = 0; int i;- u64 offset; unsigned nofs_flag; nofs_flag = memalloc_nofs_save();
@@ -649,18 +649,13 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio, sums->len = bio->bi_iter.bi_size; INIT_LIST_HEAD(&sums->list);- if (contig)- offset = file_start;- else- offset = 0; /* shut up gcc */- sums->bytenr = bio->bi_iter.bi_sector << 9; index = 0; shash->tfm = fs_info->csum_shash; bio_for_each_segment(bvec, bio, iter) {- if (!contig)+ if (page_offsets) offset = page_offset(bvec.bv_page) + bvec.bv_offset; if (!ordered) {
@@ -668,13 +663,14 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio, BUG_ON(!ordered); /* Logic error */ }- nr_sectors = BTRFS_BYTES_TO_BLKS(fs_info,+ blockcount = BTRFS_BYTES_TO_BLKS(fs_info, bvec.bv_len + fs_info->sectorsize - 1);- for (i = 0; i < nr_sectors; i++) {- if (offset >= ordered->file_offset + ordered->num_bytes ||- offset < ordered->file_offset) {+ for (i = 0; i < blockcount; i++) {+ if (!one_ordered &&+ (offset >= ordered->file_offset + ordered->num_bytes ||+ offset < ordered->file_offset)) {
Since you are changing this hunk, how about using the in_range macro:
if (!one_ordered && !in_range(offset, ordered->file_offset,
ordered->num_bytes) { foo
Though I think the "change the ordered extent now that we are working on
a different range" code should be factored out in a separate function
because currently it's somewhat breaking the flow of reading.
quoted hunk
unsigned long bytes_left;
sums->len = this_sum_bytes;
@@ -705,7 +701,8 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio, sums->sums + index); kunmap_atomic(data); index += fs_info->csum_size;- offset += fs_info->sectorsize;+ if (!one_ordered)+ offset += fs_info->sectorsize;
Instead of adding one additional conditional op can't offset always be
incremented but in the case of one_ordered then the !in_range check
should always be false i.e we won't be using the offset to lookup a new OE?
From: Nikolay Borisov <hidden> Date: 2021-08-20 08:34:21
On 18.08.21 г. 0:06, Omar Sandoval wrote:
From: Omar Sandoval <redacted>
Currently, we only create ordered extents when ram_bytes == num_bytes
and offset == 0. However, RWF_ENCODED writes may create extents which
only refer to a subset of the full unencoded extent, so we need to plumb
From: Nikolay Borisov <hidden> Date: 2021-08-20 08:51:29
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted hunk
From: Omar Sandoval <redacted>
Currently, an inline extent is always created after i_size is extended
from btrfs_dirty_pages(). However, for encoded writes, we only want to
update i_size after we successfully created the inline extent. Add an
update_i_size parameter to cow_file_range_inline() and
insert_inline_extent() and pass in the size of the extent rather than
determining it from i_size. Since the start parameter is always passed
as 0, get rid of it and simplify the logic in these two functions. While
we're here, let's document the requirements for creating an inline
extent.
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/inode.c | 100 +++++++++++++++++++++++------------------------
1 file changed, 48 insertions(+), 52 deletions(-)
@@ -236,9 +236,10 @@ static int btrfs_init_inode_security(struct btrfs_trans_handle *trans,staticintinsert_inline_extent(structbtrfs_trans_handle*trans,structbtrfs_path*path,boolextent_inserted,structbtrfs_root*root,structinode*inode,-u64start,size_tsize,size_tcompressed_size,+size_tsize,size_tcompressed_size,intcompress_type,-structpage**compressed_pages)+structpage**compressed_pages,+boolupdate_i_size){structextent_buffer*leaf;structpage*page=NULL;
@@ -247,7 +248,7 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,structbtrfs_file_extent_item*ei;intret;size_tcur_size=size;-unsignedlongoffset;+u64i_size;ASSERT((compressed_size>0&&compressed_pages)||(compressed_size==0&&!compressed_pages));
@@ -260,7 +261,7 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,size_tdatasize;key.objectid=btrfs_ino(BTRFS_I(inode));-key.offset=start;+key.offset=0;key.type=BTRFS_EXTENT_DATA_KEY;datasize=btrfs_file_extent_calc_inline_size(cur_size);
@@ -297,12 +298,10 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,btrfs_set_file_extent_compression(leaf,ei,compress_type);}else{-page=find_get_page(inode->i_mapping,-start>>PAGE_SHIFT);+page=find_get_page(inode->i_mapping,0);btrfs_set_file_extent_compression(leaf,ei,0);kaddr=kmap_atomic(page);-offset=offset_in_page(start);-write_extent_buffer(leaf,kaddr+offset,ptr,size);+write_extent_buffer(leaf,kaddr,ptr,size);kunmap_atomic(kaddr);put_page(page);}
@@ -313,8 +312,8 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,*Wealignsizetosectorsizeforinlineextentsjustforsimplicity*sake.*/-size=ALIGN(size,root->fs_info->sectorsize);-ret=btrfs_inode_set_file_extent_range(BTRFS_I(inode),start,size);+ret=btrfs_inode_set_file_extent_range(BTRFS_I(inode),0,+ALIGN(size,root->fs_info->sectorsize));if(ret)gotofail;
@@ -327,7 +326,13 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,*beforeweunlockthepages.Otherwisewe*couldendupracingwithunlink.*/-BTRFS_I(inode)->disk_i_size=inode->i_size;+i_size=i_size_read(inode);+if(update_i_size&&size>i_size){+i_size_write(inode,size);+i_size=size;+}+BTRFS_I(inode)->disk_i_size=i_size;+fail:returnret;}
@@ -338,35 +343,31 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,*doesthechecksrequiredtomakesurethedataissmallenough*tofitasaninlineextent.*/-staticnoinlineintcow_file_range_inline(structbtrfs_inode*inode,u64start,-u64end,size_tcompressed_size,+staticnoinlineintcow_file_range_inline(structbtrfs_inode*inode,u64size,+size_tcompressed_size,intcompress_type,-structpage**compressed_pages)+structpage**compressed_pages,+boolupdate_i_size){structbtrfs_drop_extents_argsdrop_args={0};structbtrfs_root*root=inode->root;structbtrfs_fs_info*fs_info=root->fs_info;structbtrfs_trans_handle*trans;-u64isize=i_size_read(&inode->vfs_inode);-u64actual_end=min(end+1,isize);-u64inline_len=actual_end-start;-u64aligned_end=ALIGN(end,fs_info->sectorsize);-u64data_len=inline_len;+u64data_len=compressed_size?compressed_size:size;intret;structbtrfs_path*path;-if(compressed_size)-data_len=compressed_size;--if(start>0||-actual_end>fs_info->sectorsize||+/*+*Wecancreateaninlineextentifitendsatorbeyondthecurrent+*i_size,isnolargerthanasector(decompressed),andthe(possibly+*compressed)datafitsinaleafandtheconfiguredmaximuminline+*size.+*/
Urgh, just some days ago Qu was talking about how awkward it is to have
mixed extents in a file. And now, AFAIU, you are making them more likely
since now they can be created not just at the beginning of the file but
also after i_size write. While this won't be a problem in and of itself
it goes just the opposite way of us trying to shrink the possible cases
when we can have mixed extents. Qu what is your take on that?
<snip>
From: Nikolay Borisov <hidden> Date: 2021-08-20 08:56:41
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted hunk
From: Omar Sandoval <redacted>
In order to allow sending and receiving compressed data without
decompressing it, we need an interface to write pre-compressed data
directly to the filesystem and the matching interface to read compressed
data without decompressing it. This adds the definitions for ioctls to
do that and detailed explanations of how to use them.
Signed-off-by: Omar Sandoval <redacted>
---
include/uapi/linux/btrfs.h | 132 +++++++++++++++++++++++++++++++++++++
1 file changed, 132 insertions(+)
@@ -861,6 +861,134 @@ struct btrfs_ioctl_get_subvol_rootref_args {__u8align[7];};+/*+*Dataandmetadataforanencodedreadorwrite.+*+*EncodedI/Obypassesanyencodingautomaticallydonebythefilesystem(e.g.,+*compression).Thiscanbeusedtoreadthecompressedcontentsofafileor+*writepre-compresseddatadirectlytoafile.+*+*BTRFS_IOC_ENCODED_READandBTRFS_IOC_ENCODED_WRITEareessentially+*preadv/pwritevwithadditionalmetadataabouthowthedataisencodedandthe+*sizeoftheunencodeddata.+*+*BTRFS_IOC_ENCODED_READfillsthegiveniovecswiththeencodeddata,fills+*themetadatafields,andreturnsthesizeoftheencodeddata.Itreadsone+*extentpercall.Itcanalsoreaddatawhichisnotencoded.+*+*BTRFS_IOC_ENCODED_WRITEusesthemetadatafields,writestheencodeddata+*fromtheiovecs,andreturnsthesizeoftheencodeddata.Notethatthe+*encodeddataisnotvalidatedwhenitiswritten;ifitisnotvalid(e.g.,+*itcannotbedecompressed),thenasubsequentreadmayreturnanerror.+*+*Sincethefilesystempagecachecontainsdecodeddata,encodedI/Obypasses+*thepagecache.EncodedI/OrequiresCAP_SYS_ADMIN.+*/+structbtrfs_ioctl_encoded_io_args{+/* Input parameters for both reads and writes. */++/*+*iovecscontainingencodeddata.+*+*Forreads,ifthesizeoftheencodeddataislargerthanthesumof+*iov[n].iov_lenfor0<=n<iovcnt,thentheioctlfailswith+*ENOBUFS.+*+*Forwrites,thesizeoftheencodeddataisthesumofiov[n].iov_len+*for0<=n<iovcnt.Thismustbelessthan128KiB(thislimitmay+*increaseinthefuture).Thismustalsobelessthanorequalto+*unencoded_len.+*/+conststructiovec__user*iov;+/* Number of iovecs. */+unsignedlongiovcnt;+/*+*Offsetinfile.+*+*Forwrites,mustbealignedtothesectorsizeofthefilesystem.+*/+__s64offset;+/* Currently must be zero. */+__u64flags;++/*+*Forreads,thefollowingmembersarefilledinwiththemetadatafor+*theencodeddata.+*Forwrites,thefollowingmembersmustbesettothemetadataforthe+*encodeddata.+*/++/*+*Lengthofthedatainthefile.+*+*Mustbelessthanorequaltounencoded_len-unencoded_offset.For+*writes,mustbealignedtothesectorsizeofthefilesystemunless+*thedataendsatorbeyondthecurrentendofthefile.+*/+__u64len;+/*+*Lengthoftheunencoded(i.e.,decryptedanddecompressed)data.+*+*Forwrites,mustbenomorethan128KiB(thislimitmayincreasein+*thefuture).Iftheunencodeddataisactuallylongerthan+*unencoded_len,thenitistruncated;ifitisshorter,thenitis+*extendedwithzeroes.+*/+__u64unencoded_len;+/*+*Offsetfromthefirstbyteoftheunencodeddatatothefirstbyteof+*logicaldatainthefile.+*+*Mustbelessthanunencoded_len.+*/+__u64unencoded_offset;+/*+*BTRFS_ENCODED_IO_COMPRESSION_*type.+*+*Forwrites,mustnotbeBTRFS_ENCODED_IO_COMPRESSION_NONE.+*/+__u32compression;+/* Currently always BTRFS_ENCODED_IO_ENCRYPTION_NONE. */+__u32encryption;+/*+*Reservedforfutureexpansion.+*+*Forreads,alwaysreturnedaszero.Usersshouldcheckfornon-zero+*bytes.Ifthereareany,thenthekernelhasanewerversionofthis+*structurewithadditionalinformationthattheuserdefinitionis+*missing.+*+*Forwrites,mustbezeroed.+*/+__u8reserved[32];+};++/* Data is not compressed. */+#define BTRFS_ENCODED_IO_COMPRESSION_NONE 0+/* Data is compressed as a single zlib stream. */+#define BTRFS_ENCODED_IO_COMPRESSION_ZLIB 1+/*+*DataiscompressedasasinglezstdframewiththewindowLogcompression+*parametersettonomorethan17.+*/+#define BTRFS_ENCODED_IO_COMPRESSION_ZSTD 2+/*+*Dataiscompressedpagebypage(usingthepagesizeindicatedbythenameof+*theconstant)withLZO1Xandwrappedintheformatdocumentedin+*fs/btrfs/lzo.c.Forwrites,thecompressionpagesizemustmatchthe+*filesystempagesize.+*/+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_4K 3+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_8K 4+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_16K 5+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_32K 6+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_64K 7+#define BTRFS_ENCODED_IO_COMPRESSION_TYPES 8++/* Data is not encrypted. */+#define BTRFS_ENCODED_IO_ENCRYPTION_NONE 0+#define BTRFS_ENCODED_IO_ENCRYPTION_TYPES 1
How about an enums for encryption/compression.
quoted hunk
+ /* Error codes as returned by the kernel */ enum btrfs_err_code { BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET = 1,
From: Omar Sandoval <redacted>
Currently, an inline extent is always created after i_size is extended
from btrfs_dirty_pages(). However, for encoded writes, we only want to
update i_size after we successfully created the inline extent.
To me, the idea of write first then update isize is just going to cause
tons of inline extent related prblems.
The current example is falloc, which only update the isize after the
falloc finishes.
This behavior has already bothered me quite a lot, as it can easily
create mixed inline and regular extents.
Can't we remember the old isize (with proper locking), enlarge isize
(with holes filled), do the write.
If something wrong happened, we truncate the isize back to its old isize.
quoted
Add an
update_i_size parameter to cow_file_range_inline() and
insert_inline_extent() and pass in the size of the extent rather than
determining it from i_size. Since the start parameter is always passed
as 0, get rid of it and simplify the logic in these two functions. While
we're here, let's document the requirements for creating an inline
extent.
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/inode.c | 100 +++++++++++++++++++++++------------------------
1 file changed, 48 insertions(+), 52 deletions(-)
@@ -236,9 +236,10 @@ static int btrfs_init_inode_security(struct btrfs_trans_handle *trans,staticintinsert_inline_extent(structbtrfs_trans_handle*trans,structbtrfs_path*path,boolextent_inserted,structbtrfs_root*root,structinode*inode,-u64start,size_tsize,size_tcompressed_size,+size_tsize,size_tcompressed_size,intcompress_type,-structpage**compressed_pages)+structpage**compressed_pages,+boolupdate_i_size){structextent_buffer*leaf;structpage*page=NULL;
@@ -247,7 +248,7 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,structbtrfs_file_extent_item*ei;intret;size_tcur_size=size;-unsignedlongoffset;+u64i_size;ASSERT((compressed_size>0&&compressed_pages)||(compressed_size==0&&!compressed_pages));
@@ -260,7 +261,7 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,size_tdatasize;key.objectid=btrfs_ino(BTRFS_I(inode));-key.offset=start;+key.offset=0;key.type=BTRFS_EXTENT_DATA_KEY;datasize=btrfs_file_extent_calc_inline_size(cur_size);
@@ -297,12 +298,10 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,btrfs_set_file_extent_compression(leaf,ei,compress_type);}else{-page=find_get_page(inode->i_mapping,-start>>PAGE_SHIFT);+page=find_get_page(inode->i_mapping,0);btrfs_set_file_extent_compression(leaf,ei,0);kaddr=kmap_atomic(page);-offset=offset_in_page(start);-write_extent_buffer(leaf,kaddr+offset,ptr,size);+write_extent_buffer(leaf,kaddr,ptr,size);kunmap_atomic(kaddr);put_page(page);}
@@ -313,8 +312,8 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,*Wealignsizetosectorsizeforinlineextentsjustforsimplicity*sake.*/-size=ALIGN(size,root->fs_info->sectorsize);-ret=btrfs_inode_set_file_extent_range(BTRFS_I(inode),start,size);+ret=btrfs_inode_set_file_extent_range(BTRFS_I(inode),0,+ALIGN(size,root->fs_info->sectorsize));if(ret)gotofail;
@@ -327,7 +326,13 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,*beforeweunlockthepages.Otherwisewe*couldendupracingwithunlink.*/-BTRFS_I(inode)->disk_i_size=inode->i_size;+i_size=i_size_read(inode);+if(update_i_size&&size>i_size){+i_size_write(inode,size);+i_size=size;+}+BTRFS_I(inode)->disk_i_size=i_size;+fail:returnret;}
@@ -338,35 +343,31 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,*doesthechecksrequiredtomakesurethedataissmallenough*tofitasaninlineextent.*/-staticnoinlineintcow_file_range_inline(structbtrfs_inode*inode,u64start,-u64end,size_tcompressed_size,+staticnoinlineintcow_file_range_inline(structbtrfs_inode*inode,u64size,+size_tcompressed_size,intcompress_type,-structpage**compressed_pages)+structpage**compressed_pages,+boolupdate_i_size){structbtrfs_drop_extents_argsdrop_args={0};structbtrfs_root*root=inode->root;structbtrfs_fs_info*fs_info=root->fs_info;structbtrfs_trans_handle*trans;-u64isize=i_size_read(&inode->vfs_inode);-u64actual_end=min(end+1,isize);-u64inline_len=actual_end-start;-u64aligned_end=ALIGN(end,fs_info->sectorsize);-u64data_len=inline_len;+u64data_len=compressed_size?compressed_size:size;intret;structbtrfs_path*path;-if(compressed_size)-data_len=compressed_size;--if(start>0||-actual_end>fs_info->sectorsize||+/*+*Wecancreateaninlineextentifitendsatorbeyondthecurrent+*i_size,isnolargerthanasector(decompressed),andthe(possibly+*compressed)datafitsinaleafandtheconfiguredmaximuminline+*size.+*/
Urgh, just some days ago Qu was talking about how awkward it is to have
mixed extents in a file. And now, AFAIU, you are making them more likely
since now they can be created not just at the beginning of the file but
also after i_size write. While this won't be a problem in and of itself
it goes just the opposite way of us trying to shrink the possible cases
when we can have mixed extents.
Tree-checker should reject such inline extent at non-zero offset.
Qu what is your take on that?
My question is, why encoded write needs to bother the inline extents at all?
My intuition of such encoded write is, it should not create inline
extents at all.
Or is there any special use-case involved for encoded write?
Thanks,
Qu
From: Nikolay Borisov <hidden> Date: 2021-08-20 12:30:08
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted hunk
From: Omar Sandoval <redacted>
There are 4 main cases:
1. Inline extents: we copy the data straight out of the extent buffer.
2. Hole/preallocated extents: we fill in zeroes.
3. Regular, uncompressed extents: we read the sectors we need directly
from disk.
4. Regular, compressed extents: we read the entire compressed extent
from disk and indicate what subset of the decompressed extent is in
the file.
This initial implementation simplifies a few things that can be improved
in the future:
- We hold the inode lock during the operation.
- Cases 1, 3, and 4 allocate temporary memory to read into before
copying out to userspace.
- We don't do read repair, because it turns out that read repair is
currently broken for compressed data.
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/ctree.h | 4 +
fs/btrfs/inode.c | 489 +++++++++++++++++++++++++++++++++++++++++++++++
fs/btrfs/ioctl.c | 111 +++++++++++
3 files changed, 604 insertions(+)
@@ -10496,6 +10496,495 @@ void btrfs_set_range_writeback(struct btrfs_inode *inode, u64 start, u64 end)}}+staticintbtrfs_encoded_io_compression_from_extent(intcompress_type)+{+switch(compress_type){+caseBTRFS_COMPRESS_NONE:+returnBTRFS_ENCODED_IO_COMPRESSION_NONE;+caseBTRFS_COMPRESS_ZLIB:+returnBTRFS_ENCODED_IO_COMPRESSION_ZLIB;+caseBTRFS_COMPRESS_LZO:+/*+*TheLZOformatdependsonthepagesize.64kisthemaximum+*sectorsize(andthuspagesize)thatwesupport.+*/+if(PAGE_SIZE<SZ_4K||PAGE_SIZE>SZ_64K)+return-EINVAL;+returnBTRFS_ENCODED_IO_COMPRESSION_LZO_4K+(PAGE_SHIFT-12);+caseBTRFS_COMPRESS_ZSTD:+returnBTRFS_ENCODED_IO_COMPRESSION_ZSTD;+default:+return-EUCLEAN;+}+}++staticssize_tbtrfs_encoded_read_inline(+structkiocb*iocb,+structiov_iter*iter,u64start,+u64lockend,+structextent_state**cached_state,+u64extent_start,size_tcount,+structbtrfs_ioctl_encoded_io_args*encoded,+bool*unlocked)+{+structinode*inode=file_inode(iocb->ki_filp);+structextent_io_tree*io_tree=&BTRFS_I(inode)->io_tree;+structbtrfs_path*path;+structextent_buffer*leaf;+structbtrfs_file_extent_item*item;+u64ram_bytes;+unsignedlongptr;+void*tmp;+ssize_tret;++path=btrfs_alloc_path();+if(!path){+ret=-ENOMEM;+gotoout;+}+ret=btrfs_lookup_file_extent(NULL,BTRFS_I(inode)->root,path,+btrfs_ino(BTRFS_I(inode)),extent_start,+0);+if(ret){+if(ret>0){+/* The extent item disappeared? */+ret=-EIO;+}+gotoout;+}+leaf=path->nodes[0];+item=btrfs_item_ptr(leaf,path->slots[0],+structbtrfs_file_extent_item);++ram_bytes=btrfs_file_extent_ram_bytes(leaf,item);+ptr=btrfs_file_extent_inline_start(item);++encoded->len=(min_t(u64,extent_start+ram_bytes,inode->i_size)-+iocb->ki_pos);+ret=btrfs_encoded_io_compression_from_extent(+btrfs_file_extent_compression(leaf,item));+if(ret<0)+gotoout;+encoded->compression=ret;+if(encoded->compression){+size_tinline_size;++inline_size=btrfs_file_extent_inline_item_len(leaf,+btrfs_item_nr(path->slots[0]));+if(inline_size>count){+ret=-ENOBUFS;+gotoout;+}+count=inline_size;+encoded->unencoded_len=ram_bytes;+encoded->unencoded_offset=iocb->ki_pos-extent_start;+}else{+encoded->len=encoded->unencoded_len=count=+min_t(u64,count,encoded->len);+ptr+=iocb->ki_pos-extent_start;+}++tmp=kmalloc(count,GFP_NOFS);+if(!tmp){+ret=-ENOMEM;+gotoout;+}+read_extent_buffer(leaf,tmp,ptr,count);+btrfs_release_path(path);+unlock_extent_cached(io_tree,start,lockend,cached_state);+inode_unlock_shared(inode);+*unlocked=true;++ret=copy_to_iter(tmp,count,iter);+if(ret!=count)+ret=-EFAULT;+kfree(tmp);+out:+btrfs_free_path(path);+returnret;+}++structbtrfs_encoded_read_private{+structinode*inode;+wait_queue_head_twait;+atomic_tpending;+blk_status_tstatus;+boolskip_csum;+};++staticblk_status_tsubmit_encoded_read_bio(structinode*inode,+structbio*bio,intmirror_num,+unsignedlongbio_flags)+{+structbtrfs_encoded_read_private*priv=bio->bi_private;+structbtrfs_io_bio*io_bio=btrfs_io_bio(bio);+structbtrfs_fs_info*fs_info=btrfs_sb(inode->i_sb);+blk_status_tret;++if(!priv->skip_csum){+ret=btrfs_lookup_bio_sums(inode,bio,NULL);+if(ret)+returnret;+}++ret=btrfs_bio_wq_end_io(fs_info,bio,BTRFS_WQ_ENDIO_DATA);+if(ret){+btrfs_io_bio_free_csum(io_bio);+returnret;+}++atomic_inc(&priv->pending);+ret=btrfs_map_bio(fs_info,bio,mirror_num);+if(ret){+atomic_dec(&priv->pending);+btrfs_io_bio_free_csum(io_bio);+}+returnret;+}++staticblk_status_tbtrfs_encoded_read_check_bio(structbtrfs_io_bio*io_bio)+{+constbooluptodate=io_bio->bio.bi_status==BLK_STS_OK;+structbtrfs_encoded_read_private*priv=io_bio->bio.bi_private;+structinode*inode=priv->inode;+structbtrfs_fs_info*fs_info=btrfs_sb(inode->i_sb);+u32sectorsize=fs_info->sectorsize;+structbio_vec*bvec;+structbvec_iter_alliter_all;+u64start=io_bio->logical;+u32bio_offset=0;++if(priv->skip_csum||!uptodate)+returnio_bio->bio.bi_status;++bio_for_each_segment_all(bvec,&io_bio->bio,iter_all){+unsignedinti,nr_sectors,pgoff;++nr_sectors=BTRFS_BYTES_TO_BLKS(fs_info,bvec->bv_len);+pgoff=bvec->bv_offset;+for(i=0;i<nr_sectors;i++){+ASSERT(pgoff<PAGE_SIZE);+if(check_data_csum(inode,io_bio,bio_offset,+bvec->bv_page,pgoff,start))+returnBLK_STS_IOERR;+start+=sectorsize;+bio_offset+=sectorsize;+pgoff+=sectorsize;+}+}+returnBLK_STS_OK;+}++staticvoidbtrfs_encoded_read_endio(structbio*bio)+{+structbtrfs_encoded_read_private*priv=bio->bi_private;+structbtrfs_io_bio*io_bio=btrfs_io_bio(bio);+blk_status_tstatus;++status=btrfs_encoded_read_check_bio(io_bio);+if(status){+/*+*Thememorybarrierimpliedbytheatomic_dec_return()here+*pairswiththememorybarrierimpliedbythe+*atomic_dec_return()orio_wait_event()in+*btrfs_encoded_read_regular_fill_pages()toensurethatthis+*writeisobservedbeforetheloadofstatusin+*btrfs_encoded_read_regular_fill_pages().+*/+WRITE_ONCE(priv->status,status);+}+if(!atomic_dec_return(&priv->pending))+wake_up(&priv->wait);+btrfs_io_bio_free_csum(io_bio);+bio_put(bio);+}++staticintbtrfs_encoded_read_regular_fill_pages(structinode*inode,u64offset,+u64disk_io_size,structpage**pages)+{+structbtrfs_fs_info*fs_info=btrfs_sb(inode->i_sb);+structbtrfs_encoded_read_privatepriv={+.inode=inode,+.pending=ATOMIC_INIT(1),+.skip_csum=BTRFS_I(inode)->flags&BTRFS_INODE_NODATASUM,+};+unsignedlongi=0;+u64cur=0;+intret;++init_waitqueue_head(&priv.wait);+/*+*Submitbiosfortheextent,splittingduetobioorstripelimitsas+*necessary.+*/+while(cur<disk_io_size){+structextent_map*em;+structbtrfs_io_geometrygeom;+structbio*bio=NULL;+u64remaining;++em=btrfs_get_chunk_map(fs_info,offset+cur,+disk_io_size-cur);+if(IS_ERR(em)){+ret=PTR_ERR(em);+}else{+ret=btrfs_get_io_geometry(fs_info,em,BTRFS_MAP_READ,+offset+cur,&geom);+free_extent_map(em);+}+if(ret){+WRITE_ONCE(priv.status,errno_to_blk_status(ret));+break;+}+remaining=min(geom.len,disk_io_size-cur);+while(bio||remaining){+size_tbytes=min_t(u64,remaining,PAGE_SIZE);++if(!bio){+bio=btrfs_bio_alloc(offset+cur);+bio->bi_end_io=btrfs_encoded_read_endio;+bio->bi_private=&priv;+bio->bi_opf=REQ_OP_READ;+}++if(!bytes||+bio_add_page(bio,pages[i],bytes,0)<bytes){+blk_status_tstatus;++status=submit_encoded_read_bio(inode,bio,0,+0);+if(status){+WRITE_ONCE(priv.status,status);+bio_put(bio);+gotoout;+}+bio=NULL;+continue;+}++i++;+cur+=bytes;+remaining-=bytes;+}+}++out:+if(atomic_dec_return(&priv.pending))+io_wait_event(priv.wait,!atomic_read(&priv.pending));+/* See btrfs_encoded_read_endio() for ordering. */+returnblk_status_to_errno(READ_ONCE(priv.status));+}++staticssize_tbtrfs_encoded_read_regular(structkiocb*iocb,+structiov_iter*iter,+u64start,u64lockend,+structextent_state**cached_state,+u64offset,u64disk_io_size,+size_tcount,boolcompressed,+bool*unlocked)+{+structinode*inode=file_inode(iocb->ki_filp);+structextent_io_tree*io_tree=&BTRFS_I(inode)->io_tree;+structpage**pages;+unsignedlongnr_pages,i;+u64cur;+size_tpage_offset;+ssize_tret;++nr_pages=DIV_ROUND_UP(disk_io_size,PAGE_SIZE);+pages=kcalloc(nr_pages,sizeof(structpage*),GFP_NOFS);+if(!pages)+return-ENOMEM;+for(i=0;i<nr_pages;i++){+pages[i]=alloc_page(GFP_NOFS|__GFP_HIGHMEM);+if(!pages[i]){+ret=-ENOMEM;+gotoout;+}+}++ret=btrfs_encoded_read_regular_fill_pages(inode,offset,disk_io_size,+pages);+if(ret)+gotoout;++unlock_extent_cached(io_tree,start,lockend,cached_state);+inode_unlock_shared(inode);+*unlocked=true;++if(compressed){+i=0;+page_offset=0;+}else{+i=(iocb->ki_pos-start)>>PAGE_SHIFT;+page_offset=(iocb->ki_pos-start)&(PAGE_SIZE-1);+}+cur=0;+while(cur<count){+size_tbytes=min_t(size_t,count-cur,+PAGE_SIZE-page_offset);++if(copy_page_to_iter(pages[i],page_offset,bytes,+iter)!=bytes){+ret=-EFAULT;+gotoout;+}+i++;+cur+=bytes;+page_offset=0;+}+ret=count;+out:+for(i=0;i<nr_pages;i++){+if(pages[i])+__free_page(pages[i]);+}+kfree(pages);+returnret;+}++ssize_tbtrfs_encoded_read(structkiocb*iocb,structiov_iter*iter,+structbtrfs_ioctl_encoded_io_args*encoded)+{+structinode*inode=file_inode(iocb->ki_filp);+structbtrfs_fs_info*fs_info=btrfs_sb(inode->i_sb);+structextent_io_tree*io_tree=&BTRFS_I(inode)->io_tree;+ssize_tret;+size_tcount=iov_iter_count(iter);+u64start,lockend,offset,disk_io_size;+structextent_state*cached_state=NULL;+structextent_map*em;+boolunlocked=false;++file_accessed(iocb->ki_filp);++inode_lock_shared(inode);++if(iocb->ki_pos>=inode->i_size){+inode_unlock_shared(inode);+return0;
Don't we need to signal beyond EOF reads somehow. As it stands returning
0 means returning zeroed portion of btrfs_ioctl_encoded_io_args to user
space?
quoted hunk
+ }+ start = ALIGN_DOWN(iocb->ki_pos, fs_info->sectorsize);+ /*+ * We don't know how long the extent containing iocb->ki_pos is, but if+ * it's compressed we know that it won't be longer than this.+ */+ lockend = start + BTRFS_MAX_UNCOMPRESSED - 1;++ for (;;) {+ struct btrfs_ordered_extent *ordered;++ ret = btrfs_wait_ordered_range(inode, start,+ lockend - start + 1);+ if (ret)+ goto out_unlock_inode;+ lock_extent_bits(io_tree, start, lockend, &cached_state);+ ordered = btrfs_lookup_ordered_range(BTRFS_I(inode), start,+ lockend - start + 1);+ if (!ordered)+ break;+ btrfs_put_ordered_extent(ordered);+ unlock_extent_cached(io_tree, start, lockend, &cached_state);+ cond_resched();+ }
Can't you simply use btrfs_lock_and_flush_ordered_range, the major
difference is btrfs_wait_ordered_range basically instantiates any
pending delalloc, whilst btrfs_lock_and_flush_ordered_range returns with
any, already-instantiated OE run to completion and the range locked ?
quoted hunk
++ em = btrfs_get_extent(BTRFS_I(inode), NULL, 0, start,+ lockend - start + 1);+ if (IS_ERR(em)) {+ ret = PTR_ERR(em);+ goto out_unlock_extent;+ }++ if (em->block_start == EXTENT_MAP_INLINE) {+ u64 extent_start = em->start;++ /*+ * For inline extents we get everything we need out of the+ * extent item.+ */+ free_extent_map(em);+ em = NULL;+ ret = btrfs_encoded_read_inline(iocb, iter, start, lockend,+ &cached_state, extent_start,+ count, encoded, &unlocked);+ goto out;+ }++ /*+ * We only want to return up to EOF even if the extent extends beyond+ * that.+ */+ encoded->len = (min_t(u64, extent_map_end(em), inode->i_size) -+ iocb->ki_pos);+ if (em->block_start == EXTENT_MAP_HOLE ||+ test_bit(EXTENT_FLAG_PREALLOC, &em->flags)) {+ offset = EXTENT_MAP_HOLE;+ encoded->len = encoded->unencoded_len = count =+ min_t(u64, count, encoded->len);+ } else if (test_bit(EXTENT_FLAG_COMPRESSED, &em->flags)) {+ offset = em->block_start;+ /*+ * Bail if the buffer isn't large enough to return the whole+ * compressed extent.+ */+ if (em->block_len > count) {+ ret = -ENOBUFS;+ goto out_em;+ }+ disk_io_size = count = em->block_len;+ encoded->unencoded_len = em->ram_bytes;+ encoded->unencoded_offset = iocb->ki_pos - em->orig_start;+ ret = btrfs_encoded_io_compression_from_extent(+ em->compress_type);+ if (ret < 0)+ goto out_em;+ encoded->compression = ret;+ } else {+ offset = em->block_start + (start - em->start);+ if (encoded->len > count)+ encoded->len = count;+ /*+ * Don't read beyond what we locked. This also limits the page+ * allocations that we'll do.+ */+ disk_io_size = min(lockend + 1,+ iocb->ki_pos + encoded->len) - start;+ encoded->len = encoded->unencoded_len = count =+ start + disk_io_size - iocb->ki_pos;+ disk_io_size = ALIGN(disk_io_size, fs_info->sectorsize);+ }+ free_extent_map(em);+ em = NULL;++ if (offset == EXTENT_MAP_HOLE) {+ unlock_extent_cached(io_tree, start, lockend, &cached_state);+ inode_unlock_shared(inode);+ unlocked = true;+ ret = iov_iter_zero(count, iter);+ if (ret != count)+ ret = -EFAULT;+ } else {+ ret = btrfs_encoded_read_regular(iocb, iter, start, lockend,+ &cached_state, offset,+ disk_io_size, count,+ encoded->compression,+ &unlocked);+ }++out:+ if (ret >= 0)+ iocb->ki_pos += encoded->len;+out_em:+ free_extent_map(em);+out_unlock_extent:+ if (!unlocked)+ unlock_extent_cached(io_tree, start, lockend, &cached_state);+out_unlock_inode:+ if (!unlocked)+ inode_unlock_shared(inode);+ return ret;+}+ #ifdef CONFIG_SWAP /* * Add an entry indicating a block group or device which is pinned by a
<snip>
quoted hunk
/* Mask out flags that are inappropriate for the given type of inode. */
From: Nikolay Borisov <hidden> Date: 2021-08-20 13:44:30
On 18.08.21 г. 0:06, Omar Sandoval wrote:
From: Omar Sandoval <redacted>
The implementation resembles direct I/O: we have to flush any ordered
extents, invalidate the page cache, and do the io tree/delalloc/extent
map/ordered extent dance. From there, we can reuse the compression code
with a minor modification to distinguish the write from writeback. This
also creates inline extents when possible.
Signed-off-by: Omar Sandoval <redacted>
<snip>
quoted hunk
* Add an entry indicating a block group or device which is pinned by a
@@ -103,6 +103,8 @@ struct btrfs_ioctl_encoded_io_args_32 {#define BTRFS_IOC_ENCODED_READ_32 _IOR(BTRFS_IOCTL_MAGIC, 64, \structbtrfs_ioctl_encoded_io_args_32)+#define BTRFS_IOC_ENCODED_WRITE_32 _IOW(BTRFS_IOCTL_MAGIC, 64, \+structbtrfs_ioctl_encoded_io_args_32)#endif/* Mask out flags that are inappropriate for the given type of inode. */
Do you intend on supporting encrypted data writeout in the future, given
that in btrfs_do_encoded_write EINVAL is returned if the data to be
written is encrypted? If not then this check could be moved earlier to
fail fast.
<snip>
quoted hunk
@@ -5138,9 +5236,13 @@ long btrfs_ioctl(struct file *file, unsigned int return fsverity_ioctl_measure(file, argp); case BTRFS_IOC_ENCODED_READ: return btrfs_ioctl_encoded_read(file, argp, false);+ case BTRFS_IOC_ENCODED_WRITE:+ return btrfs_ioctl_encoded_write(file, argp, false); #if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT) case BTRFS_IOC_ENCODED_READ_32: return btrfs_ioctl_encoded_read(file, argp, true);+ case BTRFS_IOC_ENCODED_WRITE_32:+ return btrfs_ioctl_encoded_write(file, argp, true); #endif }
@@ -74,6 +74,8 @@ enum {BTRFS_ORDERED_LOGGED_CSUM,/* We wait for this extent to complete in the current transaction */BTRFS_ORDERED_PENDING,+/* RWF_ENCODED I/O */
nit: RWF_ENCODED is no longer, we simply have ioctl-based encoded io. So
this needs to be renamed to avoid confusion for people not necessarily
faimilar with the development history of the feature.
quoted hunk
+ BTRFS_ORDERED_ENCODED, }; /* BTRFS_ORDERED_* flags that specify the type of the extent. */
On Fri, Aug 20, 2021 at 10:59:28AM +0300, Nikolay Borisov wrote:
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted
From: Omar Sandoval <redacted>
Encoded I/O in Btrfs needs to check a write with a given logical size
without an iov_iter that matches that size (because the iov_iter we have
is for the compressed data). So, factor out the parts of
generic_write_check() that expect an iov_iter into a new
__generic_write_checks() function and export that.
Signed-off-by: Omar Sandoval <redacted>
---
fs/read_write.c | 40 ++++++++++++++++++++++------------------
include/linux/fs.h | 1 +
2 files changed, 23 insertions(+), 18 deletions(-)
@@ -1633,6 +1633,26 @@ int generic_write_check_limits(struct file *file, loff_t pos, loff_t *count)return0;}+/* Like generic_write_checks(), but takes size of write instead of iter. */+int__generic_write_checks(structkiocb*iocb,loff_t*count)+{+structfile*file=iocb->ki_filp;+structinode*inode=file->f_mapping->host;++if(IS_SWAPFILE(inode))+return-ETXTBSY;
Missing 'if(!count) return 0' from original code ?
On Fri, Aug 20, 2021 at 11:08:41AM +0300, Nikolay Borisov wrote:
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted
From: Omar Sandoval <redacted>
btrfs_csum_one_bio() loops over each filesystem block in the bio while
keeping a cursor of its current logical position in the file in order to
look up the ordered extent to add the checksums to. However, this
doesn't make much sense for compressed extents, as a sector on disk does
not correspond to a sector of decompressed file data. It happens to work
because 1) the compressed bio always covers one ordered extent and 2)
the size of the bio is always less than the size of the ordered extent.
However, the second point will not always be true for encoded writes.
Let's add a boolean parameter to btrfs_csum_one_bio() to indicate that
it can assume that the bio only covers one ordered extent. Since we're
already changing the signature, let's get rid of the contig parameter
and make it implied by the offset parameter, similar to the change we
recently made to btrfs_lookup_bio_sums(). Additionally, let's rename
nr_sectors to blockcount to make it clear that it's the number of
filesystem blocks, not the number of 512-byte sectors.
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/compression.c | 5 +++--
fs/btrfs/ctree.h | 2 +-
fs/btrfs/file-item.c | 35 ++++++++++++++++-------------------
fs/btrfs/inode.c | 8 ++++----
4 files changed, 24 insertions(+), 26 deletions(-)
nit: I don't have strong preference but my gut feeling tells me
"single_ordered" might be more explicit/informative. But unless someone
else thinks the same one_ordered would also make do
nit: Again, instead of page_offsets perhaps use_page_offsets, somewhat
more explicit/informative.
Sure, that sounds better.
quoted
char *data; struct bvec_iter iter; struct bio_vec bvec; int index;- int nr_sectors;+ int blockcount; unsigned long total_bytes = 0; unsigned long this_sum_bytes = 0; int i;- u64 offset; unsigned nofs_flag; nofs_flag = memalloc_nofs_save();
@@ -649,18 +649,13 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio, sums->len = bio->bi_iter.bi_size; INIT_LIST_HEAD(&sums->list);- if (contig)- offset = file_start;- else- offset = 0; /* shut up gcc */- sums->bytenr = bio->bi_iter.bi_sector << 9; index = 0; shash->tfm = fs_info->csum_shash; bio_for_each_segment(bvec, bio, iter) {- if (!contig)+ if (page_offsets) offset = page_offset(bvec.bv_page) + bvec.bv_offset; if (!ordered) {
@@ -668,13 +663,14 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio, BUG_ON(!ordered); /* Logic error */ }- nr_sectors = BTRFS_BYTES_TO_BLKS(fs_info,+ blockcount = BTRFS_BYTES_TO_BLKS(fs_info, bvec.bv_len + fs_info->sectorsize - 1);- for (i = 0; i < nr_sectors; i++) {- if (offset >= ordered->file_offset + ordered->num_bytes ||- offset < ordered->file_offset) {+ for (i = 0; i < blockcount; i++) {+ if (!one_ordered &&+ (offset >= ordered->file_offset + ordered->num_bytes ||+ offset < ordered->file_offset)) {
Since you are changing this hunk, how about using the in_range macro:
if (!one_ordered && !in_range(offset, ordered->file_offset,
ordered->num_bytes) { foo
Will do (although I don't like that that macro evaluates b and first
twice. Someone changing the code later might not notice that.)
Though I think the "change the ordered extent now that we are working on
a different range" code should be factored out in a separate function
because currently it's somewhat breaking the flow of reading.
quoted
unsigned long bytes_left;
sums->len = this_sum_bytes;
@@ -705,7 +701,8 @@ blk_status_t btrfs_csum_one_bio(struct btrfs_inode *inode, struct bio *bio, sums->sums + index); kunmap_atomic(data); index += fs_info->csum_size;- offset += fs_info->sectorsize;+ if (!one_ordered)+ offset += fs_info->sectorsize;
Instead of adding one additional conditional op can't offset always be
incremented but in the case of one_ordered then the !in_range check
should always be false i.e we won't be using the offset to lookup a new OE?
I did it conditionally so that it's clearer that the offset isn't needed
for the one_ordered case, but I can drop the check.
On Fri, Aug 20, 2021 at 11:34:17AM +0300, Nikolay Borisov wrote:
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted
From: Omar Sandoval <redacted>
Currently, we only create ordered extents when ram_bytes == num_bytes
and offset == 0. However, RWF_ENCODED writes may create extents which
only refer to a subset of the full unencoded extent, so we need to plumb
Can you give an example of such a case?
It happens whenever we have bookend extents. Here's an easy example:
$ dd if=/dev/zero of=file bs=12k count=1
1+0 records in
1+0 records out
12288 bytes (12 kB, 12 KiB) copied, 0.000202106 s, 60.8 MB/s
$ sync
$ truncate -s 8k file
$ sync
$ sudo ~/repos/osandov-linux/scripts/btrfs_map_physical ./file | column -ts$'\t'
FILE OFFSET FILE SIZE EXTENT OFFSET EXTENT TYPE LOGICAL SIZE LOGICAL OFFSET PHYSICAL SIZE DEVID PHYSICAL OFFSET
0 8192 0 regular,compression=zstd 12288 217173213184 4096 1 217173213184
The decompressed data is 12k, but we only use 8k for the file.
On Fri, Aug 20, 2021 at 11:56:37AM +0300, Nikolay Borisov wrote:
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted
From: Omar Sandoval <redacted>
In order to allow sending and receiving compressed data without
decompressing it, we need an interface to write pre-compressed data
directly to the filesystem and the matching interface to read compressed
data without decompressing it. This adds the definitions for ioctls to
do that and detailed explanations of how to use them.
Signed-off-by: Omar Sandoval <redacted>
---
include/uapi/linux/btrfs.h | 132 +++++++++++++++++++++++++++++++++++++
1 file changed, 132 insertions(+)
@@ -861,6 +861,134 @@ struct btrfs_ioctl_get_subvol_rootref_args {__u8align[7];};+/*+*Dataandmetadataforanencodedreadorwrite.+*+*EncodedI/Obypassesanyencodingautomaticallydonebythefilesystem(e.g.,+*compression).Thiscanbeusedtoreadthecompressedcontentsofafileor+*writepre-compresseddatadirectlytoafile.+*+*BTRFS_IOC_ENCODED_READandBTRFS_IOC_ENCODED_WRITEareessentially+*preadv/pwritevwithadditionalmetadataabouthowthedataisencodedandthe+*sizeoftheunencodeddata.+*+*BTRFS_IOC_ENCODED_READfillsthegiveniovecswiththeencodeddata,fills+*themetadatafields,andreturnsthesizeoftheencodeddata.Itreadsone+*extentpercall.Itcanalsoreaddatawhichisnotencoded.+*+*BTRFS_IOC_ENCODED_WRITEusesthemetadatafields,writestheencodeddata+*fromtheiovecs,andreturnsthesizeoftheencodeddata.Notethatthe+*encodeddataisnotvalidatedwhenitiswritten;ifitisnotvalid(e.g.,+*itcannotbedecompressed),thenasubsequentreadmayreturnanerror.+*+*Sincethefilesystempagecachecontainsdecodeddata,encodedI/Obypasses+*thepagecache.EncodedI/OrequiresCAP_SYS_ADMIN.+*/+structbtrfs_ioctl_encoded_io_args{+/* Input parameters for both reads and writes. */++/*+*iovecscontainingencodeddata.+*+*Forreads,ifthesizeoftheencodeddataislargerthanthesumof+*iov[n].iov_lenfor0<=n<iovcnt,thentheioctlfailswith+*ENOBUFS.+*+*Forwrites,thesizeoftheencodeddataisthesumofiov[n].iov_len+*for0<=n<iovcnt.Thismustbelessthan128KiB(thislimitmay+*increaseinthefuture).Thismustalsobelessthanorequalto+*unencoded_len.+*/+conststructiovec__user*iov;+/* Number of iovecs. */+unsignedlongiovcnt;+/*+*Offsetinfile.+*+*Forwrites,mustbealignedtothesectorsizeofthefilesystem.+*/+__s64offset;+/* Currently must be zero. */+__u64flags;++/*+*Forreads,thefollowingmembersarefilledinwiththemetadatafor+*theencodeddata.+*Forwrites,thefollowingmembersmustbesettothemetadataforthe+*encodeddata.+*/++/*+*Lengthofthedatainthefile.+*+*Mustbelessthanorequaltounencoded_len-unencoded_offset.For+*writes,mustbealignedtothesectorsizeofthefilesystemunless+*thedataendsatorbeyondthecurrentendofthefile.+*/+__u64len;+/*+*Lengthoftheunencoded(i.e.,decryptedanddecompressed)data.+*+*Forwrites,mustbenomorethan128KiB(thislimitmayincreasein+*thefuture).Iftheunencodeddataisactuallylongerthan+*unencoded_len,thenitistruncated;ifitisshorter,thenitis+*extendedwithzeroes.+*/+__u64unencoded_len;+/*+*Offsetfromthefirstbyteoftheunencodeddatatothefirstbyteof+*logicaldatainthefile.+*+*Mustbelessthanunencoded_len.+*/+__u64unencoded_offset;+/*+*BTRFS_ENCODED_IO_COMPRESSION_*type.+*+*Forwrites,mustnotbeBTRFS_ENCODED_IO_COMPRESSION_NONE.+*/+__u32compression;+/* Currently always BTRFS_ENCODED_IO_ENCRYPTION_NONE. */+__u32encryption;+/*+*Reservedforfutureexpansion.+*+*Forreads,alwaysreturnedaszero.Usersshouldcheckfornon-zero+*bytes.Ifthereareany,thenthekernelhasanewerversionofthis+*structurewithadditionalinformationthattheuserdefinitionis+*missing.+*+*Forwrites,mustbezeroed.+*/+__u8reserved[32];+};++/* Data is not compressed. */+#define BTRFS_ENCODED_IO_COMPRESSION_NONE 0+/* Data is compressed as a single zlib stream. */+#define BTRFS_ENCODED_IO_COMPRESSION_ZLIB 1+/*+*DataiscompressedasasinglezstdframewiththewindowLogcompression+*parametersettonomorethan17.+*/+#define BTRFS_ENCODED_IO_COMPRESSION_ZSTD 2+/*+*Dataiscompressedpagebypage(usingthepagesizeindicatedbythenameof+*theconstant)withLZO1Xandwrappedintheformatdocumentedin+*fs/btrfs/lzo.c.Forwrites,thecompressionpagesizemustmatchthe+*filesystempagesize.+*/+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_4K 3+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_8K 4+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_16K 5+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_32K 6+#define BTRFS_ENCODED_IO_COMPRESSION_LZO_64K 7+#define BTRFS_ENCODED_IO_COMPRESSION_TYPES 8++/* Data is not encrypted. */+#define BTRFS_ENCODED_IO_ENCRYPTION_NONE 0+#define BTRFS_ENCODED_IO_ENCRYPTION_TYPES 1
How about an enums for encryption/compression.
With #define, the user can use #ifdef to check if the constants are
defined and provide their own definitions if not (that's what I did in
the xfstests example programs). Another option is the enum+#define
pattern:
enum {
BTRFS_ENCODED_IO_COMPRESSION_NONE,
#define BTRFS_ENCODED_IO_COMPRESSION_NONE BTRFS_ENCODED_IO_COMPRESSION_NONE
BTRFS_ENCODED_IO_COMPRESSION_ZLIB,
#define BTRFS_ENCODED_IO_COMPRESSION_ZLIB BTRFS_ENCODED_IO_COMPRESSION_ZLIB
BTRFS_ENCODED_IO_COMPRESSION_ZSTD,
#define BTRFS_ENCODED_IO_COMPRESSION_ZSTD BTRFS_ENCODED_IO_COMPRESSION_ZSTD
BTRFS_ENCODED_IO_COMPRESSION_LZO_4K,
#define BTRFS_ENCODED_IO_COMPRESSION_LZO_4K BTRFS_ENCODED_IO_COMPRESSION_LZO_4K
BTRFS_ENCODED_IO_COMPRESSION_LZO_8K,
#define BTRFS_ENCODED_IO_COMPRESSION_LZO_8K BTRFS_ENCODED_IO_COMPRESSION_LZO_8K
BTRFS_ENCODED_IO_COMPRESSION_LZO_16K,
#define BTRFS_ENCODED_IO_COMPRESSION_LZO_16K BTRFS_ENCODED_IO_COMPRESSION_LZO_16K
BTRFS_ENCODED_IO_COMPRESSION_LZO_32K,
#define BTRFS_ENCODED_IO_COMPRESSION_LZO_32K BTRFS_ENCODED_IO_COMPRESSION_LZO_32K
BTRFS_ENCODED_IO_COMPRESSION_LZO_64K,
#define BTRFS_ENCODED_IO_COMPRESSION_LZO_64K BTRFS_ENCODED_IO_COMPRESSION_LZO_64K
BTRFS_ENCODED_IO_COMPRESSION_TYPES,
};
But that seems to confuse people. I don't feel strongly one way or
another.
On Fri, Aug 20, 2021 at 03:30:02PM +0300, Nikolay Borisov wrote:
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted
From: Omar Sandoval <redacted>
There are 4 main cases:
1. Inline extents: we copy the data straight out of the extent buffer.
2. Hole/preallocated extents: we fill in zeroes.
3. Regular, uncompressed extents: we read the sectors we need directly
from disk.
4. Regular, compressed extents: we read the entire compressed extent
from disk and indicate what subset of the decompressed extent is in
the file.
This initial implementation simplifies a few things that can be improved
in the future:
- We hold the inode lock during the operation.
- Cases 1, 3, and 4 allocate temporary memory to read into before
copying out to userspace.
- We don't do read repair, because it turns out that read repair is
currently broken for compressed data.
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/ctree.h | 4 +
fs/btrfs/inode.c | 489 +++++++++++++++++++++++++++++++++++++++++++++++
fs/btrfs/ioctl.c | 111 +++++++++++
3 files changed, 604 insertions(+)
@@ -10496,6 +10496,495 @@ void btrfs_set_range_writeback(struct btrfs_inode *inode, u64 start, u64 end)}}+staticintbtrfs_encoded_io_compression_from_extent(intcompress_type)+{+switch(compress_type){+caseBTRFS_COMPRESS_NONE:+returnBTRFS_ENCODED_IO_COMPRESSION_NONE;+caseBTRFS_COMPRESS_ZLIB:+returnBTRFS_ENCODED_IO_COMPRESSION_ZLIB;+caseBTRFS_COMPRESS_LZO:+/*+*TheLZOformatdependsonthepagesize.64kisthemaximum+*sectorsize(andthuspagesize)thatwesupport.+*/+if(PAGE_SIZE<SZ_4K||PAGE_SIZE>SZ_64K)+return-EINVAL;+returnBTRFS_ENCODED_IO_COMPRESSION_LZO_4K+(PAGE_SHIFT-12);+caseBTRFS_COMPRESS_ZSTD:+returnBTRFS_ENCODED_IO_COMPRESSION_ZSTD;+default:+return-EUCLEAN;+}+}++staticssize_tbtrfs_encoded_read_inline(+structkiocb*iocb,+structiov_iter*iter,u64start,+u64lockend,+structextent_state**cached_state,+u64extent_start,size_tcount,+structbtrfs_ioctl_encoded_io_args*encoded,+bool*unlocked)+{+structinode*inode=file_inode(iocb->ki_filp);+structextent_io_tree*io_tree=&BTRFS_I(inode)->io_tree;+structbtrfs_path*path;+structextent_buffer*leaf;+structbtrfs_file_extent_item*item;+u64ram_bytes;+unsignedlongptr;+void*tmp;+ssize_tret;++path=btrfs_alloc_path();+if(!path){+ret=-ENOMEM;+gotoout;+}+ret=btrfs_lookup_file_extent(NULL,BTRFS_I(inode)->root,path,+btrfs_ino(BTRFS_I(inode)),extent_start,+0);+if(ret){+if(ret>0){+/* The extent item disappeared? */+ret=-EIO;+}+gotoout;+}+leaf=path->nodes[0];+item=btrfs_item_ptr(leaf,path->slots[0],+structbtrfs_file_extent_item);++ram_bytes=btrfs_file_extent_ram_bytes(leaf,item);+ptr=btrfs_file_extent_inline_start(item);++encoded->len=(min_t(u64,extent_start+ram_bytes,inode->i_size)-+iocb->ki_pos);+ret=btrfs_encoded_io_compression_from_extent(+btrfs_file_extent_compression(leaf,item));+if(ret<0)+gotoout;+encoded->compression=ret;+if(encoded->compression){+size_tinline_size;++inline_size=btrfs_file_extent_inline_item_len(leaf,+btrfs_item_nr(path->slots[0]));+if(inline_size>count){+ret=-ENOBUFS;+gotoout;+}+count=inline_size;+encoded->unencoded_len=ram_bytes;+encoded->unencoded_offset=iocb->ki_pos-extent_start;+}else{+encoded->len=encoded->unencoded_len=count=+min_t(u64,count,encoded->len);+ptr+=iocb->ki_pos-extent_start;+}++tmp=kmalloc(count,GFP_NOFS);+if(!tmp){+ret=-ENOMEM;+gotoout;+}+read_extent_buffer(leaf,tmp,ptr,count);+btrfs_release_path(path);+unlock_extent_cached(io_tree,start,lockend,cached_state);+inode_unlock_shared(inode);+*unlocked=true;++ret=copy_to_iter(tmp,count,iter);+if(ret!=count)+ret=-EFAULT;+kfree(tmp);+out:+btrfs_free_path(path);+returnret;+}++structbtrfs_encoded_read_private{+structinode*inode;+wait_queue_head_twait;+atomic_tpending;+blk_status_tstatus;+boolskip_csum;+};++staticblk_status_tsubmit_encoded_read_bio(structinode*inode,+structbio*bio,intmirror_num,+unsignedlongbio_flags)+{+structbtrfs_encoded_read_private*priv=bio->bi_private;+structbtrfs_io_bio*io_bio=btrfs_io_bio(bio);+structbtrfs_fs_info*fs_info=btrfs_sb(inode->i_sb);+blk_status_tret;++if(!priv->skip_csum){+ret=btrfs_lookup_bio_sums(inode,bio,NULL);+if(ret)+returnret;+}++ret=btrfs_bio_wq_end_io(fs_info,bio,BTRFS_WQ_ENDIO_DATA);+if(ret){+btrfs_io_bio_free_csum(io_bio);+returnret;+}++atomic_inc(&priv->pending);+ret=btrfs_map_bio(fs_info,bio,mirror_num);+if(ret){+atomic_dec(&priv->pending);+btrfs_io_bio_free_csum(io_bio);+}+returnret;+}++staticblk_status_tbtrfs_encoded_read_check_bio(structbtrfs_io_bio*io_bio)+{+constbooluptodate=io_bio->bio.bi_status==BLK_STS_OK;+structbtrfs_encoded_read_private*priv=io_bio->bio.bi_private;+structinode*inode=priv->inode;+structbtrfs_fs_info*fs_info=btrfs_sb(inode->i_sb);+u32sectorsize=fs_info->sectorsize;+structbio_vec*bvec;+structbvec_iter_alliter_all;+u64start=io_bio->logical;+u32bio_offset=0;++if(priv->skip_csum||!uptodate)+returnio_bio->bio.bi_status;++bio_for_each_segment_all(bvec,&io_bio->bio,iter_all){+unsignedinti,nr_sectors,pgoff;++nr_sectors=BTRFS_BYTES_TO_BLKS(fs_info,bvec->bv_len);+pgoff=bvec->bv_offset;+for(i=0;i<nr_sectors;i++){+ASSERT(pgoff<PAGE_SIZE);+if(check_data_csum(inode,io_bio,bio_offset,+bvec->bv_page,pgoff,start))+returnBLK_STS_IOERR;+start+=sectorsize;+bio_offset+=sectorsize;+pgoff+=sectorsize;+}+}+returnBLK_STS_OK;+}++staticvoidbtrfs_encoded_read_endio(structbio*bio)+{+structbtrfs_encoded_read_private*priv=bio->bi_private;+structbtrfs_io_bio*io_bio=btrfs_io_bio(bio);+blk_status_tstatus;++status=btrfs_encoded_read_check_bio(io_bio);+if(status){+/*+*Thememorybarrierimpliedbytheatomic_dec_return()here+*pairswiththememorybarrierimpliedbythe+*atomic_dec_return()orio_wait_event()in+*btrfs_encoded_read_regular_fill_pages()toensurethatthis+*writeisobservedbeforetheloadofstatusin+*btrfs_encoded_read_regular_fill_pages().+*/+WRITE_ONCE(priv->status,status);+}+if(!atomic_dec_return(&priv->pending))+wake_up(&priv->wait);+btrfs_io_bio_free_csum(io_bio);+bio_put(bio);+}++staticintbtrfs_encoded_read_regular_fill_pages(structinode*inode,u64offset,+u64disk_io_size,structpage**pages)+{+structbtrfs_fs_info*fs_info=btrfs_sb(inode->i_sb);+structbtrfs_encoded_read_privatepriv={+.inode=inode,+.pending=ATOMIC_INIT(1),+.skip_csum=BTRFS_I(inode)->flags&BTRFS_INODE_NODATASUM,+};+unsignedlongi=0;+u64cur=0;+intret;++init_waitqueue_head(&priv.wait);+/*+*Submitbiosfortheextent,splittingduetobioorstripelimitsas+*necessary.+*/+while(cur<disk_io_size){+structextent_map*em;+structbtrfs_io_geometrygeom;+structbio*bio=NULL;+u64remaining;++em=btrfs_get_chunk_map(fs_info,offset+cur,+disk_io_size-cur);+if(IS_ERR(em)){+ret=PTR_ERR(em);+}else{+ret=btrfs_get_io_geometry(fs_info,em,BTRFS_MAP_READ,+offset+cur,&geom);+free_extent_map(em);+}+if(ret){+WRITE_ONCE(priv.status,errno_to_blk_status(ret));+break;+}+remaining=min(geom.len,disk_io_size-cur);+while(bio||remaining){+size_tbytes=min_t(u64,remaining,PAGE_SIZE);++if(!bio){+bio=btrfs_bio_alloc(offset+cur);+bio->bi_end_io=btrfs_encoded_read_endio;+bio->bi_private=&priv;+bio->bi_opf=REQ_OP_READ;+}++if(!bytes||+bio_add_page(bio,pages[i],bytes,0)<bytes){+blk_status_tstatus;++status=submit_encoded_read_bio(inode,bio,0,+0);+if(status){+WRITE_ONCE(priv.status,status);+bio_put(bio);+gotoout;+}+bio=NULL;+continue;+}++i++;+cur+=bytes;+remaining-=bytes;+}+}++out:+if(atomic_dec_return(&priv.pending))+io_wait_event(priv.wait,!atomic_read(&priv.pending));+/* See btrfs_encoded_read_endio() for ordering. */+returnblk_status_to_errno(READ_ONCE(priv.status));+}++staticssize_tbtrfs_encoded_read_regular(structkiocb*iocb,+structiov_iter*iter,+u64start,u64lockend,+structextent_state**cached_state,+u64offset,u64disk_io_size,+size_tcount,boolcompressed,+bool*unlocked)+{+structinode*inode=file_inode(iocb->ki_filp);+structextent_io_tree*io_tree=&BTRFS_I(inode)->io_tree;+structpage**pages;+unsignedlongnr_pages,i;+u64cur;+size_tpage_offset;+ssize_tret;++nr_pages=DIV_ROUND_UP(disk_io_size,PAGE_SIZE);+pages=kcalloc(nr_pages,sizeof(structpage*),GFP_NOFS);+if(!pages)+return-ENOMEM;+for(i=0;i<nr_pages;i++){+pages[i]=alloc_page(GFP_NOFS|__GFP_HIGHMEM);+if(!pages[i]){+ret=-ENOMEM;+gotoout;+}+}++ret=btrfs_encoded_read_regular_fill_pages(inode,offset,disk_io_size,+pages);+if(ret)+gotoout;++unlock_extent_cached(io_tree,start,lockend,cached_state);+inode_unlock_shared(inode);+*unlocked=true;++if(compressed){+i=0;+page_offset=0;+}else{+i=(iocb->ki_pos-start)>>PAGE_SHIFT;+page_offset=(iocb->ki_pos-start)&(PAGE_SIZE-1);+}+cur=0;+while(cur<count){+size_tbytes=min_t(size_t,count-cur,+PAGE_SIZE-page_offset);++if(copy_page_to_iter(pages[i],page_offset,bytes,+iter)!=bytes){+ret=-EFAULT;+gotoout;+}+i++;+cur+=bytes;+page_offset=0;+}+ret=count;+out:+for(i=0;i<nr_pages;i++){+if(pages[i])+__free_page(pages[i]);+}+kfree(pages);+returnret;+}++ssize_tbtrfs_encoded_read(structkiocb*iocb,structiov_iter*iter,+structbtrfs_ioctl_encoded_io_args*encoded)+{+structinode*inode=file_inode(iocb->ki_filp);+structbtrfs_fs_info*fs_info=btrfs_sb(inode->i_sb);+structextent_io_tree*io_tree=&BTRFS_I(inode)->io_tree;+ssize_tret;+size_tcount=iov_iter_count(iter);+u64start,lockend,offset,disk_io_size;+structextent_state*cached_state=NULL;+structextent_map*em;+boolunlocked=false;++file_accessed(iocb->ki_filp);++inode_lock_shared(inode);++if(iocb->ki_pos>=inode->i_size){+inode_unlock_shared(inode);+return0;
Don't we need to signal beyond EOF reads somehow. As it stands returning
0 means returning zeroed portion of btrfs_ioctl_encoded_io_args to user
space?
If you do a normal read/pread beyond EOF, the syscall returns 0,
indicating that there are no bytes to read at that offset. We're doing
the same thing here: returning 0 means 0 compressed bytes were read, and
setting btrfs_ioctl_encoded_io_args::len to 0 means 0 bytes in the file
were read.
quoted
+ }+ start = ALIGN_DOWN(iocb->ki_pos, fs_info->sectorsize);+ /*+ * We don't know how long the extent containing iocb->ki_pos is, but if+ * it's compressed we know that it won't be longer than this.+ */+ lockend = start + BTRFS_MAX_UNCOMPRESSED - 1;++ for (;;) {+ struct btrfs_ordered_extent *ordered;++ ret = btrfs_wait_ordered_range(inode, start,+ lockend - start + 1);+ if (ret)+ goto out_unlock_inode;+ lock_extent_bits(io_tree, start, lockend, &cached_state);+ ordered = btrfs_lookup_ordered_range(BTRFS_I(inode), start,+ lockend - start + 1);+ if (!ordered)+ break;+ btrfs_put_ordered_extent(ordered);+ unlock_extent_cached(io_tree, start, lockend, &cached_state);+ cond_resched();+ }
Can't you simply use btrfs_lock_and_flush_ordered_range, the major
difference is btrfs_wait_ordered_range basically instantiates any
pending delalloc, whilst btrfs_lock_and_flush_ordered_range returns with
any, already-instantiated OE run to completion and the range locked ?
On Fri, Aug 20, 2021 at 04:44:26PM +0300, Nikolay Borisov wrote:
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted
From: Omar Sandoval <redacted>
The implementation resembles direct I/O: we have to flush any ordered
extents, invalidate the page cache, and do the io tree/delalloc/extent
map/ordered extent dance. From there, we can reuse the compression code
with a minor modification to distinguish the write from writeback. This
also creates inline extents when possible.
Signed-off-by: Omar Sandoval <redacted>
<snip>
quoted
* Add an entry indicating a block group or device which is pinned by a
@@ -103,6 +103,8 @@ struct btrfs_ioctl_encoded_io_args_32 {#define BTRFS_IOC_ENCODED_READ_32 _IOR(BTRFS_IOCTL_MAGIC, 64, \structbtrfs_ioctl_encoded_io_args_32)+#define BTRFS_IOC_ENCODED_WRITE_32 _IOW(BTRFS_IOCTL_MAGIC, 64, \+structbtrfs_ioctl_encoded_io_args_32)#endif/* Mask out flags that are inappropriate for the given type of inode. */
Do you intend on supporting encrypted data writeout in the future, given
that in btrfs_do_encoded_write EINVAL is returned if the data to be
written is encrypted? If not then this check could be moved earlier to
fail fast.
We probably want to support it at some point in the future, yes.
quoted
@@ -5138,9 +5236,13 @@ long btrfs_ioctl(struct file *file, unsigned int return fsverity_ioctl_measure(file, argp); case BTRFS_IOC_ENCODED_READ: return btrfs_ioctl_encoded_read(file, argp, false);+ case BTRFS_IOC_ENCODED_WRITE:+ return btrfs_ioctl_encoded_write(file, argp, false); #if defined(CONFIG_64BIT) && defined(CONFIG_COMPAT) case BTRFS_IOC_ENCODED_READ_32: return btrfs_ioctl_encoded_read(file, argp, true);+ case BTRFS_IOC_ENCODED_WRITE_32:+ return btrfs_ioctl_encoded_write(file, argp, true); #endif }
@@ -74,6 +74,8 @@ enum {BTRFS_ORDERED_LOGGED_CSUM,/* We wait for this extent to complete in the current transaction */BTRFS_ORDERED_PENDING,+/* RWF_ENCODED I/O */
nit: RWF_ENCODED is no longer, we simply have ioctl-based encoded io. So
this needs to be renamed to avoid confusion for people not necessarily
faimilar with the development history of the feature.
On Fri, Aug 20, 2021 at 05:13:34PM +0800, Qu Wenruo wrote:
On 2021/8/20 下午4:51, Nikolay Borisov wrote:
quoted
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted
From: Omar Sandoval <redacted>
Currently, an inline extent is always created after i_size is extended
from btrfs_dirty_pages(). However, for encoded writes, we only want to
update i_size after we successfully created the inline extent.
To me, the idea of write first then update isize is just going to cause
tons of inline extent related prblems.
The current example is falloc, which only update the isize after the
falloc finishes.
This behavior has already bothered me quite a lot, as it can easily
create mixed inline and regular extents.
Do you have an example of how this would happen? I have the inode and
extent bits locked during an encoded write, and I see that fallocate
does the same.
Can't we remember the old isize (with proper locking), enlarge isize
(with holes filled), do the write.
If something wrong happened, we truncate the isize back to its old isize.
quoted
quoted
Add an
update_i_size parameter to cow_file_range_inline() and
insert_inline_extent() and pass in the size of the extent rather than
determining it from i_size. Since the start parameter is always passed
as 0, get rid of it and simplify the logic in these two functions. While
we're here, let's document the requirements for creating an inline
extent.
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Omar Sandoval <redacted>
---
fs/btrfs/inode.c | 100 +++++++++++++++++++++++------------------------
1 file changed, 48 insertions(+), 52 deletions(-)
@@ -236,9 +236,10 @@ static int btrfs_init_inode_security(struct btrfs_trans_handle *trans,staticintinsert_inline_extent(structbtrfs_trans_handle*trans,structbtrfs_path*path,boolextent_inserted,structbtrfs_root*root,structinode*inode,-u64start,size_tsize,size_tcompressed_size,+size_tsize,size_tcompressed_size,intcompress_type,-structpage**compressed_pages)+structpage**compressed_pages,+boolupdate_i_size){structextent_buffer*leaf;structpage*page=NULL;
@@ -247,7 +248,7 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,structbtrfs_file_extent_item*ei;intret;size_tcur_size=size;-unsignedlongoffset;+u64i_size;ASSERT((compressed_size>0&&compressed_pages)||(compressed_size==0&&!compressed_pages));
@@ -260,7 +261,7 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,size_tdatasize;key.objectid=btrfs_ino(BTRFS_I(inode));-key.offset=start;+key.offset=0;key.type=BTRFS_EXTENT_DATA_KEY;datasize=btrfs_file_extent_calc_inline_size(cur_size);
@@ -297,12 +298,10 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,btrfs_set_file_extent_compression(leaf,ei,compress_type);}else{-page=find_get_page(inode->i_mapping,-start>>PAGE_SHIFT);+page=find_get_page(inode->i_mapping,0);btrfs_set_file_extent_compression(leaf,ei,0);kaddr=kmap_atomic(page);-offset=offset_in_page(start);-write_extent_buffer(leaf,kaddr+offset,ptr,size);+write_extent_buffer(leaf,kaddr,ptr,size);kunmap_atomic(kaddr);put_page(page);}
@@ -313,8 +312,8 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,*Wealignsizetosectorsizeforinlineextentsjustforsimplicity*sake.*/-size=ALIGN(size,root->fs_info->sectorsize);-ret=btrfs_inode_set_file_extent_range(BTRFS_I(inode),start,size);+ret=btrfs_inode_set_file_extent_range(BTRFS_I(inode),0,+ALIGN(size,root->fs_info->sectorsize));if(ret)gotofail;
@@ -327,7 +326,13 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,*beforeweunlockthepages.Otherwisewe*couldendupracingwithunlink.*/-BTRFS_I(inode)->disk_i_size=inode->i_size;+i_size=i_size_read(inode);+if(update_i_size&&size>i_size){+i_size_write(inode,size);+i_size=size;+}+BTRFS_I(inode)->disk_i_size=i_size;+fail:returnret;}
@@ -338,35 +343,31 @@ static int insert_inline_extent(struct btrfs_trans_handle *trans,*doesthechecksrequiredtomakesurethedataissmallenough*tofitasaninlineextent.*/-staticnoinlineintcow_file_range_inline(structbtrfs_inode*inode,u64start,-u64end,size_tcompressed_size,+staticnoinlineintcow_file_range_inline(structbtrfs_inode*inode,u64size,+size_tcompressed_size,intcompress_type,-structpage**compressed_pages)+structpage**compressed_pages,+boolupdate_i_size){structbtrfs_drop_extents_argsdrop_args={0};structbtrfs_root*root=inode->root;structbtrfs_fs_info*fs_info=root->fs_info;structbtrfs_trans_handle*trans;-u64isize=i_size_read(&inode->vfs_inode);-u64actual_end=min(end+1,isize);-u64inline_len=actual_end-start;-u64aligned_end=ALIGN(end,fs_info->sectorsize);-u64data_len=inline_len;+u64data_len=compressed_size?compressed_size:size;intret;structbtrfs_path*path;-if(compressed_size)-data_len=compressed_size;--if(start>0||-actual_end>fs_info->sectorsize||+/*+*Wecancreateaninlineextentifitendsatorbeyondthecurrent+*i_size,isnolargerthanasector(decompressed),andthe(possibly+*compressed)datafitsinaleafandtheconfiguredmaximuminline+*size.+*/
Urgh, just some days ago Qu was talking about how awkward it is to have
mixed extents in a file. And now, AFAIU, you are making them more likely
since now they can be created not just at the beginning of the file but
also after i_size write. While this won't be a problem in and of itself
it goes just the opposite way of us trying to shrink the possible cases
when we can have mixed extents.
Tree-checker should reject such inline extent at non-zero offset.
This change does not allow creating inline extents at a non-zero offset.
quoted
Qu what is your take on that?
My question is, why encoded write needs to bother the inline extents at all?
My intuition of such encoded write is, it should not create inline
extents at all.
Or is there any special use-case involved for encoded write?
We create compressed inline extents with normal writes. We should be
able to send and receive them without converting them into regular
extents.
On Fri, Aug 20, 2021 at 05:13:34PM +0800, Qu Wenruo wrote:
quoted
On 2021/8/20 下午4:51, Nikolay Borisov wrote:
quoted
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted
From: Omar Sandoval <redacted>
Currently, an inline extent is always created after i_size is extended
from btrfs_dirty_pages(). However, for encoded writes, we only want to
update i_size after we successfully created the inline extent.
To me, the idea of write first then update isize is just going to cause
tons of inline extent related prblems.
The current example is falloc, which only update the isize after the
falloc finishes.
This behavior has already bothered me quite a lot, as it can easily
create mixed inline and regular extents.
Do you have an example of how this would happen? I have the inode and
extent bits locked during an encoded write, and I see that fallocate
does the same.
xfs_io -f -c "pwrite 0 1K" -c "sync" -c "falloc 0 4k" -c "pwrite 4k 4k"
The [0, 1K) will be written as inline without doubt.
Then we go to falloc, it will try to zero the range [1K, 4K), but it
doesn't increase the isize.
Thus the page [0, 4k) will still be written back as inline, since isize
is still 1K.
Later [4K, 8K) will be written back as regular, causing mixed extents.
quoted
Can't we remember the old isize (with proper locking), enlarge isize
(with holes filled), do the write.
If something wrong happened, we truncate the isize back to its old isize.
[...]
quoted
quoted
Urgh, just some days ago Qu was talking about how awkward it is to have
mixed extents in a file. And now, AFAIU, you are making them more likely
since now they can be created not just at the beginning of the file but
also after i_size write. While this won't be a problem in and of itself
it goes just the opposite way of us trying to shrink the possible cases
when we can have mixed extents.
Tree-checker should reject such inline extent at non-zero offset.
This change does not allow creating inline extents at a non-zero offset.
quoted
quoted
Qu what is your take on that?
My question is, why encoded write needs to bother the inline extents at all?
My intuition of such encoded write is, it should not create inline
extents at all.
Or is there any special use-case involved for encoded write?
We create compressed inline extents with normal writes. We should be
able to send and receive them without converting them into regular
extents.
But my first impression for any encoded write is that, they should work
like DIO, thus everything should be sectorsize aligned.
Then why could they create inline extent? As inline extent can only be
possible when the isize is smaller than sectorsize.
Thanks,
Qu
On Sat, Aug 21, 2021 at 09:11:26AM +0800, Qu Wenruo wrote:
On 2021/8/21 上午2:11, Omar Sandoval wrote:
quoted
On Fri, Aug 20, 2021 at 05:13:34PM +0800, Qu Wenruo wrote:
quoted
On 2021/8/20 下午4:51, Nikolay Borisov wrote:
quoted
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted
From: Omar Sandoval <redacted>
Currently, an inline extent is always created after i_size is extended
from btrfs_dirty_pages(). However, for encoded writes, we only want to
update i_size after we successfully created the inline extent.
To me, the idea of write first then update isize is just going to cause
tons of inline extent related prblems.
The current example is falloc, which only update the isize after the
falloc finishes.
This behavior has already bothered me quite a lot, as it can easily
create mixed inline and regular extents.
Do you have an example of how this would happen? I have the inode and
extent bits locked during an encoded write, and I see that fallocate
does the same.
xfs_io -f -c "pwrite 0 1K" -c "sync" -c "falloc 0 4k" -c "pwrite 4k 4k"
The [0, 1K) will be written as inline without doubt.
Then we go to falloc, it will try to zero the range [1K, 4K), but it
doesn't increase the isize.
Thus the page [0, 4k) will still be written back as inline, since isize
is still 1K.
Later [4K, 8K) will be written back as regular, causing mixed extents.
I'll have to read fallocate more closely to follow what's going on here
and figure out if it applies to encoded writes. Please help me out if
you see how this would be an issue with encoded writes.
quoted
quoted
Can't we remember the old isize (with proper locking), enlarge isize
(with holes filled), do the write.
If something wrong happened, we truncate the isize back to its old isize.
[...]
quoted
quoted
quoted
Urgh, just some days ago Qu was talking about how awkward it is to have
mixed extents in a file. And now, AFAIU, you are making them more likely
since now they can be created not just at the beginning of the file but
also after i_size write. While this won't be a problem in and of itself
it goes just the opposite way of us trying to shrink the possible cases
when we can have mixed extents.
Tree-checker should reject such inline extent at non-zero offset.
This change does not allow creating inline extents at a non-zero offset.
quoted
quoted
Qu what is your take on that?
My question is, why encoded write needs to bother the inline extents at all?
My intuition of such encoded write is, it should not create inline
extents at all.
Or is there any special use-case involved for encoded write?
We create compressed inline extents with normal writes. We should be
able to send and receive them without converting them into regular
extents.
But my first impression for any encoded write is that, they should work
like DIO, thus everything should be sectorsize aligned.
Then why could they create inline extent? As inline extent can only be
possible when the isize is smaller than sectorsize.
ENCODED_WRITE is not defined as "O_DIRECT, but encoded". It happens to
have some resemblance to O_DIRECT because we have alignment requirements
for new extents and because we bypass the page cache, but there's no
reason to copy arbitrary restrictions from O_DIRECT. If someone is using
ENCODED_WRITE to write compressed data, then they care about space
efficiency, so we should make efficient use of inline extents.
On Sat, Aug 21, 2021 at 09:11:26AM +0800, Qu Wenruo wrote:
quoted
On 2021/8/21 上午2:11, Omar Sandoval wrote:
quoted
On Fri, Aug 20, 2021 at 05:13:34PM +0800, Qu Wenruo wrote:
quoted
On 2021/8/20 下午4:51, Nikolay Borisov wrote:
quoted
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted
From: Omar Sandoval <redacted>
Currently, an inline extent is always created after i_size is extended
from btrfs_dirty_pages(). However, for encoded writes, we only want to
update i_size after we successfully created the inline extent.
To me, the idea of write first then update isize is just going to cause
tons of inline extent related prblems.
The current example is falloc, which only update the isize after the
falloc finishes.
This behavior has already bothered me quite a lot, as it can easily
create mixed inline and regular extents.
Do you have an example of how this would happen? I have the inode and
extent bits locked during an encoded write, and I see that fallocate
does the same.
xfs_io -f -c "pwrite 0 1K" -c "sync" -c "falloc 0 4k" -c "pwrite 4k 4k"
The [0, 1K) will be written as inline without doubt.
Then we go to falloc, it will try to zero the range [1K, 4K), but it
doesn't increase the isize.
Thus the page [0, 4k) will still be written back as inline, since isize
is still 1K.
Later [4K, 8K) will be written back as regular, causing mixed extents.
I'll have to read fallocate more closely to follow what's going on here
and figure out if it applies to encoded writes. Please help me out if
you see how this would be an issue with encoded writes.
This won't cause anything wrong, if the encoded writes follows the
existing inline extents requirement (always at offset 0).
Otherwise, the read path could be affected to handle inlined extent at
non-zero offset.
quoted
quoted
quoted
Can't we remember the old isize (with proper locking), enlarge isize
(with holes filled), do the write.
If something wrong happened, we truncate the isize back to its old isize.
[...]
quoted
quoted
quoted
Urgh, just some days ago Qu was talking about how awkward it is to have
mixed extents in a file. And now, AFAIU, you are making them more likely
since now they can be created not just at the beginning of the file but
also after i_size write. While this won't be a problem in and of itself
it goes just the opposite way of us trying to shrink the possible cases
when we can have mixed extents.
Tree-checker should reject such inline extent at non-zero offset.
This change does not allow creating inline extents at a non-zero offset.
quoted
quoted
Qu what is your take on that?
My question is, why encoded write needs to bother the inline extents at all?
My intuition of such encoded write is, it should not create inline
extents at all.
Or is there any special use-case involved for encoded write?
We create compressed inline extents with normal writes. We should be
able to send and receive them without converting them into regular
extents.
But my first impression for any encoded write is that, they should work
like DIO, thus everything should be sectorsize aligned.
Then why could they create inline extent? As inline extent can only be
possible when the isize is smaller than sectorsize.
ENCODED_WRITE is not defined as "O_DIRECT, but encoded". It happens to
have some resemblance to O_DIRECT because we have alignment requirements
for new extents and because we bypass the page cache, but there's no
reason to copy arbitrary restrictions from O_DIRECT. If someone is using
ENCODED_WRITE to write compressed data, then they care about space
efficiency, so we should make efficient use of inline extents.
Then as long as the inline extent requirement for 0 offset is still
followed, I'll be fine with that.
But for non-zero offset inline extent? It looks like a much larger
change, and may affect read path.
So I'd prefer we keep the 0 offset requirement for inline extent, and
find a better way to work around.
Thanks,
Qu
On Tue, Aug 24, 2021 at 07:32:06AM +0800, Qu Wenruo wrote:
On 2021/8/24 上午2:16, Omar Sandoval wrote:
quoted
On Sat, Aug 21, 2021 at 09:11:26AM +0800, Qu Wenruo wrote:
quoted
On 2021/8/21 上午2:11, Omar Sandoval wrote:
quoted
On Fri, Aug 20, 2021 at 05:13:34PM +0800, Qu Wenruo wrote:
quoted
On 2021/8/20 下午4:51, Nikolay Borisov wrote:
quoted
On 18.08.21 г. 0:06, Omar Sandoval wrote:
quoted
From: Omar Sandoval <redacted>
Currently, an inline extent is always created after i_size is extended
from btrfs_dirty_pages(). However, for encoded writes, we only want to
update i_size after we successfully created the inline extent.
To me, the idea of write first then update isize is just going to cause
tons of inline extent related prblems.
The current example is falloc, which only update the isize after the
falloc finishes.
This behavior has already bothered me quite a lot, as it can easily
create mixed inline and regular extents.
Do you have an example of how this would happen? I have the inode and
extent bits locked during an encoded write, and I see that fallocate
does the same.
xfs_io -f -c "pwrite 0 1K" -c "sync" -c "falloc 0 4k" -c "pwrite 4k 4k"
The [0, 1K) will be written as inline without doubt.
Then we go to falloc, it will try to zero the range [1K, 4K), but it
doesn't increase the isize.
Thus the page [0, 4k) will still be written back as inline, since isize
is still 1K.
Later [4K, 8K) will be written back as regular, causing mixed extents.
I'll have to read fallocate more closely to follow what's going on here
and figure out if it applies to encoded writes. Please help me out if
you see how this would be an issue with encoded writes.
This won't cause anything wrong, if the encoded writes follows the
existing inline extents requirement (always at offset 0).
Otherwise, the read path could be affected to handle inlined extent at
non-zero offset.
quoted
quoted
quoted
quoted
Can't we remember the old isize (with proper locking), enlarge isize
(with holes filled), do the write.
If something wrong happened, we truncate the isize back to its old isize.
[...]
quoted
quoted
quoted
Urgh, just some days ago Qu was talking about how awkward it is to have
mixed extents in a file. And now, AFAIU, you are making them more likely
since now they can be created not just at the beginning of the file but
also after i_size write. While this won't be a problem in and of itself
it goes just the opposite way of us trying to shrink the possible cases
when we can have mixed extents.
Tree-checker should reject such inline extent at non-zero offset.
This change does not allow creating inline extents at a non-zero offset.
quoted
quoted
Qu what is your take on that?
My question is, why encoded write needs to bother the inline extents at all?
My intuition of such encoded write is, it should not create inline
extents at all.
Or is there any special use-case involved for encoded write?
We create compressed inline extents with normal writes. We should be
able to send and receive them without converting them into regular
extents.
But my first impression for any encoded write is that, they should work
like DIO, thus everything should be sectorsize aligned.
Then why could they create inline extent? As inline extent can only be
possible when the isize is smaller than sectorsize.
ENCODED_WRITE is not defined as "O_DIRECT, but encoded". It happens to
have some resemblance to O_DIRECT because we have alignment requirements
for new extents and because we bypass the page cache, but there's no
reason to copy arbitrary restrictions from O_DIRECT. If someone is using
ENCODED_WRITE to write compressed data, then they care about space
efficiency, so we should make efficient use of inline extents.
Then as long as the inline extent requirement for 0 offset is still
followed, I'll be fine with that.
But for non-zero offset inline extent? It looks like a much larger
change, and may affect read path.
So I'd prefer we keep the 0 offset requirement for inline extent, and
find a better way to work around.
Ah, okay. I didn't get rid of the 0 offset requirement and I have no
plans to. In fact, this patch kind of does the opposite: it gets rid of
the start parameter to cow_file_range_inline() because it doesn't make
sense for it to ever be anything other than 0 (and we're already
checking that start == 0 in the callers).