From: Andrew Morton <hidden> Date: 2015-03-27 09:01:59
On Fri, 27 Mar 2015 01:48:33 -0700 Christoph Hellwig [off-list ref] wrote:
On Fri, Mar 27, 2015 at 01:35:16AM -0700, Andrew Morton wrote:
quoted
fincore() doesn't have to be ugly. Please address the design issues I
raised. How is pread2() useful to the class of applications which
cannot proceed until all data is available?
It actually makes them work correctly? preadv2( ..., DONTWAIT) will
return -EGAIN, which causes them to bounce to the threadpool where
they call preadv(...).
(I assume you mean RWF_NONBLOCK)
That isn't how pread2() works. If the leading one or more pages are
uptodate, pread2() will return a partial read. Now what? Either the
application reads the same data a second time via the worker thread
(dumb, but it will usually be a rare case) or it reads the remainder of
the data in the worker thread and splices the data back together.
Which, as I said, will often result in a second load of the initial
read result into CPU cache.
On Fri, Mar 27, 2015 at 02:01:59AM -0700, Andrew Morton wrote:
On Fri, 27 Mar 2015 01:48:33 -0700 Christoph Hellwig [off-list ref] wrote:
quoted
On Fri, Mar 27, 2015 at 01:35:16AM -0700, Andrew Morton wrote:
quoted
fincore() doesn't have to be ugly. Please address the design issues I
raised. How is pread2() useful to the class of applications which
cannot proceed until all data is available?
It actually makes them work correctly? preadv2( ..., DONTWAIT) will
return -EGAIN, which causes them to bounce to the threadpool where
they call preadv(...).
(I assume you mean RWF_NONBLOCK)
That isn't how pread2() works. If the leading one or more pages are
uptodate, pread2() will return a partial read. Now what? Either the
application reads the same data a second time via the worker thread
(dumb, but it will usually be a rare case) or it reads the remainder of
the data in the worker thread and splices the data back together.
Which, as I said, will often result in a second load of the initial
read result into CPU cache.
Sorry, but I don't have a good picture how we are supposed
to use that. I'm fine with two syscalls, but I need a way to
tell the kernel to either block or not. Or do you want Samba
to do repeated pread calls for ever shorter blocks? Right
now I don't see a way to tell pread to either give me a
short result or really block. To me that's the core of
preadv2. I'm perfectly find for a syscall to give me a short
read instead of a global EWOULDBLOCK. I need a way to tell
the kernel which behaviour I want.
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
From: Jeremy Allison <hidden> Date: 2015-03-27 15:58:54
On Fri, Mar 27, 2015 at 02:01:59AM -0700, Andrew Morton wrote:
On Fri, 27 Mar 2015 01:48:33 -0700 Christoph Hellwig [off-list ref] wrote:
quoted
On Fri, Mar 27, 2015 at 01:35:16AM -0700, Andrew Morton wrote:
quoted
fincore() doesn't have to be ugly. Please address the design issues I
raised. How is pread2() useful to the class of applications which
cannot proceed until all data is available?
It actually makes them work correctly? preadv2( ..., DONTWAIT) will
return -EGAIN, which causes them to bounce to the threadpool where
they call preadv(...).
(I assume you mean RWF_NONBLOCK)
That isn't how pread2() works. If the leading one or more pages are
uptodate, pread2() will return a partial read. Now what? Either the
application reads the same data a second time via the worker thread
(dumb, but it will usually be a rare case)
The problem with the above is that we can't tell the difference
between pread2() returning a short read because the pages are not
in cache, or because someone truncated the file. So we need some
way to differentiate this.
My preference from userspace would be for pread2() to return
EAGAIN if *all* the data requested is not available (where
'all' can be less than the size requested if the file has
been truncated in the meantime).
So:
ret = pread2(fd, buf, size_wanted, RWF_NONBLOCK)
if (ret == -1) {
if (errno == EAGAIN) {
goto threadpool...
}
.. real error..
}
if (ret == size_wanted) {
.. normal read, file not truncated...
}
if (ret < size_wanted) {
.. file was truncated..
}
The thing I want to avoid is the case where
ret < size_wanted means only part of the file
is in cache.
--
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>
From: Andrew Morton <hidden> Date: 2015-03-27 16:30:46
On Fri, 27 Mar 2015 08:58:54 -0700 Jeremy Allison [off-list ref] wrote:
On Fri, Mar 27, 2015 at 02:01:59AM -0700, Andrew Morton wrote:
quoted
On Fri, 27 Mar 2015 01:48:33 -0700 Christoph Hellwig [off-list ref] wrote:
quoted
On Fri, Mar 27, 2015 at 01:35:16AM -0700, Andrew Morton wrote:
quoted
fincore() doesn't have to be ugly. Please address the design issues I
raised. How is pread2() useful to the class of applications which
cannot proceed until all data is available?
It actually makes them work correctly? preadv2( ..., DONTWAIT) will
return -EGAIN, which causes them to bounce to the threadpool where
they call preadv(...).
(I assume you mean RWF_NONBLOCK)
That isn't how pread2() works. If the leading one or more pages are
uptodate, pread2() will return a partial read. Now what? Either the
application reads the same data a second time via the worker thread
(dumb, but it will usually be a rare case)
The problem with the above is that we can't tell the difference
between pread2() returning a short read because the pages are not
in cache, or because someone truncated the file. So we need some
way to differentiate this.
My preference from userspace would be for pread2() to return
EAGAIN if *all* the data requested is not available (where
'all' can be less than the size requested if the file has
been truncated in the meantime).
...
The thing I want to avoid is the case where
ret < size_wanted means only part of the file
is in cache.
From my reading of the code, pread2() will return -EAGAIN only when it
copied zero bytes to userspace. ie, the very first page wasn't in
cache. If pread2() does copy some data to userspace then it will
return the amount of data copied. This is traditional read()
behaviour.
Maybe there's some other code somewhere in the patch which converts
that short read into -EAGAIN, dunno - the changelogs don't appear to
mention it and the manpage update is ambiguous about this.
But from an interface perspective the behaviour you're asking for is
insane, frankly - if the kernel copied out 8k of data then pread2()
should return 8k. Otherwise there's no way for userspace to know that
the 8k copy actually happened and we have just wasted a great pile of
CPU doing a pointless memcpy.
I expect that this situation (first part in cache, latter part not in
cache) is rare - for reasonably small requests the common cases will be
"all cached" and "nothing cached". So perhaps the best approach here
is for samba to add special handling for the short read, to work out
the reason for its occurrence.
Alternatively we could add another flag to pread2() to select this
"throw away my data and return -EAGAIN" behaviour. Presumably
implemented with an i_size check, but it's gonna be racy.
I take it from your comments that nobody has actually wired up pread2()
into samba yet? That's a bit disturbing, because if we later want to
go and change something like this short-read behaviour, we're screwed -
it's a non back-compat userspace-visible change.
And a note on cosmetics: why are we using EAGAIN here rather than
EWOULDBLOCK? They have the same numerical value, but EWOULDBLOCK is a
better name - EAGAIN says "run it again", but that won't work.
From: Andrew Morton <akpm@linux-foundation.org> Date: 2015-03-27 16:38:30
On Fri, 27 Mar 2015 09:30:46 -0700 Andrew Morton [off-list ref] wrote:
I expect that this situation (first part in cache, latter part not in
cache) is rare - for reasonably small requests the common cases will be
"all cached" and "nothing cached". So perhaps the best approach here
is for samba to add special handling for the short read, to work out
the reason for its occurrence.
Alternatively we could add another flag to pread2() to select this
"throw away my data and return -EAGAIN" behaviour. Presumably
implemented with an i_size check, but it's gonna be racy.
Here's a better way:
nr_read = pread2(buf, len);
if (nr_read < len)
nr_read += pread(buf + nr_read, len - nr_read);
if (nr_read < len)
we_hit_eof();
On Fri, Mar 27, 2015 at 11:58 AM, Jeremy Allison [off-list ref] wrote:
On Fri, Mar 27, 2015 at 02:01:59AM -0700, Andrew Morton wrote:
quoted
On Fri, 27 Mar 2015 01:48:33 -0700 Christoph Hellwig [off-list ref] wrote:
quoted
On Fri, Mar 27, 2015 at 01:35:16AM -0700, Andrew Morton wrote:
quoted
fincore() doesn't have to be ugly. Please address the design issues I
raised. How is pread2() useful to the class of applications which
cannot proceed until all data is available?
It actually makes them work correctly? preadv2( ..., DONTWAIT) will
return -EGAIN, which causes them to bounce to the threadpool where
they call preadv(...).
(I assume you mean RWF_NONBLOCK)
That isn't how pread2() works. If the leading one or more pages are
uptodate, pread2() will return a partial read. Now what? Either the
application reads the same data a second time via the worker thread
(dumb, but it will usually be a rare case)
The problem with the above is that we can't tell the difference
between pread2() returning a short read because the pages are not
in cache, or because someone truncated the file. So we need some
way to differentiate this.
My preference from userspace would be for pread2() to return
EAGAIN if *all* the data requested is not available (where
'all' can be less than the size requested if the file has
been truncated in the meantime).
So:
ret = pread2(fd, buf, size_wanted, RWF_NONBLOCK)
if (ret == -1) {
if (errno == EAGAIN) {
goto threadpool...
}
.. real error..
}
if (ret == size_wanted) {
.. normal read, file not truncated...
}
if (ret < size_wanted) {
.. file was truncated..
}
The thing I want to avoid is the case where
ret < size_wanted means only part of the file
is in cache.
I very much like the short read behavior. It lets you overlap some CPU
work partial data (like TLS and then sticking it network output
buffer) with waiting for the test of the data (enequed in the thread
pool).
Short reads are the current behavior, if you call preadv2 a second
time around at EOF it'll return 0 instead of EWOULDBLOCK today. I
actually test for this in the preadv2 test in xfstest here:
https://github.com/mtanski/xfstests/commit/688db24c292999c81ee17caf2b61fe8cf7bb3cd6#diff-114416ea98ce29dde3b5b3d145afbd2bR81.
There's one caveat, that it's possible to get EWOULDBLOCK when reading
at end of file if the file metadata is not paged in.
--
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>
From: Jeremy Allison <hidden> Date: 2015-03-27 16:39:08
On Fri, Mar 27, 2015 at 09:30:46AM -0700, Andrew Morton wrote:
But from an interface perspective the behaviour you're asking for is
insane, frankly - if the kernel copied out 8k of data then pread2()
should return 8k. Otherwise there's no way for userspace to know that
the 8k copy actually happened and we have just wasted a great pile of
CPU doing a pointless memcpy.
Why would it do the copy in the first place if we asked (for example)
for 16k, but only 8k was available ? Just return EAGAIN and have
done with it.
I expect that this situation (first part in cache, latter part not in
cache) is rare - for reasonably small requests the common cases will be
"all cached" and "nothing cached". So perhaps the best approach here
is for samba to add special handling for the short read, to work out
the reason for its occurrence.
We can do that, but as Volker says this is a very hot code path.
I take it from your comments that nobody has actually wired up pread2()
into samba yet? That's a bit disturbing, because if we later want to
go and change something like this short-read behaviour, we're screwed -
it's a non back-compat userspace-visible change.
It's been done as a test, so the code exists and has run (and improved
perforamance as I recall). Not much point commiting it without kernel
support :-).
And a note on cosmetics: why are we using EAGAIN here rather than
EWOULDBLOCK? They have the same numerical value, but EWOULDBLOCK is a
better name - EAGAIN says "run it again", but that won't work.
Sounds good to me !
--
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, Mar 27, 2015 at 12:30 PM, Andrew Morton
[off-list ref] wrote:
On Fri, 27 Mar 2015 08:58:54 -0700 Jeremy Allison [off-list ref] wrote:
quoted
On Fri, Mar 27, 2015 at 02:01:59AM -0700, Andrew Morton wrote:
quoted
On Fri, 27 Mar 2015 01:48:33 -0700 Christoph Hellwig [off-list ref] wrote:
quoted
On Fri, Mar 27, 2015 at 01:35:16AM -0700, Andrew Morton wrote:
quoted
fincore() doesn't have to be ugly. Please address the design issues I
raised. How is pread2() useful to the class of applications which
cannot proceed until all data is available?
It actually makes them work correctly? preadv2( ..., DONTWAIT) will
return -EGAIN, which causes them to bounce to the threadpool where
they call preadv(...).
(I assume you mean RWF_NONBLOCK)
That isn't how pread2() works. If the leading one or more pages are
uptodate, pread2() will return a partial read. Now what? Either the
application reads the same data a second time via the worker thread
(dumb, but it will usually be a rare case)
The problem with the above is that we can't tell the difference
between pread2() returning a short read because the pages are not
in cache, or because someone truncated the file. So we need some
way to differentiate this.
My preference from userspace would be for pread2() to return
EAGAIN if *all* the data requested is not available (where
'all' can be less than the size requested if the file has
been truncated in the meantime).
...
The thing I want to avoid is the case where
ret < size_wanted means only part of the file
is in cache.
From my reading of the code, pread2() will return -EAGAIN only when it
copied zero bytes to userspace. ie, the very first page wasn't in
cache. If pread2() does copy some data to userspace then it will
return the amount of data copied. This is traditional read()
behaviour.
Maybe there's some other code somewhere in the patch which converts
that short read into -EAGAIN, dunno - the changelogs don't appear to
mention it and the manpage update is ambiguous about this.
But from an interface perspective the behaviour you're asking for is
insane, frankly - if the kernel copied out 8k of data then pread2()
should return 8k. Otherwise there's no way for userspace to know that
the 8k copy actually happened and we have just wasted a great pile of
CPU doing a pointless memcpy.
I expect that this situation (first part in cache, latter part not in
cache) is rare - for reasonably small requests the common cases will be
"all cached" and "nothing cached". So perhaps the best approach here
is for samba to add special handling for the short read, to work out
the reason for its occurrence.
Alternatively we could add another flag to pread2() to select this
"throw away my data and return -EAGAIN" behaviour. Presumably
implemented with an i_size check, but it's gonna be racy.
I take it from your comments that nobody has actually wired up pread2()
into samba yet? That's a bit disturbing, because if we later want to
go and change something like this short-read behaviour, we're screwed -
it's a non back-compat userspace-visible change.
Volker and did wired so we can use Samba as a test / use case. The
change we made was quick and dirty 9 lines of code, if you exclude the
syscall boiler plate. In fact, right now it does the stupid thing of
throwing away the partial result and enqueing in the threadpool if it
doesn't get the whole block. Volker agreed that was as much as we need
to do to get the numbers and we'll make a proper patch once it's in
upstream.
Patch to samba at end of email for reference.
And a note on cosmetics: why are we using EAGAIN here rather than
EWOULDBLOCK? They have the same numerical value, but EWOULDBLOCK is a
better name - EAGAIN says "run it again", but that won't work.
From: Christoph Hellwig <hch@infradead.org> Date: 2015-03-30 07:36:04
On Fri, Mar 27, 2015 at 08:58:54AM -0700, Jeremy Allison wrote:
The problem with the above is that we can't tell the difference
between pread2() returning a short read because the pages are not
in cache, or because someone truncated the file. So we need some
way to differentiate this.
Is a race vs truncate really that time critical that you can't
wait for the thread pool to do the second read to notice it?
My preference from userspace would be for pread2() to return
EAGAIN if *all* the data requested is not available (where
'all' can be less than the size requested if the file has
been truncated in the meantime).
That is easily implementable, but I can see that for example web apps
would be happy to get as much as possible. So if Samba can be ok
with short reads and only detecting the truncated case in the slow
path that would make life simpler. Otherwise we might indeed need two
flags.
From: Andrew Morton <akpm@linux-foundation.org> Date: 2015-03-30 20:26:25
On Mon, 30 Mar 2015 00:36:04 -0700 Christoph Hellwig [off-list ref] wrote:
On Fri, Mar 27, 2015 at 08:58:54AM -0700, Jeremy Allison wrote:
quoted
The problem with the above is that we can't tell the difference
between pread2() returning a short read because the pages are not
in cache, or because someone truncated the file. So we need some
way to differentiate this.
Is a race vs truncate really that time critical that you can't
wait for the thread pool to do the second read to notice it?
quoted
My preference from userspace would be for pread2() to return
EAGAIN if *all* the data requested is not available (where
'all' can be less than the size requested if the file has
been truncated in the meantime).
That is easily implementable, but I can see that for example web apps
would be happy to get as much as possible. So if Samba can be ok
with short reads and only detecting the truncated case in the slow
path that would make life simpler. Otherwise we might indeed need two
flags.
The problem is that many applications (including samba!) want
all-or-nothing behaviour, and preadv2() cannot provide it. By the time
preadv2() discovers a not-present page, it has already copied bulk data
out to userspace.
To fix this, preadv2() would need to take two passes across the pages,
pinning them in between and somehow blocking out truncate. That's a
big change.
With the current preadv2(), applications would have to do
nr_read = preadv2(..., offset, len, ...);
if (nr_read == len)
process data;
else
punt(offset + nr_read, len - nr_read);
and the worker thread will later have to splice together the initial
data and the later-arriving data, probably on another CPU, probably
after the initial data has gone cache-cold.
A cleaner solution is
if (fincore(fd, NULL, offset, len) == len) {
preadv(..., offset, len);
process data;
} else {
punt(offset, len);
}
This way all the data gets copied in a single hit and is cache-hot when
userspace processes it.
Comparing fincore()+pread() to preadv2():
pros:
a) fincore() may be used to provide both all-or-nothing and
part-read-ok behaviour cleanly and with optimum cache behaviour.
b) fincore() doesn't add overhead, complexity and stack depth to
core pagecache read() code. Nor does it expand VFS data structures.
c) with a non-NULL second argument, fincore provides the
mincore()-style page map.
cons:
d) fincore() is more expensive
e) fincore() will very occasionally block
Tradeoffs are involved. To decide on the best path we should examine
d). I expect that the overhead will be significant for small reads but
not significant for medium and large reads. Needs quantifying.
And I don't believe that e) will be a problem in the real world. It's
a significant increase in worst-case latency and a negligible increase
in average latency. I've asked at least three times for someone to
explain why this is unacceptable and no explanation has been provided.
--
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>
From: Jeremy Allison <hidden> Date: 2015-03-30 20:32:27
On Mon, Mar 30, 2015 at 01:26:25PM -0700, Andrew Morton wrote:
cons:
d) fincore() is more expensive
e) fincore() will very occasionally block
The above is the killer for Samba. If fincore
returns true but when we schedule the pread
we block, we're hosed.
Once we block, we're done serving clients on the main
thread until this returns. That can cause unpredictable
response times which can cause client timeouts.
A fincore+pread solution that blocks is simply unsafe
to use for us. We'll have to stay with the threadpool :-(.
And I don't believe that e) will be a problem in the real world. It's
a significant increase in worst-case latency and a negligible increase
in average latency. I've asked at least three times for someone to
explain why this is unacceptable and no explanation has been provided.
From: Andrew Morton <akpm@linux-foundation.org> Date: 2015-03-30 20:37:58
On Mon, 30 Mar 2015 13:32:27 -0700 Jeremy Allison [off-list ref] wrote:
On Mon, Mar 30, 2015 at 01:26:25PM -0700, Andrew Morton wrote:
quoted
cons:
d) fincore() is more expensive
e) fincore() will very occasionally block
The above is the killer for Samba. If fincore
returns true but when we schedule the pread
we block, we're hosed.
Once we block, we're done serving clients on the main
thread until this returns. That can cause unpredictable
response times which can cause client timeouts.
A fincore+pread solution that blocks is simply unsafe
to use for us. We'll have to stay with the threadpool :-(.
Finally. Thanks ;)
This implies that the samba main thread also has to avoid any memory
allocations both direct and within syscall and pagefault - those will
occasionally exhibit similar worse-case latency. Is this done now?
From: Jeremy Allison <hidden> Date: 2015-03-30 20:49:40
On Mon, Mar 30, 2015 at 01:37:58PM -0700, Andrew Morton wrote:
On Mon, 30 Mar 2015 13:32:27 -0700 Jeremy Allison [off-list ref] wrote:
quoted
On Mon, Mar 30, 2015 at 01:26:25PM -0700, Andrew Morton wrote:
quoted
cons:
d) fincore() is more expensive
e) fincore() will very occasionally block
The above is the killer for Samba. If fincore
returns true but when we schedule the pread
we block, we're hosed.
Once we block, we're done serving clients on the main
thread until this returns. That can cause unpredictable
response times which can cause client timeouts.
A fincore+pread solution that blocks is simply unsafe
to use for us. We'll have to stay with the threadpool :-(.
Finally. Thanks ;)
This implies that the samba main thread also has to avoid any memory
allocations both direct and within syscall and pagefault - those will
occasionally exhibit similar worse-case latency. Is this done now?
We don't do anything special around allocations in syscall.
For aio read we do talloc (internal memory allocator) the
return chunk before going into the pthread pread, so I
suppose this could block. Haven't seen this as a reported
problem though. I suppose you can say "well exactly the
same thing is true of fincore()" :-).
From: Andrew Morton <akpm@linux-foundation.org> Date: 2015-03-30 21:33:53
On Mon, 30 Mar 2015 13:49:37 -0700 Jeremy Allison [off-list ref] wrote:
quoted
This implies that the samba main thread also has to avoid any memory
allocations both direct and within syscall and pagefault - those will
occasionally exhibit similar worse-case latency. Is this done now?
We don't do anything special around allocations in syscall.
For aio read we do talloc (internal memory allocator) the
return chunk before going into the pthread pread, so I
suppose this could block. Haven't seen this as a reported
problem though. I suppose you can say "well exactly the
same thing is true of fincore()" :-).
yup. If we tickle the page's referenced bit in fincore() then the race
will only happen under the most withering memory loads, and it sounds
like the main thread will be suffering allocation stalls before that
point anyway.
On Mon, Mar 30, 2015 at 4:37 PM, Andrew Morton
[off-list ref] wrote:
On Mon, 30 Mar 2015 13:32:27 -0700 Jeremy Allison [off-list ref] wrote:
quoted
On Mon, Mar 30, 2015 at 01:26:25PM -0700, Andrew Morton wrote:
quoted
cons:
d) fincore() is more expensive
e) fincore() will very occasionally block
The above is the killer for Samba. If fincore
returns true but when we schedule the pread
we block, we're hosed.
Once we block, we're done serving clients on the main
thread until this returns. That can cause unpredictable
response times which can cause client timeouts.
A fincore+pread solution that blocks is simply unsafe
to use for us. We'll have to stay with the threadpool :-(.
Finally. Thanks ;)
This implies that the samba main thread also has to avoid any memory
allocations both direct and within syscall and pagefault - those will
occasionally exhibit similar worse-case latency. Is this done now?
It's entirely possible to have an application with a low / semi static
working set, and leave lots of free memory for the kernel especially
for the page cache. For example the Google want to minimize malloc().
So in tcmalloc() they grab large chunks and rarely release it to back
to the OS, in fact old version never shrank it. So you can entirely
avoid stalls in malloc() for many workloads.
--
Milosz Tanski
CTO
16 East 34th Street, 15th floor
New York, NY 10016
p: 646-253-9055
e: milosz@adfin.com
On Mon, Mar 30, 2015 at 4:32 PM, Jeremy Allison [off-list ref] wrote:
On Mon, Mar 30, 2015 at 01:26:25PM -0700, Andrew Morton wrote:
quoted
cons:
d) fincore() is more expensive
e) fincore() will very occasionally block
The above is the killer for Samba. If fincore
returns true but when we schedule the pread
we block, we're hosed.
Once we block, we're done serving clients on the main
thread until this returns. That can cause unpredictable
response times which can cause client timeouts.
A fincore+pread solution that blocks is simply unsafe
to use for us. We'll have to stay with the threadpool :-(.
We're getting data from a network filesystem Ceph in our case, but it
could be pNFS. In many cases those filesystems have some kind
hierarchy and it's not uncommon for us to se requests that take 20 to
25 milliseconds to complete. In this case the miss becomes very
expensive. And it's not just that one requests experiences the slow
down all the request being serviced by that (single) epoll thread
experience head-of-line blocking because of one stalled request.
10K request a second is a common load for many web services / video
servers servings chunks of data. If we experience one miss a second,
that 25 million stall will impact 250 other requests (all of them will
have a 25ms latency tacked on).
quoted
And I don't believe that e) will be a problem in the real world. It's
a significant increase in worst-case latency and a negligible increase
in average latency. I've asked at least three times for someone to
explain why this is unacceptable and no explanation has been provided.
See above.
--
Milosz Tanski
CTO
16 East 34th Street, 15th floor
New York, NY 10016
p: 646-253-9055
e: milosz@adfin.com
From: Andrew Morton <akpm@linux-foundation.org> Date: 2015-03-30 22:57:00
On Mon, 30 Mar 2015 18:49:06 -0400 Milosz Tanski [off-list ref] wrote:
quoted
A fincore+pread solution that blocks is simply unsafe
to use for us. We'll have to stay with the threadpool :-(.
We're getting data from a network filesystem Ceph in our case, but it
could be pNFS. In many cases those filesystems have some kind
hierarchy and it's not uncommon for us to se requests that take 20 to
25 milliseconds to complete. In this case the miss becomes very
expensive. And it's not just that one requests experiences the slow
down all the request being serviced by that (single) epoll thread
experience head-of-line blocking because of one stalled request.
10K request a second is a common load for many web services / video
servers servings chunks of data. If we experience one miss a second,
that 25 million stall will impact 250 other requests (all of them will
have a 25ms latency tacked on).
I'd expect a fincore() which doesn't do SetPageReferenced() to be
orders of magnitude better than this. A fincore() which does use
SetPageReferenced() will be in the "basically never happens" region -
it would take massive and artificial memory stress to trigger.
On Mon, Mar 30, 2015 at 3:36 AM, Christoph Hellwig [off-list ref] wrote:
On Fri, Mar 27, 2015 at 08:58:54AM -0700, Jeremy Allison wrote:
quoted
The problem with the above is that we can't tell the difference
between pread2() returning a short read because the pages are not
in cache, or because someone truncated the file. So we need some
way to differentiate this.
Is a race vs truncate really that time critical that you can't
wait for the thread pool to do the second read to notice it?
quoted
My preference from userspace would be for pread2() to return
EAGAIN if *all* the data requested is not available (where
'all' can be less than the size requested if the file has
been truncated in the meantime).
That is easily implementable, but I can see that for example web apps
would be happy to get as much as possible. So if Samba can be ok
with short reads and only detecting the truncated case in the slow
path that would make life simpler. Otherwise we might indeed need two
flags.
I'm okay with an old or nothing flag. Although I think that would much
more useful with RWF_NONWAIT with pwritev, in applications that don't
want to block while logging (but it's okay to drop low level log
messages). That's a whole different use case in my mind.
--
Milosz Tanski
CTO
16 East 34th Street, 15th floor
New York, NY 10016
p: 646-253-9055
e: milosz@adfin.com
On Mon, Mar 30, 2015 at 4:26 PM, Andrew Morton
[off-list ref] wrote:
On Mon, 30 Mar 2015 00:36:04 -0700 Christoph Hellwig [off-list ref] wrote:
quoted
On Fri, Mar 27, 2015 at 08:58:54AM -0700, Jeremy Allison wrote:
quoted
The problem with the above is that we can't tell the difference
between pread2() returning a short read because the pages are not
in cache, or because someone truncated the file. So we need some
way to differentiate this.
Is a race vs truncate really that time critical that you can't
wait for the thread pool to do the second read to notice it?
quoted
My preference from userspace would be for pread2() to return
EAGAIN if *all* the data requested is not available (where
'all' can be less than the size requested if the file has
been truncated in the meantime).
That is easily implementable, but I can see that for example web apps
would be happy to get as much as possible. So if Samba can be ok
with short reads and only detecting the truncated case in the slow
path that would make life simpler. Otherwise we might indeed need two
flags.
The problem is that many applications (including samba!) want
all-or-nothing behaviour, and preadv2() cannot provide it. By the time
preadv2() discovers a not-present page, it has already copied bulk data
out to userspace.
To fix this, preadv2() would need to take two passes across the pages,
pinning them in between and somehow blocking out truncate. That's a
big change.
With the current preadv2(), applications would have to do
nr_read = preadv2(..., offset, len, ...);
if (nr_read == len)
process data;
else
punt(offset + nr_read, len - nr_read);
and the worker thread will later have to splice together the initial
data and the later-arriving data, probably on another CPU, probably
after the initial data has gone cache-cold.
A cleaner solution is
if (fincore(fd, NULL, offset, len) == len) {
preadv(..., offset, len);
process data;
} else {
punt(offset, len);
}
This way all the data gets copied in a single hit and is cache-hot when
userspace processes it.
Comparing fincore()+pread() to preadv2():
pros:
a) fincore() may be used to provide both all-or-nothing and
part-read-ok behaviour cleanly and with optimum cache behaviour.
b) fincore() doesn't add overhead, complexity and stack depth to
core pagecache read() code. Nor does it expand VFS data structures.
Actually, we're not expanding any VFS structures with the next
patchset. I've rebased the forthcoming patchset ontop of Al's
vfs/linux-next tree to keep track of the refactoring already done with
some of the code paths I touched. The refactoring work done there
already ads a flag argument to kiocb struct for other reasons.
c) with a non-NULL second argument, fincore provides the
mincore()-style page map.
cons:
d) fincore() is more expensive
e) fincore() will very occasionally block
Tradeoffs are involved. To decide on the best path we should examine
d). I expect that the overhead will be significant for small reads but
not significant for medium and large reads. Needs quantifying.
And I don't believe that e) will be a problem in the real world. It's
a significant increase in worst-case latency and a negligible increase
in average latency. I've asked at least three times for someone to
explain why this is unacceptable and no explanation has been provided.
--
Milosz Tanski
CTO
16 East 34th Street, 15th floor
New York, NY 10016
p: 646-253-9055
e: milosz@adfin.com
On Fri, Mar 27, 2015 at 12:30 PM, Andrew Morton
[off-list ref] wrote:
On Fri, 27 Mar 2015 08:58:54 -0700 Jeremy Allison [off-list ref] wrote:
quoted
On Fri, Mar 27, 2015 at 02:01:59AM -0700, Andrew Morton wrote:
quoted
On Fri, 27 Mar 2015 01:48:33 -0700 Christoph Hellwig [off-list ref] wrote:
quoted
On Fri, Mar 27, 2015 at 01:35:16AM -0700, Andrew Morton wrote:
quoted
fincore() doesn't have to be ugly. Please address the design issues I
raised. How is pread2() useful to the class of applications which
cannot proceed until all data is available?
It actually makes them work correctly? preadv2( ..., DONTWAIT) will
return -EGAIN, which causes them to bounce to the threadpool where
they call preadv(...).
(I assume you mean RWF_NONBLOCK)
That isn't how pread2() works. If the leading one or more pages are
uptodate, pread2() will return a partial read. Now what? Either the
application reads the same data a second time via the worker thread
(dumb, but it will usually be a rare case)
The problem with the above is that we can't tell the difference
between pread2() returning a short read because the pages are not
in cache, or because someone truncated the file. So we need some
way to differentiate this.
My preference from userspace would be for pread2() to return
EAGAIN if *all* the data requested is not available (where
'all' can be less than the size requested if the file has
been truncated in the meantime).
...
The thing I want to avoid is the case where
ret < size_wanted means only part of the file
is in cache.
From my reading of the code, pread2() will return -EAGAIN only when it
copied zero bytes to userspace. ie, the very first page wasn't in
cache. If pread2() does copy some data to userspace then it will
return the amount of data copied. This is traditional read()
behaviour.
Maybe there's some other code somewhere in the patch which converts
that short read into -EAGAIN, dunno - the changelogs don't appear to
mention it and the manpage update is ambiguous about this.
But from an interface perspective the behaviour you're asking for is
insane, frankly - if the kernel copied out 8k of data then pread2()
should return 8k. Otherwise there's no way for userspace to know that
the 8k copy actually happened and we have just wasted a great pile of
CPU doing a pointless memcpy.
I expect that this situation (first part in cache, latter part not in
cache) is rare - for reasonably small requests the common cases will be
"all cached" and "nothing cached". So perhaps the best approach here
is for samba to add special handling for the short read, to work out
the reason for its occurrence.
Alternatively we could add another flag to pread2() to select this
"throw away my data and return -EAGAIN" behaviour. Presumably
implemented with an i_size check, but it's gonna be racy.
I take it from your comments that nobody has actually wired up pread2()
into samba yet? That's a bit disturbing, because if we later want to
go and change something like this short-read behaviour, we're screwed -
it's a non back-compat userspace-visible change.
And a note on cosmetics: why are we using EAGAIN here rather than
EWOULDBLOCK? They have the same numerical value, but EWOULDBLOCK is a
better name - EAGAIN says "run it again", but that won't work.
Per definition EWOULDBLOCK seams like a better fit. Like you said
above it won't stop blocking unless you do something. I also did a
search in the kernel source (excluding drivers / sound directories)
use of EAGAIN (even in network code) is like 2 magnitudes bigger then
EWOULDBLOCK. In fact some places that grep found check for both
(although I'm sure it's optimized out).
Does anybody feel strongly about it being EWOULDBLOCK instead of
EAGAIN? Esp. since they are same on Linux? The convention (by numbers)
seams to favor EAGAIN.
--
Milosz Tanski
CTO
16 East 34th Street, 15th floor
New York, NY 10016
p: 646-253-9055
e: milosz@adfin.com
From: Andrew Morton <akpm@linux-foundation.org> Date: 2015-04-04 03:39:22
On Mon, 30 Mar 2015 13:26:25 -0700 Andrew Morton [off-list ref] wrote:
d) fincore() is more expensive
Actually, I kinda take that back. fincore() will be faster than
preadv2() in the case of a pagecache miss, and slower in the case of a
pagecache hit.
The breakpoint appears to be a hit rate of 30% - if fewer than 30% of
queries find the page in pagecache, fincore() will be faster than
preadv2().
This is because for a pagecache miss, fincore() will be about twice as
fast as preadv2(). For a pagecache hit, fincore()+pread() is 55%
slower than preadv2(). If there are lots of misses, fincore() is
faster overall.
Minimal fincore() implementation is below. It doesn't implement the
page_map!=NULL mode at all and will be slow for large areas - it needs
to be taught about radix_tree_for_each_*(). But it's good enough for
testing.
On a slow machine, in nanoseconds:
null syscall: 528
fincore (miss): 674
fincore (hit): 729
single byte pread: 1026
single byte preadv: 1134
pread() is a bit faster than preadv() and samba uses pread(), so the
implementations are:
if (fincore(fd, NULL, offset, len) == len)
pread();
else
punt();
if (preadv2(fd, ..., offset, len) == len)
...
else
punt();
fincore+pread, pagecache-hit: 1755ns
fincore+pread, pagecache-miss: 674ns
preadv(): 1134ns (preadv2() will be a little faster for misses)
Now, a pagecache hit rate of 30% sounds high so one would think that
fincore+pread is clearly ahead. But the pagecache hit rate in this
code will actually be quite high, because of readahead.
For a large linear read of a file which is perfectly laid out on disk
and is fully *uncached*, the hit rates will be as good as 99.8%,
because readahead is bringing in data in 2MB blobs.
In practice I expect that fincore()+pread() will be slower for linear
reads of medium to large files and faster for small files and seeky
accesses.
How much does all this matter? Not much. On a fast machine a
single-byte pread() takes 240ns. So if your server thread is handling
25000 requests/sec, we're only talking 0.6% overhead.
Note that we can trivially monitor the hit rate with either preadv2()
or fincore()+pread(): just count how many times all the data is there
versus how many times it isn't.
Also, note that we can use *both* fincore() and preadv2() to detect the
problematic page-just-disappeared race:
if (fincore(fd, NULL, offset, len) == len) {
if (preadv2(fd, offset, len) != len)
printf("race just happened");
It would be great if someone could apply the below, modify the
preadv2() callsite as above and determine under what conditions (if
any) the page-stealing race occurs.
arch/x86/syscalls/syscall_64.tbl | 1
include/linux/syscalls.h | 2
mm/Makefile | 2
mm/fincore.c | 65 +++++++++++++++++++++++++++++
4 files changed, 69 insertions(+), 1 deletion(-)
diff -puN arch/x86/syscalls/syscall_64.tbl~fincore arch/x86/syscalls/syscall_64.tbl