On Wed, 3 Dec 2014 11:48:28 -0500 Milosz Tanski [off-list ref] wrote:
On Tue, Dec 2, 2014 at 5:42 PM, Andrew Morton [off-list ref] wrote:
quoted
On Tue, 2 Dec 2014 17:17:42 -0500 Milosz Tanski [off-list ref] wrote:
quoted
quoted
There have been several incomplete attempts to implement fincore(). If
we were to complete those attempts, preadv2() could be implemented
using fincore()+pread(). Plus we get fincore(), which is useful for
other (but probably similar) reasons. Probably fincore()+pwrite() could
be used to implement pwritev2(), but I don't know what pwritev2() does
yet.
Implementing fincore() is more flexible, requires less code and is less
likely to have bugs. So why not go that way? Yes, it's more CPU
intensive, but how much? Is the difference sufficient to justify the
preadv2()/pwritev2() approach?
I would like to see a fincore() functionality (for other reasons) I
don't think it does the job here. fincore() + preadv() is inherently
racy as there's no guarantee that the data becomes uncached between
the two calls.
There will always be holes. For example find_get_page() could block on
lock_page() while some other process is doing IO.
page_cache_async_readahead() does lots of memory allocation which can
get blocked for long periods in the page allocator.
page_cache_async_readahead() can block on synchronous metadata reads,
etc.
Andrew I think it would helpful if you did read through the patches.
The first 3 are somewhat uninteresting as it's just wiring up the new
syscalls and plumbing the flag argument through. The core of the
RWF_NONBLOCK is patch 4: https://lkml.org/lkml/2014/11/10/463 and if
you strip away the fs specific changes the core of it is very simple.
The core is mostly contained in do_generic_file_read() in filemap.c,
and is very short and easy to understand. It boils down to we read as
much data as we can given what's in the page cache. There's no
fallback to diskio for readpage() in case of missing pages and we bail
before any calls to page_cache_async_readahead(). And to the best of
my knowledge lock_page() does not lock the page, all it does is call
pagecache_get_page() without the FGP_LOCK flag.
I've spent time a decent amount of time looking at this to make sure
we cover all our major bases. It's possible I missed something but the
biggest offenders should be covered and if I missed something I'd love
to cover that as well.
OK.
quoted
quoted
There's no overlap between prwritev2 and fincore() functionality.
Do we actually need pwritev2()? What's the justification for that?
I'm okay with splitting up the pwritev2 and preadv2 into two
independent patchsets to be considered on their own merits.
Well, we can do both together if both are wanted. The changelogs are
very skimpy on pwritev2(). A full description and careful
justification in the [0/n] changelog would be useful - something that
tells us "what's wrong with O_DSYNC+pwrite".
quoted
Please let's examine the alternative(s) seriously. It would be mistake
to add preadv2/pwritev2 if fincore+pread would have sufficed.
What the motivation for my change and also approach is a very common
pattern to async buffered disk IO in userspace server applications. It
comes down to having one thread to handle the network and a thread
pool to perform IO requests. Why a threadpool and not something like a
sendfile() for reads? Many non-trivial applications perform additional
processing (ssl, checksuming, transformation). Unfortunately this has
a inherent increase in average latency due to increased
synchronization penalties (enqueue and notify) but primarily due to
fast requests (already in cache) behind stuck behind slow request.
Here's the illustration of the common architecture:
http://i.imgur.com/f8Pla7j.png. In fact, most apps are even simpler
where they replace the request queue, task worker with a single thread
doing network IO using epoll or such.
preadv2 with RWF_NONBLOCK is analogous to the kernel recvmsg() with
the MSG_NOWAIT flag. It's really frustrating that such capacity
doesn't exist today. As with the user space application design we can
skip the io threadpool and decrease average request latency in many
common workloads (linear reads or zipf data accesses).
preadv2 with RWF_NONBLOCK as implemented does not suffer the same
eviction races as fincore + pread because it's not implemented as two
syscalls. It also has a much lower surface of possible blocking /
locking then fincore + pread because it cannot fallback to reading
from disk, it does not trigger read-ahead, and does not wait for page
lock.
I can see all that, but it's handwaving. Yes, preadv2() will perform
better in some circumstances than fincore+pread. But how much better?
Enough to justify this approach, or not?
Alas, the only way to really settle that is to implement fincore() and
to subject it to a decent amount of realistic quantitative testing.
Ho hum.
Could you please hunt down some libuv developers, see if we can solicit
some quality input from them? As I said, we really don't want to merge
this then find that people don't use it for some reason, or that it
needs changes.
On Thu, Dec 04, 2014 at 03:11:02PM -0800, Andrew Morton wrote:
I can see all that, but it's handwaving. Yes, preadv2() will perform
better in some circumstances than fincore+pread. But how much better?
Enough to justify this approach, or not?
Alas, the only way to really settle that is to implement fincore() and
to subject it to a decent amount of realistic quantitative testing.
Ho hum.
Could you please hunt down some libuv developers, see if we can solicit
some quality input from them? As I said, we really don't want to merge
this then find that people don't use it for some reason, or that it
needs changes.
All I can say from a Samba perspective is that none of the ARM based
Storage boxes I have seen so far do AIO because of the base footprint
for every read. For sequential reads kernel-level readahead could kick
in properly and we should be able to give them the best of both worlds:
No context switches in the default case but also good parallel behaviour
for other workloads. The most important benchmark for those guys is to
read a DVD image, whether it makes sense or not.
Volker
--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt@sernet.de
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
On Fri, Dec 5, 2014 at 3:17 AM, Volker Lendecke
[off-list ref] wrote:
On Thu, Dec 04, 2014 at 03:11:02PM -0800, Andrew Morton wrote:
quoted
I can see all that, but it's handwaving. Yes, preadv2() will perform
better in some circumstances than fincore+pread. But how much better?
Enough to justify this approach, or not?
Alas, the only way to really settle that is to implement fincore() and
to subject it to a decent amount of realistic quantitative testing.
Ho hum.
Could you please hunt down some libuv developers, see if we can solicit
some quality input from them? As I said, we really don't want to merge
this then find that people don't use it for some reason, or that it
needs changes.
All I can say from a Samba perspective is that none of the ARM based
Storage boxes I have seen so far do AIO because of the base footprint
for every read. For sequential reads kernel-level readahead could kick
in properly and we should be able to give them the best of both worlds:
No context switches in the default case but also good parallel behaviour
for other workloads. The most important benchmark for those guys is to
read a DVD image, whether it makes sense or not.
I just made wanted to share some progress on this. And I apologize for
for all these different threads (this, LSF/FS and then Jermey and
Volker).
I recently implemented cifs support (via libsmbcli) for FIO so I can
have some hard numbers on the benchmarks. So all you guys will be
seeing more data soon enough. It's going to take a bit of time to put
it together because it takes a lot of time to benchmark to make sure
we have correct and non-noisy numbers.
In the meantime I have some numbers from my first run here:
http://i.imgur.com/05SMu8d.jpg
Sorry for the link to the image, it was easier. The test case is a
single FIO client doing 4K random reads, on localhost smbd server, on
a fully cached file for 10 minutes with a 1 minute warm up.
Threadpool + preadv2 for fast read does much better in terms of
bandwidth and a bit better in terms of latency. Sync is still the
fastest, but the gap is narrowed. Not a bad improvement for (Volker's)
9 line change to samba code.
Also, I look into why the gap between sync and threadpool + preadv2 is
not even smaller. From my preliminary investigation it looks like the
async threadpool code path does a lot more work then the sync call...
even in the case we do the fast read. According to perf the hotest
code userspace (smbd+ library) is malloc + free. So I imagine the
optimizing the fast read case to avoid a bunch of extra request
allocations will bring us even closer to sync.
Again, I'll have and more complex test cases soon just wanted to share
progress. I imagine that they'll the gap between threadpool + preadv2
and just threadpool is going to get wider as add more blocking calls
into the queue. I'll have number on that as soon as week can.
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 5634cc0..90348d8 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -718,6 +741,7 @@ static struct tevent_req
*vfswrap_pread_send(struct vfs_handle_struct *handle,
struct tevent_req *req;
struct vfswrap_asys_state *state;
int ret;
+ ssize_t nread;
req = tevent_req_create(mem_ctx, &state, struct vfswrap_asys_state);
if (req == NULL) {@@ -730,6 +754,14 @@ static struct tevent_req
*vfswrap_pread_send(struct vfs_handle_struct *handle,
state->asys_ctx = handle->conn->sconn->asys_ctx;
state->req = req;
+ nread = pread2(fsp->fh->fd, data, n, offset, RWF_NONBLOCK);
+ // TODO: partial reads
+ if (nread == n) {
+ state->ret = nread;
+ tevent_req_done(req);
+ return tevent_req_post(req, ev);
+ }
+
SMBPROFILE_BYTES_ASYNC_START(syscall_asys_pread, profile_p,
state->profile_bytes, n);
ret = asys_pread(state->asys_ctx, fsp->fh->fd, data, n, offset, req);
--
Milosz Tanski
CTO
16 East 34th Street, 15th floor
New York, NY 10016
p: 646-253-9055
e: milosz@adfin.com
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>