doubt in chunk_aligned_read()

doubt in chunk_aligned_read()

[Anuj Goel]

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

--
Best Regards,
Anuj Goel
Experimental Computer Science Lab
Stony Brook University.
Cell: +1-801-209-5873

Re: doubt in chunk_aligned_read()

[NeilBrown]

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

On Wed, 4 Apr 2012 21:25:53 -0400 Anuj Goel <talk2anuj@gmail.com> 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
> Experimental Computer Science Lab
> Stony Brook University.
> Cell: +1-801-209-5873
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: doubt in chunk_aligned_read()

[Anuj Goel]

cool :)
Thanks !!

On Wed, Apr 4, 2012 at 9:38 PM, NeilBrown <neilb@suse.de> wrote:
> On Wed, 4 Apr 2012 21:25:53 -0400 Anuj Goel <talk2anuj@gmail.com> 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
>> Experimental Computer Science Lab
>> Stony Brook University.
>> Cell: +1-801-209-5873
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Best Regards,
Anuj Goel
Experimental Computer Science Lab
Stony Brook University.
Cell: +1-801-209-5873
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: doubt in chunk_aligned_read()

[Anuj Goel]

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 ?

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 ?

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

On Wed, Apr 4, 2012 at 9:38 PM, NeilBrown <neilb@suse.de> wrote:
> On Wed, 4 Apr 2012 21:25:53 -0400 Anuj Goel <talk2anuj@gmail.com> 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@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Best Regards,
Anuj Goel
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: doubt in chunk_aligned_read()

[NeilBrown]

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

On Sun, 8 Apr 2012 15:47:01 -0400 Anuj Goel <talk2anuj@gmail.com> 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@suse.de> wrote:
> > On Wed, 4 Apr 2012 21:25:53 -0400 Anuj Goel <talk2anuj@gmail.com> 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@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 
> 
> 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]