2004-03-19 22:11:13

by Russell Cattelan

[permalink] [raw]

Subject: Small bug in bio_clone?

---

Shouldn't __bio_clone be checking the state flags
of the src bio?

--- /usr/tmp/TmpDir.29150-0/fs/bio.c_1.3 2004-03-19
16:07:12.000000000 -0600
+++ fs/bio.c 2004-03-19 16:06:24.348491070 -0600
@@ -225,7 +225,7 @@
*/
bio->bi_vcnt = bio_src->bi_vcnt;
bio->bi_idx = bio_src->bi_idx;
- if (bio_flagged(bio, BIO_SEG_VALID)) {
+ if (bio_flagged(bio_src, BIO_SEG_VALID)) {
bio->bi_phys_segments = bio_src->bi_phys_segments;
bio->bi_hw_segments = bio_src->bi_hw_segments;
bio->bi_flags |= (1 << BIO_SEG_VALID);

---

Attachments:

signature.asc (189.00 B)
This is a digitally signed message part

2004-03-20 10:18:06

by Jens Axboe

[permalink] [raw]

Subject: Re: Small bug in bio_clone?

---

On Fri, Mar 19 2004, Russell Cattelan wrote:
> Shouldn't __bio_clone be checking the state flags
> of the src bio?
>
> --- /usr/tmp/TmpDir.29150-0/fs/bio.c_1.3 2004-03-19
> 16:07:12.000000000 -0600
> +++ fs/bio.c 2004-03-19 16:06:24.348491070 -0600
> @@ -225,7 +225,7 @@
> */
> bio->bi_vcnt = bio_src->bi_vcnt;
> bio->bi_idx = bio_src->bi_idx;
> - if (bio_flagged(bio, BIO_SEG_VALID)) {
> + if (bio_flagged(bio_src, BIO_SEG_VALID)) {
> bio->bi_phys_segments = bio_src->bi_phys_segments;
> bio->bi_hw_segments = bio_src->bi_hw_segments;
> bio->bi_flags |= (1 << BIO_SEG_VALID);

Yes, in theory. What is done now is for sure a mistake, however I'm
thinking it's probably safer to just delete the check and setting of
segments, and do it on-demand the next time (if ever) someone calls
bio_*_segments(bio). Hmm, should be done every time someone assigns
->bi_bdev(), maybe it would be a good idea to add something like that.

static inline void bio_set_bdev(struct bio *bio, struct block_device *bdev)
{
bio->bi_bdev = bdev;
bio->bi_flags &= ~(1 << BIO_SEG_VALID);
}

--
Jens Axboe