Re: [RFC 0/5] dio: clean up completion phase of direct_io_worker()

[Suparna Bhattacharya]

On Tue, Sep 05, 2006 at 04:57:32PM -0700, Zach Brown wrote:
> There have been a lot of bugs recently due to the way direct_io_worker() tries
> to decide how to finish direct IO operations.  In the worst examples it has
> failed to call aio_complete() at all (hang) or called it too many times (oops).
> 
> This set of patches cleans up the completion phase with the goal of removing
> the complexity that lead to these bugs.  We end up with one path that
> calculates the result of the operation after all off the bios have completed.
> We decide when to generate a result of the operation using that path based on
> the final release of a refcount on the dio structure.

Very nice piece of work ! Thanks for taking this up :)

I have looked through all the patches and the completion path unification
logic looks sound to me (btw, the explanations and comments are very good too).
Not the usual bandaids that we often end up with, but a real cleanup that
should go some way in making at least the most errorprone part of the DIO
code more maintainable (I hope/wish we could also do something similar with
simplifying the locking as well).

Of course with this code, we have to await rigorous testing
... and more reviews, but please consider this as my ack for the approach.

Regards
Suparna

> 
> I tried to progress towards the final state in steps that were relatively easy
> to understand.  Each step should compile but I only tested the final result of
> having all the patches applied.
> 
> The patches result in a slight net decrease in code and binary size:
> 
>  2.6.18-rc4-dio-cleanup/fs/direct-io.c |    8
>  2.6.18-rc5-dio-cleanup/fs/direct-io.c |   94 +++++------
>  2.6.18-rc5-dio-cleanup/mm/filemap.c   |    4
>  fs/direct-io.c                        |  273 ++++++++++++++--------------------
>  4 files changed, 159 insertions(+), 220 deletions(-)
> 
>    text    data     bss     dec     hex filename
> 2592385  450996  210296 3253677  31a5ad vmlinux.before
> 2592113  450980  210296 3253389  31a48d vmlinux.after
> 
> The patches pass light testing with aio-stress, the direct IO tests I could
> manage to get running in LTP, and some home-brew functional tests.  It's still
> making its way through stress testing.  It should not be merged until we hear
> from that more rigorous testing, I don't think.
> 
> I hoped to get some feedback (and maybe volunteers for testing!) by sending the
> patches out before waiting for the stress tests.
> 
> - z
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-aio' in
> the body to majordomo@xxxxxxxxxx  For more info on Linux AIO,
> see: http://www.kvack.org/aio/
> Don't email: <a href=mailto:"aart@xxxxxxxxx";>aart@xxxxxxxxx</a>

-- 
Suparna Bhattacharya (suparna@xxxxxxxxxx)
Linux Technology Center
IBM Software Lab, India

-
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/