RE: [Drbd-dev] DRBD-8 - next crash in bio_split()

RE: [Drbd-dev] DRBD-8 - next crash in bio_split()

[Graham, Simon]

> Index: drbd_req.c
> ===================================================================
> --- drbd_req.c	(revision 2415)
> +++ drbd_req.c	(working copy)
> @@ -1068,7 +1068,11 @@
>  	unsigned int bio_size = bio->bi_size;
>  	int max;
> 
> +#if 1
> +	max = DRBD_MAX_SEGMENT_SIZE - ((bio_offset &
> (DRBD_MAX_SEGMENT_SIZE-1)) + bio_size);
> +#else
>  	max = AL_EXTENT_SIZE - ((bio_offset & (AL_EXTENT_SIZE-1)) +
> bio_size);
> +#endif
>  	if (max < 0) max = 0;
>  	if (max <= bvec->bv_len && bio_size == 0)
>  		return bvec->bv_len;

I'm not sure to be honest - I take it the intent here is to ensure there
can only ever be one bio_vec in a bio that needs to be split? And the
way to do that is to ensure that the only way a request that is too
large can be passed to DRBD is if it is the first entry that is too
large?

I'll try this and let you know...

Re: [Drbd-dev] DRBD-8 - next crash in bio_split()

[Lars Ellenberg]

/ 2006-09-13 16:41:25 -0400
\ Graham, Simon:
> > Index: drbd_req.c
> > ===================================================================
> > --- drbd_req.c	(revision 2415)
> > +++ drbd_req.c	(working copy)
> > @@ -1068,7 +1068,11 @@
> >  	unsigned int bio_size = bio->bi_size;
> >  	int max;
> > 
> > +#if 1
> > +	max = DRBD_MAX_SEGMENT_SIZE - ((bio_offset &
> > (DRBD_MAX_SEGMENT_SIZE-1)) + bio_size);
> > +#else
> >  	max = AL_EXTENT_SIZE - ((bio_offset & (AL_EXTENT_SIZE-1)) +
> > bio_size);
> > +#endif
> >  	if (max < 0) max = 0;
> >  	if (max <= bvec->bv_len && bio_size == 0)
> >  		return bvec->bv_len;
> 
> I'm not sure to be honest - I take it the intent here is to ensure there
> can only ever be one bio_vec in a bio that needs to be split? And the
> way to do that is to ensure that the only way a request that is too
> large can be passed to DRBD is if it is the first entry that is too
> large?

basically, yes.  but the "first_sectors" calculation for the bio_split
was terribly wrong, too.
I just committed the correct fix (I think).
its just been my own "stupidity" (again).

please verify. at least for me, it just copied the complete linux-2.6.17
source tree several times without even triggering an assert.
before that fix, it would just blow up in the very first seconds...

cheers, and good night

-- 
: Lars Ellenberg                                  Tel +43-1-8178292-55 :
: LINBIT Information Technologies GmbH            Fax +43-1-8178292-82 :
: Schoenbrunner Str. 244, A-1120 Vienna/Europe   http://www.linbit.com :

[Drbd-dev] DRBD-8 - next crash in bio_split()

[Graham, Simon]

Yesterday's checkins have allowed me to get past the booting stage and
I've made some good progress - however, I've started to notice system
crashes when I failover use of one of the drbd devices from one node to
the other - always inside bio_split() at an assert that there is a
single bio_vec in the array (bio->bi_vcnt==1). 

I see that there has been some discussion of this previously
(http://lists.linbit.com/pipermail/drbd-dev/2005-March/000272.html) so
we are all aware that bio_split can only handle a bio with a single
bio_vec -- part of the mega-reorg of request processing was this change:

#if 1
	/* to make some things easier, force allignment of requests
within the
	 * granularity of our hash tables */
	s_enr = bio->bi_sector >> HT_SHIFT;
	e_enr = (bio->bi_sector+(bio->bi_size>>9)-1) >> HT_SHIFT;
#else
	/* when not using two primaries (and not being as paranoid as
lge),
	 * actually there is no need to be as strict.
	 * only force allignment within AL_EXTENT boundaries */
	s_enr = bio->bi_sector >> (AL_EXTENT_SIZE_B-9);
	e_enr = (bio->bi_sector+(bio->bi_size>>9)-1) >>
(AL_EXTENT_SIZE_B-9);
#endif

Since HT_SHIFT is 9 and AL_EXTENT_SIZE_B-9 is 13, this means we are far
more likely to decide to split now and maybe it is happening where the
bi_io_vec array has more than one entry...

Simon

Re: [Drbd-dev] DRBD-8 - next crash in bio_split()

[Lars Ellenberg]

/ 2006-09-13 14:15:19 -0400
\ Graham, Simon:
> Yesterday's checkins have allowed me to get past the booting stage and
> I've made some good progress - however, I've started to notice system
> crashes when I failover use of one of the drbd devices from one node to
> the other - always inside bio_split() at an assert that there is a
> single bio_vec in the array (bio->bi_vcnt==1). 
> 
> I see that there has been some discussion of this previously
> (http://lists.linbit.com/pipermail/drbd-dev/2005-March/000272.html) so
> we are all aware that bio_split can only handle a bio with a single
> bio_vec -- part of the mega-reorg of request processing was this change:
> 
> #if 1
> 	/* to make some things easier, force allignment of requests
> within the
> 	 * granularity of our hash tables */
> 	s_enr = bio->bi_sector >> HT_SHIFT;
> 	e_enr = (bio->bi_sector+(bio->bi_size>>9)-1) >> HT_SHIFT;
> #else
> 	/* when not using two primaries (and not being as paranoid as
> lge),
> 	 * actually there is no need to be as strict.
> 	 * only force allignment within AL_EXTENT boundaries */
> 	s_enr = bio->bi_sector >> (AL_EXTENT_SIZE_B-9);
> 	e_enr = (bio->bi_sector+(bio->bi_size>>9)-1) >>
> (AL_EXTENT_SIZE_B-9);
> #endif
> 
> Since HT_SHIFT is 9 and AL_EXTENT_SIZE_B-9 is 13, this means we are far
> more likely to decide to split now and maybe it is happening where the
> bi_io_vec array has more than one entry...

would that do the trick?
both sections should probably be run-time conditional on
"two_primaries", but they have to be consistent, at least...

Index: drbd_req.c
===================================================================
--- drbd_req.c	(revision 2415)
+++ drbd_req.c	(working copy)
@@ -1068,7 +1068,11 @@
 	unsigned int bio_size = bio->bi_size;
 	int max;
 
+#if 1
+	max = DRBD_MAX_SEGMENT_SIZE - ((bio_offset & (DRBD_MAX_SEGMENT_SIZE-1)) + bio_size);
+#else
 	max = AL_EXTENT_SIZE - ((bio_offset & (AL_EXTENT_SIZE-1)) + bio_size);
+#endif
 	if (max < 0) max = 0;
 	if (max <= bvec->bv_len && bio_size == 0)
 		return bvec->bv_len;


-- 
: Lars Ellenberg                                  Tel +43-1-8178292-55 :
: LINBIT Information Technologies GmbH            Fax +43-1-8178292-82 :
: Schoenbrunner Str. 244, A-1120 Vienna/Europe   http://www.linbit.com :