Re: [PATCH] fixes for linked list bugs in block I/O code

[Bartlomiej Zolnierkiewicz]

On Wed, 7 May 2003, Dave Peterson wrote:

> On Wednesday 07 May 2003 04:42 pm, Bartlomiej Zolnierkiewicz wrote:
> > > ========== START OF 2.5.69 PATCH FOR drivers/block/ll_rw_blk.c
> > > =========== --- ll_rw_blk.c.old     Wed May  7 15:55:18 2003
> > > +++ ll_rw_blk.c.new     Wed May  7 16:01:56 2003
> > > @@ -1721,6 +1721,7 @@
> > >                                 break;
> > >                         }
> > >
> > > +                       bio->bi_next = req->biotail->bi_next;
> >
> > This is simply wrong, look at the line below.
> >
> > >                         req->biotail->bi_next = bio;
> >
> > req->bio - first bio
> > req->bio->bi_next - next bio
> > ...
> > req->biotail - last bio
> >
> > so req->biotail->bi_next should be NULL
>
> I believe it is correct.  Assuming that the list is initially in a
> sane state, req->biotail->bi_next will be NULL immediately before
> executing the statement that I added.  Therefore, my fix will set
> bio->bi_next to NULL, which is what we want because bio becomes the
> new end of the list.

Yes, but bio->bi_next is a NULL already.

> > >                         req->biotail = bio;
> > >                         req->nr_sectors = req->hard_nr_sectors +=
> > > nr_sectors; @@ -1811,6 +1812,7 @@
> > >         req->buffer = bio_data(bio);    /* see ->buffer comment above */
> > >         req->waiting = NULL;
> > >         req->bio = req->biotail = bio;
> > > +       bio->bi_next = NULL;
> >
> > No need for that, look at bio_init() in fs/bio.c.
>
> Yes, it looks like bio_init has been added in the 2.5 kernels, solving
> the problem.  However, this is still a bug in 2.4.20.

Maybe. If so only second part of the patch is important.

Regards,
--
Bartlomiej