| Message ID | 20230724142243.5742-6-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] btrfs: fix error handling when in a COW window in run_delalloc_nocow | expand |
On Mon, Jul 24, 2023 at 07:22:42AM -0700, Christoph Hellwig wrote: > When run_delalloc_cow allocates an ordered extent for an actual > NOCOW range, it uses the nocow_end variable calculated based on > the current offset and the nocow_args.num_bytes value returned > from can_nocow_file_extent for all the actual I/O, but the loop > iteration then resets cur_offset to extent_end, which caused > me a lot of confusion. It turns out that nocow_end is based > of the minimum of the extent end and the range end, and thus > actually works perfectly fine for the loop iteration, but > using a different variable here from the actual I/O submission > is horribly confusing and wasted some of my precious brain > cells when train to understand the logic. I'm editing out such personal notes from changelogs or rephrase them to be relevant to the code regarding readability. In the long term nobody cares how a developer felt while understanding or writing the code, we always know better in the hindsight.
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 92182e0d27fdb5..caaf2c002d795d 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -2187,7 +2187,7 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode, EXTENT_CLEAR_DATA_RESV, PAGE_UNLOCK | PAGE_SET_ORDERED); - cur_offset = extent_end; + cur_offset = nocow_end + 1; /* * btrfs_reloc_clone_csums() error, now we're OK to call error
When run_delalloc_cow allocates an ordered extent for an actual NOCOW range, it uses the nocow_end variable calculated based on the current offset and the nocow_args.num_bytes value returned from can_nocow_file_extent for all the actual I/O, but the loop iteration then resets cur_offset to extent_end, which caused me a lot of confusion. It turns out that nocow_end is based of the minimum of the extent end and the range end, and thus actually works perfectly fine for the loop iteration, but using a different variable here from the actual I/O submission is horribly confusing and wasted some of my precious brain cells when train to understand the logic. Switch to using nocow_end adjusted by the the off by one to make it an exclusive range. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/btrfs/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)