[PATCH] btrfs: simplify async csum synchronization

Daniel Vacek posted 1 patch 2 months, 3 weeks ago
There is a newer version of this series
fs/btrfs/bio.c       | 2 +-
fs/btrfs/bio.h       | 1 -
fs/btrfs/file-item.c | 6 +++---
3 files changed, 4 insertions(+), 5 deletions(-)
[PATCH] btrfs: simplify async csum synchronization
Posted by Daniel Vacek 2 months, 3 weeks ago
We don't need the completion csum_done which marks the csum work
has been executed. We can simply flush_work() instead.

This way we can slim down the btrfs_bio structure by 32 bytes matching
it's size to what it used to be before introducing the async csums.
Hence not making any change with respect to the structure size.
---
This is a simple fixup for "btrfs: introduce btrfs_bio::async_csum" in
for-next and can be squashed into it.
---
 fs/btrfs/bio.c       | 2 +-
 fs/btrfs/bio.h       | 1 -
 fs/btrfs/file-item.c | 6 +++---
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index a73652b8724a..fd6e4278a62f 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -106,7 +106,7 @@ void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status)
 	ASSERT(in_task());
 
 	if (bbio->async_csum)
-		wait_for_completion(&bbio->csum_done);
+		flush_work(&bbio->csum_work);
 
 	bbio->bio.bi_status = status;
 	if (bbio->bio.bi_pool == &btrfs_clone_bioset) {
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index deaeea3becf4..0b09d9122fa2 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -57,7 +57,6 @@ struct btrfs_bio {
 			struct btrfs_ordered_extent *ordered;
 			struct btrfs_ordered_sum *sums;
 			struct work_struct csum_work;
-			struct completion csum_done;
 			struct bvec_iter csum_saved_iter;
 			u64 orig_physical;
 		};
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 72be3ede0edf..3e9241f360c8 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -792,7 +792,6 @@ static void csum_one_bio_work(struct work_struct *work)
 	ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
 	ASSERT(bbio->async_csum == true);
 	csum_one_bio(bbio, &bbio->csum_saved_iter);
-	complete(&bbio->csum_done);
 }
 
 /*
@@ -805,6 +804,7 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	struct bio *bio = &bbio->bio;
 	struct btrfs_ordered_sum *sums;
+	struct workqueue_struct *wq;
 	unsigned nofs_flag;
 
 	nofs_flag = memalloc_nofs_save();
@@ -825,11 +825,11 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
 		csum_one_bio(bbio, &bbio->bio.bi_iter);
 		return 0;
 	}
-	init_completion(&bbio->csum_done);
 	bbio->async_csum = true;
 	bbio->csum_saved_iter = bbio->bio.bi_iter;
 	INIT_WORK(&bbio->csum_work, csum_one_bio_work);
-	schedule_work(&bbio->csum_work);
+	wq = bio->bi_opf & REQ_META? fs_info->endio_meta_workers: fs_info->endio_workers;
+	queue_work(wq, &bbio->csum_work);
 	return 0;
 }
 
-- 
2.43.0
Re: [PATCH] btrfs: simplify async csum synchronization
Posted by Qu Wenruo 2 months, 3 weeks ago

在 2025/11/13 20:01, Daniel Vacek 写道:
> We don't need the completion csum_done which marks the csum work
> has been executed. We can simply flush_work() instead.
> 
> This way we can slim down the btrfs_bio structure by 32 bytes matching
> it's size to what it used to be before introducing the async csums.
> Hence not making any change with respect to the structure size.
> ---
> This is a simple fixup for "btrfs: introduce btrfs_bio::async_csum" in
> for-next and can be squashed into it.
> ---
>   fs/btrfs/bio.c       | 2 +-
>   fs/btrfs/bio.h       | 1 -
>   fs/btrfs/file-item.c | 6 +++---
>   3 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index a73652b8724a..fd6e4278a62f 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -106,7 +106,7 @@ void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status)
>   	ASSERT(in_task());
>   
>   	if (bbio->async_csum)
> -		wait_for_completion(&bbio->csum_done);
> +		flush_work(&bbio->csum_work);
>   
>   	bbio->bio.bi_status = status;
>   	if (bbio->bio.bi_pool == &btrfs_clone_bioset) {
> diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> index deaeea3becf4..0b09d9122fa2 100644
> --- a/fs/btrfs/bio.h
> +++ b/fs/btrfs/bio.h
> @@ -57,7 +57,6 @@ struct btrfs_bio {
>   			struct btrfs_ordered_extent *ordered;
>   			struct btrfs_ordered_sum *sums;
>   			struct work_struct csum_work;
> -			struct completion csum_done;
>   			struct bvec_iter csum_saved_iter;
>   			u64 orig_physical;
>   		};
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 72be3ede0edf..3e9241f360c8 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -792,7 +792,6 @@ static void csum_one_bio_work(struct work_struct *work)
>   	ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
>   	ASSERT(bbio->async_csum == true);
>   	csum_one_bio(bbio, &bbio->csum_saved_iter);
> -	complete(&bbio->csum_done);
>   }
>   
>   /*
> @@ -805,6 +804,7 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
>   	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>   	struct bio *bio = &bbio->bio;
>   	struct btrfs_ordered_sum *sums;
> +	struct workqueue_struct *wq;
>   	unsigned nofs_flag;
>   
>   	nofs_flag = memalloc_nofs_save();
> @@ -825,11 +825,11 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
>   		csum_one_bio(bbio, &bbio->bio.bi_iter);
>   		return 0;
>   	}
> -	init_completion(&bbio->csum_done);
>   	bbio->async_csum = true;
>   	bbio->csum_saved_iter = bbio->bio.bi_iter;
>   	INIT_WORK(&bbio->csum_work, csum_one_bio_work);
> -	schedule_work(&bbio->csum_work);
> +	wq = bio->bi_opf & REQ_META? fs_info->endio_meta_workers: fs_info->endio_workers;

Metadata will not go into btrfs_csum_one_bio(), thus it's fixed to 
endio_workers().


> +	queue_work(wq, &bbio->csum_work);
>   	return 0;
>   }
>   
Re: [PATCH] btrfs: simplify async csum synchronization
Posted by Daniel Vacek 2 months, 3 weeks ago
On Thu, 13 Nov 2025 at 10:44, Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> 在 2025/11/13 20:01, Daniel Vacek 写道:
> > We don't need the completion csum_done which marks the csum work
> > has been executed. We can simply flush_work() instead.
> >
> > This way we can slim down the btrfs_bio structure by 32 bytes matching
> > it's size to what it used to be before introducing the async csums.
> > Hence not making any change with respect to the structure size.
> > ---
> > This is a simple fixup for "btrfs: introduce btrfs_bio::async_csum" in
> > for-next and can be squashed into it.
> > ---
> >   fs/btrfs/bio.c       | 2 +-
> >   fs/btrfs/bio.h       | 1 -
> >   fs/btrfs/file-item.c | 6 +++---
> >   3 files changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> > index a73652b8724a..fd6e4278a62f 100644
> > --- a/fs/btrfs/bio.c
> > +++ b/fs/btrfs/bio.c
> > @@ -106,7 +106,7 @@ void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status)
> >       ASSERT(in_task());
> >
> >       if (bbio->async_csum)
> > -             wait_for_completion(&bbio->csum_done);
> > +             flush_work(&bbio->csum_work);
> >
> >       bbio->bio.bi_status = status;
> >       if (bbio->bio.bi_pool == &btrfs_clone_bioset) {
> > diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
> > index deaeea3becf4..0b09d9122fa2 100644
> > --- a/fs/btrfs/bio.h
> > +++ b/fs/btrfs/bio.h
> > @@ -57,7 +57,6 @@ struct btrfs_bio {
> >                       struct btrfs_ordered_extent *ordered;
> >                       struct btrfs_ordered_sum *sums;
> >                       struct work_struct csum_work;
> > -                     struct completion csum_done;
> >                       struct bvec_iter csum_saved_iter;
> >                       u64 orig_physical;
> >               };
> > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> > index 72be3ede0edf..3e9241f360c8 100644
> > --- a/fs/btrfs/file-item.c
> > +++ b/fs/btrfs/file-item.c
> > @@ -792,7 +792,6 @@ static void csum_one_bio_work(struct work_struct *work)
> >       ASSERT(btrfs_op(&bbio->bio) == BTRFS_MAP_WRITE);
> >       ASSERT(bbio->async_csum == true);
> >       csum_one_bio(bbio, &bbio->csum_saved_iter);
> > -     complete(&bbio->csum_done);
> >   }
> >
> >   /*
> > @@ -805,6 +804,7 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
> >       struct btrfs_fs_info *fs_info = inode->root->fs_info;
> >       struct bio *bio = &bbio->bio;
> >       struct btrfs_ordered_sum *sums;
> > +     struct workqueue_struct *wq;
> >       unsigned nofs_flag;
> >
> >       nofs_flag = memalloc_nofs_save();
> > @@ -825,11 +825,11 @@ int btrfs_csum_one_bio(struct btrfs_bio *bbio, bool async)
> >               csum_one_bio(bbio, &bbio->bio.bi_iter);
> >               return 0;
> >       }
> > -     init_completion(&bbio->csum_done);
> >       bbio->async_csum = true;
> >       bbio->csum_saved_iter = bbio->bio.bi_iter;
> >       INIT_WORK(&bbio->csum_work, csum_one_bio_work);
> > -     schedule_work(&bbio->csum_work);
> > +     wq = bio->bi_opf & REQ_META? fs_info->endio_meta_workers: fs_info->endio_workers;
>
> Metadata will not go into btrfs_csum_one_bio(), thus it's fixed to
> endio_workers().

I suspected, but I was not sure so rather be safe than sorry.
That simplifies it quite nicely. Thanks.

> > +     queue_work(wq, &bbio->csum_work);
> >       return 0;
> >   }
> >
>