Re: doubt in chunk_aligned_read()

[NeilBrown <neilb@xxxxxxx>]

On Sun, 8 Apr 2012 15:47:01 -0400 Anuj Goel <talk2anuj@xxxxxxxxx> wrote:

> Thanks for the hint !!
> 
> I have few more doubts in the function,
> 
> static int chunk_aligned_read(mddev_t *mddev, struct bio * raid_bio)
> 
> if all the preliminary checks pass, we increment active_aligned_reads
> and call generic_make_request() which will add this bio to the
> "current->bio_list".
> This being an asynchronous read, chunk_aligned_read returns and the
> result of the read will be notified by the registered callback
> function "raid5_align_endio".
> Based on this understanding, I have some questions below:
> 
> 1. How and when is the read from the disk initiated ?

When the original q->make_request_fn call into raid5 returns,
generic_make_request will notice the request on current->bio_list
(bio_list_pop will pop it off) and its ->make_request_fn will be called.


> 
> 2. The bio passed to raid5_align_endio() is the one which was cloned
> in chunk_aligned read(). The below statement in raid5_align_endio()
> extracts the original bio :
> struct bio* raid_bi  = bi->bi_private;
> My doubt is in the assignment,
> rdev = (void*)raid_bi->bi_next;
> raid_bi is the original bio and its bi_next field should have been
> pointing to the next bio in the request queue "current->bio_list"  (if
> active_aligned_reads is > 0 ). How can this point to a rdev structure
> ? Or better put,  where do we make the bio->bi_next point to a rdev
> struct ?

When ->make_request_fn is called, bi_next must be NULL.  It will no longer be
on the current->bio_list (it was popped off if it was ever on).

See:
		raid_bio->bi_next = (void*)rdev;
in chunk_aligned_read.  Note that raid_bio doesn't get added to
current->bio_list, align_bi does.  So the rdev on raid_bi->bi_next is safe.


> 
> 3. after this, we have,
>     i) mddev = rdev->mddev;
>     ii) conf = mddev->private;
>     iii) rdev_dec_pending(rdev, conf->mddev);
> In the call in line iii) we could have just sent the mddev retrieved
> in line i). I am curious if there is a specific reason for reading it
> again from the raid configuration struct "conf" ? Could there be a
> case when mddev in line i) can be different from the conf->mddev in
> line iii)  ??

No.  Just clumsy coding.
iii should be
       rdev_dec_pending(rdev, mddev);

NeilBrown


> 
> On Wed, Apr 4, 2012 at 9:38 PM, NeilBrown <neilb@xxxxxxx> wrote:
> > On Wed, 4 Apr 2012 21:25:53 -0400 Anuj Goel <talk2anuj@xxxxxxxxx> wrote:
> >
> >> HI Guys,
> >>
> >> Why is the below call needed in the function  chunk_aligned_read()
> >>
> >>       align_bi = bio_clone_mddev(raid_bio, GFP_NOIO, mddev);
> >>
> >> Why do we need to create a clone of the bio passed to
> >> chunk_aligned_read(mddev_t *mddev, struct bio * raid_bio) ??
> >
> > Think about what happens if the read fails.
> >
> > NeilBrown
> >
> >
> >>
> >> --
> >> Best Regards,
> >> Anuj Goel
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> >> the body of a message to majordomo@xxxxxxxxxxxxxxx
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 
> 
>

Attachment: signature.asc
Description: PGP signature