Small bug in bio_clone?

Small bug in bio_clone?

[Russell Cattelan]

[-- Attachment #1: Type: text/plain, Size: 615 bytes --]

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);


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Small bug in bio_clone?

[Jens Axboe]

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