Martin,
On Fri, Mar 22, 2019 at 11:46:11PM +0800, Martin Liu wrote:
As the discussion https://lore.kernel.org/patchwork/patch/334982/
We know an open file's ra_pages might run out of sync from
bdi.ra_pages since sequential, random or error read. Current design
is we have to ask users to reopen the file or use fdavise system
call to get it sync. However, we might have some cases to change
system wide file ra_pages to enhance system performance such as
enhance the boot time by increasing the ra_pages or decrease it to
Do you have examples that some distro making use of larger ra_pages
for boot time optimization?
There are fadvise() based readahead/preload tools that can be more
efficient for such purpose.
mitigate the trashing under memory pressure. Those are the cases
Suppose N read streams with equal read speed. The thrash-free memory
requirement would be (N * 2 * ra_pages).
If N=1000 and ra_pages=1MB, it'd require 2GB memory. Which looks
affordable in mainstream servers.
Also we have try_context_readahead() that's good at estimating the
thrashing free readahead size -- as long as the system load and
read speed remains stable, and node/zone reclaims are balanced.
For normal systems w/o huge number slow read streams, e.g. desktop,
reducing ra_pages may save a little memory by reducing readahead
misses, especially on mmap() read-around. Though at the risk of more
number of IOs. So I'd imagine desktops with 1GB or fewer memory
to really need worry about conservative readahead size.
we need to globally and immediately change this value. However,
Such requirement should be rare enough?
OTOH, the current Linux behavior of "changing ra_pages only changes
future file opens" does seem not a typical end user would expect and
understand.
from the discussion, we know bdi readahead tend to be an optimal
value to be a physical property of the storage and doesn't tend to
change dynamically. Thus we'd like to purpose readahead_shift into
backing device. This value is used as a shift bit number with bdi
readahead value to come out the readahead value and the negative
value means the division effect in the fixup function. The fixup
would only take effect when shift value is not zero;otherwise,
it returns file's ra_pages directly. Thus, an administrator could
use this value to control the amount readahead referred to
its backing device to achieve its goal.
Sorry but it sounds like introducing an unnecessarily twisted new
interface. I'm afraid it fixes the pain for 0.001% users while
bringing more puzzle to the majority others.
If we make up mind to fix the problem, it'd be more acceptable to
limit to an internal per-file data structure change, like this:
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -884,7 +884,7 @@ struct file_ra_state {
unsigned int async_size; /* do asynchronous readahead when
there are only # of pages ahead */
- unsigned int ra_pages; /* Maximum readahead window */
+ unsigned int ra_pages_shift;
unsigned int mmap_miss; /* Cache miss stat for mmap accesses */
loff_t prev_pos; /* Cache last read() position */
};
Then let fadvise() and shrink_readahead_size_eio() adjust that
per-file ra_pages_shift.
Cheers,
Fengguang
Signed-off-by: Martin Liu <liumartin@xxxxxxxxxx>
---
I'd like to get some feedback about the patch. If you have any
concern, please let me know. Thanks.
---
block/blk-sysfs.c | 28 ++++++++++++++++++++++++++++
include/linux/backing-dev-defs.h | 1 +
include/linux/backing-dev.h | 10 ++++++++++
mm/backing-dev.c | 27 +++++++++++++++++++++++++++
mm/filemap.c | 8 +++++---
mm/readahead.c | 2 +-
6 files changed, 72 insertions(+), 4 deletions(-)
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 59685918167e..f48b68e05d0e 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -107,6 +107,27 @@ queue_ra_store(struct request_queue *q, const char *page, size_t count)
return ret;
}
+static ssize_t queue_ra_shift_show(struct request_queue *q, char *page)
+{
+ int ra_shift = q->backing_dev_info->read_ahead_shift;
+
+ return scnprintf(page, PAGE_SIZE - 1, "%d\n", ra_shift);
+}
+
+static ssize_t queue_ra_shift_store(struct request_queue *q, const char *page,
+ size_t count)
+{
+ s8 ra_shift;
+ ssize_t ret = kstrtos8(page, 10, &ra_shift);
+
+ if (ret)
+ return ret;
+
+ q->backing_dev_info->read_ahead_shift = ra_shift;
+
+ return count;
+}
+
static ssize_t queue_max_sectors_show(struct request_queue *q, char *page)
{
int max_sectors_kb = queue_max_sectors(q) >> 1;
@@ -540,6 +561,12 @@ static struct queue_sysfs_entry queue_ra_entry = {
.store = queue_ra_store,
};
+static struct queue_sysfs_entry queue_ra_shift_entry = {
+ .attr = {.name = "read_ahead_shift", .mode = 0644 },
+ .show = queue_ra_shift_show,
+ .store = queue_ra_shift_store,
+};
+
static struct queue_sysfs_entry queue_max_sectors_entry = {
.attr = {.name = "max_sectors_kb", .mode = 0644 },
.show = queue_max_sectors_show,
@@ -729,6 +756,7 @@ static struct queue_sysfs_entry throtl_sample_time_entry = {
static struct attribute *default_attrs[] = {
&queue_requests_entry.attr,
&queue_ra_entry.attr,
+ &queue_ra_shift_entry.attr,
&queue_max_hw_sectors_entry.attr,
&queue_max_sectors_entry.attr,
&queue_max_segments_entry.attr,
diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index 07e02d6df5ad..452002df5232 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -167,6 +167,7 @@ struct bdi_writeback {
struct backing_dev_info {
struct list_head bdi_list;
unsigned long ra_pages; /* max readahead in PAGE_SIZE units */
+ s8 read_ahead_shift; /* shift to ra_pages */
unsigned long io_pages; /* max allowed IO size */
congested_fn *congested_fn; /* Function pointer if device is md/dm */
void *congested_data; /* Pointer to aux data for congested func */
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index f9b029180241..23f755fe386e 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -221,6 +221,16 @@ static inline int bdi_sched_wait(void *word)
return 0;
}
+static inline unsigned long read_ahead_fixup(struct file *file)
+{
+ unsigned long ra = file->f_ra.ra_pages;
+ s8 shift = (inode_to_bdi(file->f_mapping->host))->read_ahead_shift;
+
+ ra = (shift >= 0) ? ra << shift : ra >> abs(shift);
+
+ return ra;
+}
+
#ifdef CONFIG_CGROUP_WRITEBACK
struct bdi_writeback_congested *
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 72e6d0c55cfa..1b5163a5ead5 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -172,6 +172,32 @@ static DEVICE_ATTR_RW(name);
BDI_SHOW(read_ahead_kb, K(bdi->ra_pages))
+static ssize_t read_ahead_shift_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct backing_dev_info *bdi = dev_get_drvdata(dev);
+ s8 read_ahead_shift;
+ ssize_t ret = kstrtos8(buf, 10, &read_ahead_shift);
+
+ if (ret)
+ return ret;
+
+ bdi->read_ahead_shift = read_ahead_shift;
+
+ return count;
+}
+
+static ssize_t read_ahead_shift_show(struct device *dev,
+ struct device_attribute *attr, char *page)
+{
+ struct backing_dev_info *bdi = dev_get_drvdata(dev);
+
+ return scnprintf(page, PAGE_SIZE - 1, "%d\n", bdi->read_ahead_shift);
+}
+
+static DEVICE_ATTR_RW(read_ahead_shift);
+
static ssize_t min_ratio_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
@@ -223,6 +249,7 @@ static DEVICE_ATTR_RO(stable_pages_required);
static struct attribute *bdi_dev_attrs[] = {
&dev_attr_read_ahead_kb.attr,
+ &dev_attr_read_ahead_shift.attr,
&dev_attr_min_ratio.attr,
&dev_attr_max_ratio.attr,
&dev_attr_stable_pages_required.attr,
diff --git a/mm/filemap.c b/mm/filemap.c
index d78f577baef2..cd8d0e00ff93 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2469,6 +2469,7 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf)
struct address_space *mapping = file->f_mapping;
struct file *fpin = NULL;
pgoff_t offset = vmf->pgoff;
+ unsigned long max;
/* If we don't want any read-ahead, don't bother */
if (vmf->vma->vm_flags & VM_RAND_READ)
@@ -2498,9 +2499,10 @@ static struct file *do_sync_mmap_readahead(struct vm_fault *vmf)
* mmap read-around
*/
fpin = maybe_unlock_mmap_for_io(vmf, fpin);
- ra->start = max_t(long, 0, offset - ra->ra_pages / 2);
- ra->size = ra->ra_pages;
- ra->async_size = ra->ra_pages / 4;
+ max = read_ahead_fixup(file);
+ ra->start = max_t(long, 0, offset - max / 2);
+ ra->size = max;
+ ra->async_size = max / 4;
ra_submit(ra, mapping, file);
return fpin;
}
diff --git a/mm/readahead.c b/mm/readahead.c
index a4593654a26c..546bf344fc4d 100644
--- a/mm/readahead.c
+++ b/mm/readahead.c
@@ -384,7 +384,7 @@ ondemand_readahead(struct address_space *mapping,
unsigned long req_size)
{
struct backing_dev_info *bdi = inode_to_bdi(mapping->host);
- unsigned long max_pages = ra->ra_pages;
+ unsigned long max_pages = read_ahead_fixup(filp);
unsigned long add_pages;
pgoff_t prev_offset;
--
2.21.0.392.gf8f6787159e-goog
