| Message ID | 20230324023207.544800-6-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/11] btrfs: add function to create and return an ordered extent | expand |
On Fri, Mar 24, 2023 at 10:32:01AM +0800, Christoph Hellwig wrote: > btrfs_extract_ordered_extent must always be called for the beginning of > an ordered_extent. Add an assert to check for that and simplify the > calculation of the split ranges for btrfs_split_ordered_extent and > split_zoned_em. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/btrfs/inode.c | 29 +++++++++++++---------------- > 1 file changed, 13 insertions(+), 16 deletions(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 4f2f1aafd1720e..2cbc6c316effc1 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -2632,39 +2632,36 @@ blk_status_t btrfs_extract_ordered_extent(struct btrfs_bio *bbio) > u64 len = bbio->bio.bi_iter.bi_size; > struct btrfs_inode *inode = bbio->inode; > struct btrfs_ordered_extent *ordered; > - u64 file_len; > - u64 end = start + len; > - u64 ordered_end; > - u64 pre, post; > + u64 ordered_len; > int ret = 0; > > ordered = btrfs_lookup_ordered_extent(inode, bbio->file_offset); > if (WARN_ON_ONCE(!ordered)) > return BLK_STS_IOERR; > + ordered_len = ordered->num_bytes; > > - /* No need to split */ > - if (ordered->disk_num_bytes == len) > + /* Must always be called for the beginning of an ordered extent. */ > + if (WARN_ON_ONCE(start != ordered->disk_bytenr)) { > + ret = -EINVAL; > goto out; > + } > > - ordered_end = ordered->disk_bytenr + ordered->disk_num_bytes; > - /* bio must be in one ordered extent */ > - if (WARN_ON_ONCE(start < ordered->disk_bytenr || end > ordered_end)) { > + /* The bio must be entirely covered by the ordered extent */ > + if (WARN_ON_ONCE(len > ordered_len)) { > ret = -EINVAL; > goto out; > } > > - file_len = ordered->num_bytes; > - pre = start - ordered->disk_bytenr; > - post = ordered_end - end; > + /* No need to split if the ordered extent covers the entire bio */ > + if (ordered->disk_num_bytes == len) > + goto out; I thought this can go up to catch the non-split case early, but the above check will be dropped in the following patch, so it's OK. > - ret = btrfs_split_ordered_extent(ordered, pre, post); > + ret = btrfs_split_ordered_extent(ordered, len, 0); The next patch will overwrite this anyway, but the pre and post are the length of new ordered extents which are not covered by the bio. So, giving them (len, 0) breaks the semantics so that a new ordered extent is allocated for the bio range. They should be (0, ordered_len - len). > if (ret) > goto out; > - ret = split_zoned_em(inode, bbio->file_offset, file_len, pre, post); > - > + ret = split_zoned_em(inode, bbio->file_offset, ordered_len, len, 0); Same here. But, it is harmless unless a bisecting will fall into these commits. > out: > btrfs_put_ordered_extent(ordered); > - > return errno_to_blk_status(ret); > } > > -- > 2.39.2 >
On Fri, Mar 24, 2023 at 06:07:26AM +0000, Naohiro Aota wrote: > > - ret = btrfs_split_ordered_extent(ordered, pre, post); > > + ret = btrfs_split_ordered_extent(ordered, len, 0); > > The next patch will overwrite this anyway, but the pre and post are the > length of new ordered extents which are not covered by the bio. So, giving > them (len, 0) breaks the semantics so that a new ordered extent is > allocated for the bio range. They should be (0, ordered_len - len). Hmm, nothing trips up in my ZNS tests even at this bisection point, and I don't think it should matter at this stage what part is split off. Later we rely on the front being split off and a new ordered_extent being allocated for the front range covering the bio. So maybe this just need to be documented in the commit log?
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 4f2f1aafd1720e..2cbc6c316effc1 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -2632,39 +2632,36 @@ blk_status_t btrfs_extract_ordered_extent(struct btrfs_bio *bbio) u64 len = bbio->bio.bi_iter.bi_size; struct btrfs_inode *inode = bbio->inode; struct btrfs_ordered_extent *ordered; - u64 file_len; - u64 end = start + len; - u64 ordered_end; - u64 pre, post; + u64 ordered_len; int ret = 0; ordered = btrfs_lookup_ordered_extent(inode, bbio->file_offset); if (WARN_ON_ONCE(!ordered)) return BLK_STS_IOERR; + ordered_len = ordered->num_bytes; - /* No need to split */ - if (ordered->disk_num_bytes == len) + /* Must always be called for the beginning of an ordered extent. */ + if (WARN_ON_ONCE(start != ordered->disk_bytenr)) { + ret = -EINVAL; goto out; + } - ordered_end = ordered->disk_bytenr + ordered->disk_num_bytes; - /* bio must be in one ordered extent */ - if (WARN_ON_ONCE(start < ordered->disk_bytenr || end > ordered_end)) { + /* The bio must be entirely covered by the ordered extent */ + if (WARN_ON_ONCE(len > ordered_len)) { ret = -EINVAL; goto out; } - file_len = ordered->num_bytes; - pre = start - ordered->disk_bytenr; - post = ordered_end - end; + /* No need to split if the ordered extent covers the entire bio */ + if (ordered->disk_num_bytes == len) + goto out; - ret = btrfs_split_ordered_extent(ordered, pre, post); + ret = btrfs_split_ordered_extent(ordered, len, 0); if (ret) goto out; - ret = split_zoned_em(inode, bbio->file_offset, file_len, pre, post); - + ret = split_zoned_em(inode, bbio->file_offset, ordered_len, len, 0); out: btrfs_put_ordered_extent(ordered); - return errno_to_blk_status(ret); }
btrfs_extract_ordered_extent must always be called for the beginning of an ordered_extent. Add an assert to check for that and simplify the calculation of the split ranges for btrfs_split_ordered_extent and split_zoned_em. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/btrfs/inode.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-)