| Message ID | 6527c36f8fd775cf3ca87e5541cf550537d8e757.1712187452.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: more explaination on extent_map members | expand |
On Thu, Apr 4, 2024 at 12:47 AM Qu Wenruo <wqu@suse.com> wrote: > > The function create_io_em() is called before we submit an IO, to update > the in-memory extent map for the involved range. > > This patch changes the following aspects: > > - Does not allow BTRFS_ORDERED_NOCOW type > For real NOCOW (excluding NOCOW writes into preallocated ranges) > writes, we never call create_io_em(), as we does not need to update > the extent map at all. > > So remove the sanity check allowing BTRFS_ORDERED_NOCOW type. > > - Add extra sanity checks > * PREALLOC > - @block_len == len > For uncompressed writes. > > * REGULAR > - @block_len == @orig_block_len == @ram_bytes == @len > We're creating a new uncompressed extent, and referring all of it. > > - @orig_start == @start > We haven no offset inside the extent. > > * COMPRESSED > - valid @compress_type > - @len <= @ram_bytes > This is to co-operate with encoded writes, which can cause a new > file extent referring only part of a uncompressed extent. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/inode.c | 40 +++++++++++++++++++++++++++++++++++++++- > 1 file changed, 39 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index c6f2b5d1dee1..261fafc66533 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -7320,11 +7320,49 @@ static struct extent_map *create_io_em(struct btrfs_inode *inode, u64 start, > struct extent_map *em; > int ret; > > + /* > + * Note the missing of NOCOW type. > + * > + * For pure NOCOW writes, we should not create an io extent map, > + * but just reusing the existing one. > + * Only PREALLOC writes (NOCOW write into preallocated range) can > + * create io extent map. > + */ > ASSERT(type == BTRFS_ORDERED_PREALLOC || > type == BTRFS_ORDERED_COMPRESSED || > - type == BTRFS_ORDERED_NOCOW || > type == BTRFS_ORDERED_REGULAR); > > + if (type == BTRFS_ORDERED_PREALLOC) { > + /* Uncompressed extents. */ > + ASSERT(block_len == len); > + > + /* We're only referring part of a larger preallocated extent. */ > + ASSERT(block_len <= ram_bytes); > + } > + > + if (type == BTRFS_ORDERED_REGULAR) { > + /* Uncompressed extents. */ > + ASSERT(block_len == len); > + > + /* COW results a new extent matching our file extent size. */ > + ASSERT(orig_block_len == len); > + ASSERT(ram_bytes == len); > + > + /* Since it's a new extent, we should not have any offset. */ > + ASSERT(orig_start == start); > + } > + > + if (type == BTRFS_ORDERED_COMPRESSED) { > + /* Must be compressed. */ > + ASSERT(compress_type != BTRFS_COMPRESS_NONE); > + > + /* > + * Encoded write can make us to refer part of the refer part -> refer to part > + * uncompressed extent. > + */ > + ASSERT(len <= ram_bytes); > + } All these if statements to test the type should be combined into if - else if - else if ... If we match a type, it's pointless to keep comparing the type again. Or use a switch statement. Otherwise it looks good, thanks. > + > em = alloc_extent_map(); > if (!em) > return ERR_PTR(-ENOMEM); > -- > 2.44.0 > >
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index c6f2b5d1dee1..261fafc66533 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -7320,11 +7320,49 @@ static struct extent_map *create_io_em(struct btrfs_inode *inode, u64 start, struct extent_map *em; int ret; + /* + * Note the missing of NOCOW type. + * + * For pure NOCOW writes, we should not create an io extent map, + * but just reusing the existing one. + * Only PREALLOC writes (NOCOW write into preallocated range) can + * create io extent map. + */ ASSERT(type == BTRFS_ORDERED_PREALLOC || type == BTRFS_ORDERED_COMPRESSED || - type == BTRFS_ORDERED_NOCOW || type == BTRFS_ORDERED_REGULAR); + if (type == BTRFS_ORDERED_PREALLOC) { + /* Uncompressed extents. */ + ASSERT(block_len == len); + + /* We're only referring part of a larger preallocated extent. */ + ASSERT(block_len <= ram_bytes); + } + + if (type == BTRFS_ORDERED_REGULAR) { + /* Uncompressed extents. */ + ASSERT(block_len == len); + + /* COW results a new extent matching our file extent size. */ + ASSERT(orig_block_len == len); + ASSERT(ram_bytes == len); + + /* Since it's a new extent, we should not have any offset. */ + ASSERT(orig_start == start); + } + + if (type == BTRFS_ORDERED_COMPRESSED) { + /* Must be compressed. */ + ASSERT(compress_type != BTRFS_COMPRESS_NONE); + + /* + * Encoded write can make us to refer part of the + * uncompressed extent. + */ + ASSERT(len <= ram_bytes); + } + em = alloc_extent_map(); if (!em) return ERR_PTR(-ENOMEM);
The function create_io_em() is called before we submit an IO, to update the in-memory extent map for the involved range. This patch changes the following aspects: - Does not allow BTRFS_ORDERED_NOCOW type For real NOCOW (excluding NOCOW writes into preallocated ranges) writes, we never call create_io_em(), as we does not need to update the extent map at all. So remove the sanity check allowing BTRFS_ORDERED_NOCOW type. - Add extra sanity checks * PREALLOC - @block_len == len For uncompressed writes. * REGULAR - @block_len == @orig_block_len == @ram_bytes == @len We're creating a new uncompressed extent, and referring all of it. - @orig_start == @start We haven no offset inside the extent. * COMPRESSED - valid @compress_type - @len <= @ram_bytes This is to co-operate with encoded writes, which can cause a new file extent referring only part of a uncompressed extent. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/inode.c | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-)