Messages in this thread | | | Date | Wed, 19 Aug 2009 15:44:06 +0300 | From | Boaz Harrosh <> | Subject | Re: [PATCH 0/4] Page based O_DIRECT v2 |
| |
On 08/18/2009 11:34 AM, Jens Axboe wrote: > Hi, > > Updated patchset for page based O_DIRECT. I didn't include the > loop bits this time, lets focus on getting these core bits into > shape and then loop is easily patchable on top of this. > > Changes since last post: > > - Changed do_dio() to generic_file_direct_IO() as per Christophs > suggestion. > - Split the first patch into two parts. One simply adds dio_args > and maintains the current code, the next has the functional change > but without changing file systems (except NFS). > - Add ->rw to dio_args (Christoph). > - A locking fixup. Not really related, but should be fixed up anyways. > > There are at least two pending things to work on: > > 1) NFS is still broken, I get a crash in freeing some data that > is not related to the pages. Will debug this. > 2) As Christoph suggested, we need some way to wait for a dio > when all segments are submitted. Currently it waits for each > segment. Not sure how best to solve this issue, will think a > bit more about this. Basically we need to pass down the wait > list to the generic_file_direct_IO() and have that do the > queue kick and wait. >
Jens hi.
I please have some basic question on the subject?
[1] So before, the complete iovec from user mode could potentially be submitted in a single request, depending on the implementor. With new code, each iovec entry is broken to it's few pages and is submitted as a separate request. This might not be bad for block based devices that could see these segments merged back by the IO elevator. But what about the other implementers that see a grate performance boost in the current scatter-gather nature of the iovec API. It's almost as if the application was calling the kernel for each segment separately.
I wish you would use a more generic page carrier then page-* array. and submit the complete iovec at once.
We used to use scatter-lists but these are best only used inside DMA engines and Drivers as they are more then 2 times too big. The ideal for me is the bio_vec array as used inside a bio. scatter-list has all these helpers, iterators, and wrappers, which bio_vec do not, so I don't know what the best choice is.
But your current solution, (from inspection only I have not tested any of this), might mean a grate performance degradation for some work scenarios. For example a user-mode app the gathers lots of small memory sources and hopes to write it as a single very large on-the-wire-NFS-write , might find itself writing lots of small on-the-wire-NFS-writes.
[2] Please address linux-fsdevel on these patches. lkml is so crowded and after all these files do sit in fs/
Thanks Boaz
| |