Re: [PATCH 16/16] mm: strictlimit feature
From: Andrew Morton <akpm@linux-foundation.org>
Date: 2013-07-01 21:16:12
Also in:
linux-fsdevel, lkml
On Sat, 29 Jun 2013 21:48:54 +0400 Maxim Patlasov [off-list ref] wrote:
From: Miklos Szeredi <redacted> The feature prevents mistrusted filesystems to grow a large number of dirty pages before throttling. For such filesystems balance_dirty_pages always check bdi counters against bdi limits. I.e. even if global "nr_dirty" is under "freerun", it's not allowed to skip bdi checks. The only use case for now is fuse: it sets bdi max_ratio to 1% by default and system administrators are supposed to expect that this limit won't be exceeded. The feature is on if address space is marked by AS_STRICTLIMIT flag. A filesystem may set the flag when it initializes a new inode.
Fengguang, could you please review this patch? I suggest you await the next version, which hopefully will be more reviewable...
quoted hunk ↗ jump to hunk
...--- a/include/linux/backing-dev.h +++ b/include/linux/backing-dev.h@@ -33,6 +33,8 @@ enum bdi_state { BDI_sync_congested, /* The sync queue is getting full */ BDI_registered, /* bdi_register() was done */ BDI_writeback_running, /* Writeback is in progress */ + BDI_idle, /* No pages under writeback at the moment of + * last update of write bw */
Why does BDI_idle exist?
quoted hunk ↗ jump to hunk
BDI_unused, /* Available bits start here */ };@@ -43,6 +45,7 @@ enum bdi_stat_item { BDI_WRITEBACK, BDI_DIRTIED, BDI_WRITTEN, + BDI_WRITTEN_BACK, NR_BDI_STAT_ITEMS };@@ -76,6 +79,8 @@ struct backing_dev_info { unsigned long bw_time_stamp; /* last time write bw is updated */ unsigned long dirtied_stamp; unsigned long written_stamp; /* pages written at bw_time_stamp */ + unsigned long writeback_stamp; /* pages sent to writeback at + * bw_time_stamp */
Well this sucks. Some of the "foo_stamp" fields are in units of time (jiffies? We aren't told) and some of the "foo_stamp" fields are in units of number-of-pages. It would be good to fix the naming here.
quoted hunk ↗ jump to hunk
unsigned long write_bandwidth; /* the estimated write bandwidth */ unsigned long avg_write_bandwidth; /* further smoothed write bw */diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index e3dea75..baac702 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h@@ -25,6 +25,7 @@ enum mapping_flags { AS_MM_ALL_LOCKS = __GFP_BITS_SHIFT + 2, /* under mm_take_all_locks() */ AS_UNEVICTABLE = __GFP_BITS_SHIFT + 3, /* e.g., ramdisk, SHM_LOCK */ AS_BALLOON_MAP = __GFP_BITS_SHIFT + 4, /* balloon page special map */ + AS_STRICTLIMIT = __GFP_BITS_SHIFT + 5, /* strict dirty limit */
Thing is, "strict dirty limit" isn't documented anywhere, so this reference is left dangling.
quoted hunk ↗ jump to hunk
...--- a/mm/backing-dev.c +++ b/mm/backing-dev.c@@ -94,6 +94,7 @@ static int bdi_debug_stats_show(struct seq_file *m, void *v) "BackgroundThresh: %10lu kB\n" "BdiDirtied: %10lu kB\n" "BdiWritten: %10lu kB\n" + "BdiWrittenBack: %10lu kB\n" "BdiWriteBandwidth: %10lu kBps\n" "b_dirty: %10lu\n" "b_io: %10lu\n"
I can't imagine what the difference is between BdiWritten and BdiWrittenBack. I suggest you document this at the BDI_WRITTEN_BACK definition site in enum bdi_stat_item. BDI_WRITTEN (at least) will also need documentation so people can understand the difference.
quoted hunk ↗ jump to hunk
...@@ -679,29 +711,31 @@ static unsigned long bdi_position_ratio(struct backing_dev_info *bdi, if (unlikely(dirty >= limit)) return 0; + if (unlikely(strictlimit)) { + if (bdi_dirty < 8) + return 2 << RATELIMIT_CALC_SHIFT; + + if (bdi_dirty >= bdi_thresh) + return 0; + + bdi_setpoint = bdi_thresh + bdi_dirty_limit(bdi, bg_thresh); + bdi_setpoint /= 2; + + if (bdi_setpoint == 0 || bdi_setpoint == bdi_thresh) + return 0; + + pos_ratio = pos_ratio_polynom(bdi_setpoint, bdi_dirty, + bdi_thresh); + return min_t(long long, pos_ratio, 2 << RATELIMIT_CALC_SHIFT); + }
This would be a suitable site at which to document the strictlimit feature. What it is, how it works and most importantly, why it exists.
quoted hunk ↗ jump to hunk
...@@ -994,6 +1029,16 @@ static void bdi_update_dirty_ratelimit(struct backing_dev_info *bdi, * keep that period small to reduce time lags). */ step = 0; + + if (unlikely(strictlimit)) { + dirty = bdi_dirty; + if (bdi_dirty < 8) + setpoint = bdi_dirty + 1; + else + setpoint = (bdi_thresh + + bdi_dirty_limit(bdi, bg_thresh)) / 2; + }
Explain this to the reader, please.
...
-- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>