* Fwd: [PATCH v3] bcache: use bio cloning for detached device requests [not found] <20260122013650.321766-1-zhangshida@kylinos.cn> @ 2026-01-22 1:40 ` Stephen Zhang 2026-01-22 2:08 ` Jens Axboe 2026-01-22 3:25 ` Coly Li 0 siblings, 2 replies; 5+ messages in thread From: Stephen Zhang @ 2026-01-22 1:40 UTC (permalink / raw) To: Coly Li, Kent Overstreet, Jens Axboe, Sasha Levin, Christoph Hellwig Cc: linux-bcache, Linux Kernel Mailing List, zhangshida ---------- Forwarded message --------- 发件人： zhangshida <starzhangzsd@gmail.com> Date: 2026年1月22日周四 09:37 Subject: [PATCH v3] bcache: use bio cloning for detached device requests To: <colyli@fnnas.com>, <kent.overstreet@linux.dev>, <axboe@kernel.dk>, <sashal@kernel.org>, <hch@infradead.org> Cc: <linux-bcache@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <zhangshida@kylinos.cn>, <starzhangzsd@gmail.com>, Christoph Hellwig <hch@lst.de> From: Shida Zhang <zhangshida@kylinos.cn> Previously, bcache hijacked the bi_end_io and bi_private fields of the incoming bio when the backing device was in a detached state. This is fragile and breaks if the bio is needed to be processed by other layers. This patch transitions to using a cloned bio embedded within a private structure. This ensures the original bio's metadata remains untouched. Fixes: 53280e398471 ("bcache: fix improper use of bi_end_io") Co-developed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Shida Zhang <zhangshida@kylinos.cn> --- Changelog: v2: - Renamed `bio_detach` to `bio_detached` https://lore.kernel.org/all/CANubcdXsWsdueYf_aN9FSm+hnE-rpXx_hHhwP9_Z1ni1YGEH9Q@mail.gmail.com/ v1: https://lore.kernel.org/all/20260115074811.230807-1-zhangshida@kylinos.cn/ drivers/md/bcache/bcache.h | 9 +++++ drivers/md/bcache/request.c | 79 ++++++++++++++++--------------------- drivers/md/bcache/super.c | 12 +++++- 3 files changed, 54 insertions(+), 46 deletions(-) diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h index 8ccacba8554..ec9ff971508 100644 --- a/drivers/md/bcache/bcache.h +++ b/drivers/md/bcache/bcache.h @@ -273,6 +273,8 @@ struct bcache_device { struct bio_set bio_split; + struct bio_set bio_detached; + unsigned int data_csum:1; int (*cache_miss)(struct btree *b, struct search *s, @@ -753,6 +755,13 @@ struct bbio { struct bio bio; }; +struct detached_dev_io_private { + struct bcache_device *d; + unsigned long start_time; + struct bio *orig_bio; + struct bio bio; +}; + #define BTREE_PRIO USHRT_MAX #define INITIAL_PRIO 32768U diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c index 82fdea7dea7..a02aecac05c 100644 --- a/drivers/md/bcache/request.c +++ b/drivers/md/bcache/request.c @@ -1077,68 +1077,58 @@ static CLOSURE_CALLBACK(cached_dev_nodata) continue_at(cl, cached_dev_bio_complete, NULL); } -struct detached_dev_io_private { - struct bcache_device *d; - unsigned long start_time; - bio_end_io_t *bi_end_io; - void *bi_private; - struct block_device *orig_bdev; -}; - static void detached_dev_end_io(struct bio *bio) { - struct detached_dev_io_private *ddip; - - ddip = bio->bi_private; - bio->bi_end_io = ddip->bi_end_io; - bio->bi_private = ddip->bi_private; + struct detached_dev_io_private *ddip = + container_of(bio, struct detached_dev_io_private, bio); + struct bio *orig_bio = ddip->orig_bio; /* Count on the bcache device */ - bio_end_io_acct_remapped(bio, ddip->start_time, ddip->orig_bdev); + bio_end_io_acct(orig_bio, ddip->start_time); if (bio->bi_status) { - struct cached_dev *dc = container_of(ddip->d, - struct cached_dev, disk); + struct cached_dev *dc = bio->bi_private; + /* should count I/O error for backing device here */ bch_count_backing_io_errors(dc, bio); + orig_bio->bi_status = bio->bi_status; } - kfree(ddip); - bio_endio(bio); + bio_put(bio); + bio_endio(orig_bio); } -static void detached_dev_do_request(struct bcache_device *d, struct bio *bio, - struct block_device *orig_bdev, unsigned long start_time) +static void detached_dev_do_request(struct bcache_device *d, + struct bio *orig_bio, unsigned long start_time) { struct detached_dev_io_private *ddip; struct cached_dev *dc = container_of(d, struct cached_dev, disk); + struct bio *clone_bio; - /* - * no need to call closure_get(&dc->disk.cl), - * because upper layer had already opened bcache device, - * which would call closure_get(&dc->disk.cl) - */ - ddip = kzalloc(sizeof(struct detached_dev_io_private), GFP_NOIO); - if (!ddip) { - bio->bi_status = BLK_STS_RESOURCE; - bio_endio(bio); + if (bio_op(orig_bio) == REQ_OP_DISCARD && + !bdev_max_discard_sectors(dc->bdev)) { + bio_endio(orig_bio); return; } - ddip->d = d; + clone_bio = bio_alloc_clone(dc->bdev, orig_bio, GFP_NOIO, + &d->bio_detached); + if (!clone_bio) { + orig_bio->bi_status = BLK_STS_RESOURCE; + bio_endio(orig_bio); + return; + } + + ddip = container_of(clone_bio, struct detached_dev_io_private, bio); /* Count on the bcache device */ - ddip->orig_bdev = orig_bdev; + ddip->d = d; ddip->start_time = start_time; - ddip->bi_end_io = bio->bi_end_io; - ddip->bi_private = bio->bi_private; - bio->bi_end_io = detached_dev_end_io; - bio->bi_private = ddip; - - if ((bio_op(bio) == REQ_OP_DISCARD) && - !bdev_max_discard_sectors(dc->bdev)) - detached_dev_end_io(bio); - else - submit_bio_noacct(bio); + ddip->orig_bio = orig_bio; + + clone_bio->bi_end_io = detached_dev_end_io; + clone_bio->bi_private = dc; + + submit_bio_noacct(clone_bio); } static void quit_max_writeback_rate(struct cache_set *c, @@ -1214,10 +1204,10 @@ void cached_dev_submit_bio(struct bio *bio) start_time = bio_start_io_acct(bio); - bio_set_dev(bio, dc->bdev); bio->bi_iter.bi_sector += dc->sb.data_offset; if (cached_dev_get(dc)) { + bio_set_dev(bio, dc->bdev); s = search_alloc(bio, d, orig_bdev, start_time); trace_bcache_request_start(s->d, bio); @@ -1237,9 +1227,10 @@ void cached_dev_submit_bio(struct bio *bio) else cached_dev_read(dc, s); } - } else + } else { /* I/O request sent to backing device */ - detached_dev_do_request(d, bio, orig_bdev, start_time); + detached_dev_do_request(d, bio, start_time); + } } static int cached_dev_ioctl(struct bcache_device *d, blk_mode_t mode, diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c index c17d4517af2..238d12ffdae 100644 --- a/drivers/md/bcache/super.c +++ b/drivers/md/bcache/super.c @@ -887,6 +887,7 @@ static void bcache_device_free(struct bcache_device *d) } bioset_exit(&d->bio_split); + bioset_exit(&d->bio_detached); kvfree(d->full_dirty_stripes); kvfree(d->stripe_sectors_dirty); @@ -949,6 +950,11 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size, BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER)) goto out_ida_remove; + if (bioset_init(&d->bio_detached, 4, + offsetof(struct detached_dev_io_private, bio), + BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER)) + goto out_bioset_split_exit; + if (lim.logical_block_size > PAGE_SIZE && cached_bdev) { /* * This should only happen with BCACHE_SB_VERSION_BDEV. @@ -964,7 +970,7 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size, d->disk = blk_alloc_disk(&lim, NUMA_NO_NODE); if (IS_ERR(d->disk)) - goto out_bioset_exit; + goto out_bioset_detach_exit; set_capacity(d->disk, sectors); snprintf(d->disk->disk_name, DISK_NAME_LEN, "bcache%i", idx); @@ -976,7 +982,9 @@ static int bcache_device_init(struct bcache_device *d, unsigned int block_size, d->disk->private_data = d; return 0; -out_bioset_exit: +out_bioset_detach_exit: + bioset_exit(&d->bio_detached); +out_bioset_split_exit: bioset_exit(&d->bio_split); out_ida_remove: ida_free(&bcache_device_idx, idx); -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: Fwd: [PATCH v3] bcache: use bio cloning for detached device requests 2026-01-22 1:40 ` Fwd: [PATCH v3] bcache: use bio cloning for detached device requests Stephen Zhang @ 2026-01-22 2:08 ` Jens Axboe 2026-01-22 2:20 ` Stephen Zhang 2026-01-22 3:25 ` Coly Li 1 sibling, 1 reply; 5+ messages in thread From: Jens Axboe @ 2026-01-22 2:08 UTC (permalink / raw) To: Stephen Zhang, Coly Li, Kent Overstreet, Sasha Levin, Christoph Hellwig Cc: linux-bcache, Linux Kernel Mailing List, zhangshida On 1/21/26 6:40 PM, Stephen Zhang wrote: > ---------- Forwarded message --------- Can you please just send it out as a regular patch? The forward has complete whitespace damage (it's all spaces, no tabs), and when you forward, b4 doesn't pick it up either. Maybe there's some reason for why you are doing these as forwards, if so do tell? -- Jens Axboe ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Fwd: [PATCH v3] bcache: use bio cloning for detached device requests 2026-01-22 2:08 ` Jens Axboe @ 2026-01-22 2:20 ` Stephen Zhang 2026-01-22 2:22 ` Jens Axboe 0 siblings, 1 reply; 5+ messages in thread From: Stephen Zhang @ 2026-01-22 2:20 UTC (permalink / raw) To: Jens Axboe Cc: Coly Li, Kent Overstreet, Sasha Levin, Christoph Hellwig, linux-bcache, Linux Kernel Mailing List, zhangshida Jens Axboe <axboe@kernel.dk> 于2026年1月22日周四 10:08写道： > > On 1/21/26 6:40 PM, Stephen Zhang wrote: > > ---------- Forwarded message --------- > > Can you please just send it out as a regular patch? The forward has > complete whitespace damage (it's all spaces, no tabs), and when you > forward, b4 doesn't pick it up either. Maybe there's some reason for > why you are doing these as forwards, if so do tell? > Apologies for the formatting issues with the forwarded patch. When I try using git send-email, I'm encountering server rejection issues. Interestingly, I found that if I cut the patch content in half, it goes through without rejection. This suggests there may be a size limit or specific content filter causing the problem. I'm investigating which part triggers the rejection and will resend the patch properly once resolved. Thank you for your patience, Shida > -- > Jens Axboe > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Fwd: [PATCH v3] bcache: use bio cloning for detached device requests 2026-01-22 2:20 ` Stephen Zhang @ 2026-01-22 2:22 ` Jens Axboe 0 siblings, 0 replies; 5+ messages in thread From: Jens Axboe @ 2026-01-22 2:22 UTC (permalink / raw) To: Stephen Zhang Cc: Coly Li, Kent Overstreet, Sasha Levin, Christoph Hellwig, linux-bcache, Linux Kernel Mailing List, zhangshida On 1/21/26 7:20 PM, Stephen Zhang wrote: > Jens Axboe <axboe@kernel.dk> 于2026年1月22日周四 10:08写道： >> >> On 1/21/26 6:40 PM, Stephen Zhang wrote: >>> ---------- Forwarded message --------- >> >> Can you please just send it out as a regular patch? The forward has >> complete whitespace damage (it's all spaces, no tabs), and when you >> forward, b4 doesn't pick it up either. Maybe there's some reason for >> why you are doing these as forwards, if so do tell? >> > > Apologies for the formatting issues with the forwarded patch. When I try > using git send-email, I'm encountering server rejection issues. > Interestingly, I found that if I cut the patch content in half, it goes > through without rejection. This suggests there may be a size limit or > specific content filter causing the problem. > > I'm investigating which part triggers the rejection and will resend the > patch properly once resolved. Can you put it in a git tree? Your original patches seem to have been applied by Coly just fine, so maybe just go that route again? -- Jens Axboe ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Fwd: [PATCH v3] bcache: use bio cloning for detached device requests 2026-01-22 1:40 ` Fwd: [PATCH v3] bcache: use bio cloning for detached device requests Stephen Zhang 2026-01-22 2:08 ` Jens Axboe @ 2026-01-22 3:25 ` Coly Li 1 sibling, 0 replies; 5+ messages in thread From: Coly Li @ 2026-01-22 3:25 UTC (permalink / raw) To: Stephen Zhang Cc: Kent Overstreet, Jens Axboe, Sasha Levin, Christoph Hellwig, linux-bcache, Linux Kernel Mailing List, zhangshida On Thu, Jan 22, 2026 at 09:40:32AM +0800, Stephen Zhang wrote: > ---------- Forwarded message --------- > 发件人： zhangshida <starzhangzsd@gmail.com> > Date: 2026年1月22日周四 09:37 > Subject: [PATCH v3] bcache: use bio cloning for detached device requests > To: <colyli@fnnas.com>, <kent.overstreet@linux.dev>, > <axboe@kernel.dk>, <sashal@kernel.org>, <hch@infradead.org> > Cc: <linux-bcache@vger.kernel.org>, <linux-kernel@vger.kernel.org>, > <zhangshida@kylinos.cn>, <starzhangzsd@gmail.com>, Christoph Hellwig > <hch@lst.de> > > > From: Shida Zhang <zhangshida@kylinos.cn> > > Previously, bcache hijacked the bi_end_io and bi_private fields of > the incoming bio when the backing device was in a detached state. > This is fragile and breaks if the bio is needed to be processed by > other layers. > > This patch transitions to using a cloned bio embedded within a private > structure. This ensures the original bio's metadata remains untouched. > > Fixes: 53280e398471 ("bcache: fix improper use of bi_end_io") > Co-developed-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Shida Zhang <zhangshida@kylinos.cn> Acked-by: Coly Li <colyli@fnnas.com> Thanks. Coly Li > --- > > > Changelog: > v2: > - Renamed `bio_detach` to `bio_detached` > https://lore.kernel.org/all/CANubcdXsWsdueYf_aN9FSm+hnE-rpXx_hHhwP9_Z1ni1YGEH9Q@mail.gmail.com/ > > v1: > https://lore.kernel.org/all/20260115074811.230807-1-zhangshida@kylinos.cn/ > > drivers/md/bcache/bcache.h | 9 +++++ > drivers/md/bcache/request.c | 79 ++++++++++++++++--------------------- > drivers/md/bcache/super.c | 12 +++++- > 3 files changed, 54 insertions(+), 46 deletions(-) > > diff --git a/drivers/md/bcache/bcache.h b/drivers/md/bcache/bcache.h > index 8ccacba8554..ec9ff971508 100644 > --- a/drivers/md/bcache/bcache.h > +++ b/drivers/md/bcache/bcache.h > @@ -273,6 +273,8 @@ struct bcache_device { > > struct bio_set bio_split; > > + struct bio_set bio_detached; > + > unsigned int data_csum:1; > > int (*cache_miss)(struct btree *b, struct search *s, > @@ -753,6 +755,13 @@ struct bbio { > struct bio bio; > }; > > +struct detached_dev_io_private { > + struct bcache_device *d; > + unsigned long start_time; > + struct bio *orig_bio; > + struct bio bio; > +}; > + > #define BTREE_PRIO USHRT_MAX > #define INITIAL_PRIO 32768U > > diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c > index 82fdea7dea7..a02aecac05c 100644 > --- a/drivers/md/bcache/request.c > +++ b/drivers/md/bcache/request.c > @@ -1077,68 +1077,58 @@ static CLOSURE_CALLBACK(cached_dev_nodata) > continue_at(cl, cached_dev_bio_complete, NULL); > } > > -struct detached_dev_io_private { > - struct bcache_device *d; > - unsigned long start_time; > - bio_end_io_t *bi_end_io; > - void *bi_private; > - struct block_device *orig_bdev; > -}; > - > static void detached_dev_end_io(struct bio *bio) > { > - struct detached_dev_io_private *ddip; > - > - ddip = bio->bi_private; > - bio->bi_end_io = ddip->bi_end_io; > - bio->bi_private = ddip->bi_private; > + struct detached_dev_io_private *ddip = > + container_of(bio, struct detached_dev_io_private, bio); > + struct bio *orig_bio = ddip->orig_bio; > > /* Count on the bcache device */ > - bio_end_io_acct_remapped(bio, ddip->start_time, ddip->orig_bdev); > + bio_end_io_acct(orig_bio, ddip->start_time); > > if (bio->bi_status) { > - struct cached_dev *dc = container_of(ddip->d, > - struct cached_dev, disk); > + struct cached_dev *dc = bio->bi_private; > + > /* should count I/O error for backing device here */ > bch_count_backing_io_errors(dc, bio); > + orig_bio->bi_status = bio->bi_status; > } > > - kfree(ddip); > - bio_endio(bio); > + bio_put(bio); > + bio_endio(orig_bio); > } > > -static void detached_dev_do_request(struct bcache_device *d, struct bio *bio, > - struct block_device *orig_bdev, unsigned long start_time) > +static void detached_dev_do_request(struct bcache_device *d, > + struct bio *orig_bio, unsigned long start_time) > { > struct detached_dev_io_private *ddip; > struct cached_dev *dc = container_of(d, struct cached_dev, disk); > + struct bio *clone_bio; > > - /* > - * no need to call closure_get(&dc->disk.cl), > - * because upper layer had already opened bcache device, > - * which would call closure_get(&dc->disk.cl) > - */ > - ddip = kzalloc(sizeof(struct detached_dev_io_private), GFP_NOIO); > - if (!ddip) { > - bio->bi_status = BLK_STS_RESOURCE; > - bio_endio(bio); > + if (bio_op(orig_bio) == REQ_OP_DISCARD && > + !bdev_max_discard_sectors(dc->bdev)) { > + bio_endio(orig_bio); > return; > } > > - ddip->d = d; > + clone_bio = bio_alloc_clone(dc->bdev, orig_bio, GFP_NOIO, > + &d->bio_detached); > + if (!clone_bio) { > + orig_bio->bi_status = BLK_STS_RESOURCE; > + bio_endio(orig_bio); > + return; > + } > + > + ddip = container_of(clone_bio, struct detached_dev_io_private, bio); > /* Count on the bcache device */ > - ddip->orig_bdev = orig_bdev; > + ddip->d = d; > ddip->start_time = start_time; > - ddip->bi_end_io = bio->bi_end_io; > - ddip->bi_private = bio->bi_private; > - bio->bi_end_io = detached_dev_end_io; > - bio->bi_private = ddip; > - > - if ((bio_op(bio) == REQ_OP_DISCARD) && > - !bdev_max_discard_sectors(dc->bdev)) > - detached_dev_end_io(bio); > - else > - submit_bio_noacct(bio); > + ddip->orig_bio = orig_bio; > + > + clone_bio->bi_end_io = detached_dev_end_io; > + clone_bio->bi_private = dc; > + > + submit_bio_noacct(clone_bio); > } > > static void quit_max_writeback_rate(struct cache_set *c, > @@ -1214,10 +1204,10 @@ void cached_dev_submit_bio(struct bio *bio) > > start_time = bio_start_io_acct(bio); > > - bio_set_dev(bio, dc->bdev); > bio->bi_iter.bi_sector += dc->sb.data_offset; > > if (cached_dev_get(dc)) { > + bio_set_dev(bio, dc->bdev); > s = search_alloc(bio, d, orig_bdev, start_time); > trace_bcache_request_start(s->d, bio); > > @@ -1237,9 +1227,10 @@ void cached_dev_submit_bio(struct bio *bio) > else > cached_dev_read(dc, s); > } > - } else > + } else { > /* I/O request sent to backing device */ > - detached_dev_do_request(d, bio, orig_bdev, start_time); > + detached_dev_do_request(d, bio, start_time); > + } > } > > static int cached_dev_ioctl(struct bcache_device *d, blk_mode_t mode, > diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c > index c17d4517af2..238d12ffdae 100644 > --- a/drivers/md/bcache/super.c > +++ b/drivers/md/bcache/super.c > @@ -887,6 +887,7 @@ static void bcache_device_free(struct bcache_device *d) > } > > bioset_exit(&d->bio_split); > + bioset_exit(&d->bio_detached); > kvfree(d->full_dirty_stripes); > kvfree(d->stripe_sectors_dirty); > > @@ -949,6 +950,11 @@ static int bcache_device_init(struct > bcache_device *d, unsigned int block_size, > BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER)) > goto out_ida_remove; > > + if (bioset_init(&d->bio_detached, 4, > + offsetof(struct detached_dev_io_private, bio), > + BIOSET_NEED_BVECS|BIOSET_NEED_RESCUER)) > + goto out_bioset_split_exit; > + > if (lim.logical_block_size > PAGE_SIZE && cached_bdev) { > /* > * This should only happen with BCACHE_SB_VERSION_BDEV. > @@ -964,7 +970,7 @@ static int bcache_device_init(struct bcache_device > *d, unsigned int block_size, > > d->disk = blk_alloc_disk(&lim, NUMA_NO_NODE); > if (IS_ERR(d->disk)) > - goto out_bioset_exit; > + goto out_bioset_detach_exit; > > set_capacity(d->disk, sectors); > snprintf(d->disk->disk_name, DISK_NAME_LEN, "bcache%i", idx); > @@ -976,7 +982,9 @@ static int bcache_device_init(struct bcache_device > *d, unsigned int block_size, > d->disk->private_data = d; > return 0; > > -out_bioset_exit: > +out_bioset_detach_exit: > + bioset_exit(&d->bio_detached); > +out_bioset_split_exit: > bioset_exit(&d->bio_split); > out_ida_remove: > ida_free(&bcache_device_idx, idx); > -- > 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-01-22 4:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260122013650.321766-1-zhangshida@kylinos.cn>
2026-01-22 1:40 ` Fwd: [PATCH v3] bcache: use bio cloning for detached device requests Stephen Zhang
2026-01-22 2:08 ` Jens Axboe
2026-01-22 2:20 ` Stephen Zhang
2026-01-22 2:22 ` Jens Axboe
2026-01-22 3:25 ` Coly Li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox