RE: [patch] direct-io: bug fix in dio handling write error
From: Badari Pulavarty
Date: Tue Mar 21 2006 - 15:53:22 EST
On Tue, 2006-03-21 at 11:03 -0800, Chen, Kenneth W wrote:
> Badari Pulavarty wrote on Tuesday, March 21, 2006 8:57 AM
> > I hate to do this you - but your patch breaks error handling on
> > synchronous DIO requests.
> >
> > Since you are using "dio->io_error" instead of "dio->result" to
> > represent an error - you need to make sure to check that (also ?)
> > instead of dio->result in direct_io_worker() before calling
> > dio_complete().
> >
> > Isn't it ? Am I missing something ?
>
>
> That's the other part of the maze. AFAICS, in the synchronous path,
> dio_bio_complete already implicitly checks -EIO error:
>
> static int dio_bio_complete(struct dio *dio, struct bio *bio)
> { ...
> return uptodate ? 0 : -EIO;
> }
>
> And such error code bubbles up to direct_io_worker's sync path:
>
> direct_io_worker {
> ...
> if (dio->is_async) {
> ...
> } else {
> ret2 = dio_await_completion(dio);
> if (ret == 0)
> ret = ret2;
>
> I've also explicitly ran test case for synchronous write and found no
> regression there. I admit my test coverage may not be very comprehensive.
> But I've done the best I can.
>
> It's entirely possible there are more corner cases. But let's get some
> coverage here with -mm and then add fixes as we go.
I know that is not your fault - but does this mean that we can't
return success for partial IO ?
If some one asks to do IO for 128K and if we get -EIO after say, 64K
- we fail the whole IO with -EIO ?
Thanks,
Badari
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/