Messages in this thread | | | Date | Fri, 2 Jan 2004 10:44:22 +0530 | From | Suparna Bhattacharya <> | Subject | Re: [PATCH linux-2.6.1-rc1-mm1] aiodio_fallback_bio_count.patch |
| |
On Wed, Dec 31, 2003 at 03:46:48PM -0800, Andrew Morton wrote: > Daniel McNeil <daniel@osdl.org> wrote: > > > > This is an update of AIO fallback patch and with the bio_count race > > fixed by changing bio_list_lock into bio_lock and using that for all > > the bio fields. I changed bio_count and bios_in_flight from atomics > > into int. They are now proctected by the bio_lock. I fixed the race, > > by in finished_one_bio() by leaving the bio_count at 1 until after the > > dio_complete() and then do the bio_count decrement and wakeup holding > > the bio_lock. > > Sob. Daniel, please assume that I have an extremely small brain and forget > things very easily. And that 2,000 patches have passed under my nose since > last I contemplated the direct-io code, OK? > > What bio_count race? > > And the patch seems to be doing more than your description describes? > --
We need to combine this with my original patch description of the aio-dio-fallback + follow on patch which had the race that Daniel fixed and then rolled together into one working patch.
Does the following sound better as complete description ?
-----------------------------------------------------------------
This patch ensures that when the DIO code falls back to buffered i/o after having submitted part of the i/o, then buffered i/o is issued only for the remaining part of the request (i.e. the part not already covered by DIO), rather than redo the entire i/o.
We need to careful not to access dio fields if its possible that the dio could already have been freed asynchronously during i/o completion. A tricky part of this involves plugging the window between the decrement of bio_count and accessing dio->waiter during i/o completion where the dio could get freed by the submission path. This potential "bio_count race" was tackled (by Daniel) by changing bio_list_lock into bio_lock and using that for all the bio fields. Now bio_count and bios_in_flight have been converted from atomics into int and are both protected by the bio_lock. The race in finished_one_bio() could thus be fixed by leaving the bio_count at 1 until after the dio_complete() and then doing the bio_count decrement and wakeup holding the bio_lock. It appears that shifting to the spin_lock instead of atomic_inc/decs is ok performance wise as well.
Regards Suparna
-- Suparna Bhattacharya (suparna@in.ibm.com) 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |