Re: [PATCH 2/2] aio: propogate post-EIOCBQUEUED errors to completion event

From: Zach Brown
Date: Wed Feb 21 2007 - 13:35:22 EST


This is an interesting trick, but I'd like to consider hard whether the added
complexity is worth it. Could you list the various other cases you have in mind
which would want to use it ?

I'm happy to report that the sync case and the invalidate_inode_pages2_range() case are the only two which try to rewrite 'ret'. I audited all the call paths between aio_{read,write} and -EIOCBQUUEUED.

So, sure, maybe we can shuffle the house of cards a little in the direction away from having a fs/aio.c helper for the situation by simply removing the few current instances of the problem. It won't scale as people add problems without knowing about the rule to not overwrite -EIOCBQUEUED, but hopefully that won't be a rule for much longer :) :).

For the O_SYNC case we could add another magical test to the is_async assignment which won't perform AIO on O_SYNC descriptors.

For the invalidate_inode_pages2_range() case, I wonder if the right
place to issue this is after the direct IO write has completed and before
aio_complete() is issued (somewhat like the way we do bio_check_pages_dirty
for DIO reads), rather than after submission when the IO may still not have
hit the disk. This would also make the behaviour uniform for synchronous and
async cases.

Hmm, I think I like that. It solves the problem for the current sole user of -EIOCBQUEUED without too much disruption.

BTW, am I right in interpreting that with your change aio_complete () may
trigger an io_getevents() wakeup, before the corresponding event is placed
on the ring buffer ?

Hmm, yeah, it looks like I goofed that.

I'll roll a patch which does the invalidation down in fs/direct-io.c.

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