[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [dm-devel] clone_endio question (kernel 2.5/2.6)

On Thursday 24 July 2003 18:49, Christophe Saout wrote:
> Am Fr, 2003-07-25 um 01.40 schrieb Christophe Saout:
> > Is it possible that someone calls bio_endio with done < bi_size and an
> > error set? If it is so, the error is ignored by clone_endio.
> >
> > "if (bio->bi_size) return 1;" gets executed before the error parameter
> > gets checked.
> >
> > When clone_endio is executed, say, two times and the first time it get's
> > called with an error and the second time without, device-mapper calls
> > bio_endio on the original bio without noticing the error.
> I just checked: The raid1 code checks the BIO_UPTODATE flag in the bio
> and returns an error if it got cleared. It can get cleared by bio_endio
> when an error is passed.

The "if (bio->bi_size) return 1;" is the accepted convention for drivers that 
don't care about partial bio completions (MD and DM certainly don't care). 
IIRC, the original assumption is that if a driver sets an error when it 
completes one part of a bio, it will set an error on completions of the 
remaining parts. Now, if that assumption is no longer valid, then there could 
be a problem. But...if you look at raid1.c and multipath.c, they have this 
same check and return, so DM won't be the only thing that's broken.

As for the error that DM will return to the original submitter, if the above 
assumption is correct, the code in dec_pending() should handle returning the 
correct error code.

About the only thing that *could* be done differently would be: instead of 
dec_pending(io, error)
dec_pending(io, test_bit(BIO_UPTODATE, &bio->bi_flags) ? 0 : -EIO)

If the above assumption is incorrect, it looks like this change should fix it.

-Kevin Corry

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]