| Message ID | 20180529024025.58353-1-gthelen@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 28, 2018 at 07:40:25PM -0700, Greg Thelen wrote: > Reclaim priorities range from 0..12(DEF_PRIORITY). > scan_control.priority is a 4 byte int, which is overkill. > > Since commit 6538b8ea886e ("x86_64: expand kernel stack to 16K") x86_64 > stack overflows are not an issue. But it's inefficient to use 4 bytes > for priority. If you're looking to shave a few more bytes, allocation order can fit in a u8 too (can't be more than 6 bits, and realistically won't be more than 4 bits). reclaim_idx likewise will fit in a u8, and actually won't be more than 3 bits. I am sceptical that nr_to_reclaim should really be an unsigned long; I don't think we should be trying to free 4 billion pages in a single call. nr_scanned might be over 4 billion (!) but nr_reclaimed can probably shrink to unsigned int along with nr_to_reclaim.
Matthew Wilcox <willy@infradead.org> wrote: > On Mon, May 28, 2018 at 07:40:25PM -0700, Greg Thelen wrote: >> Reclaim priorities range from 0..12(DEF_PRIORITY). >> scan_control.priority is a 4 byte int, which is overkill. >> >> Since commit 6538b8ea886e ("x86_64: expand kernel stack to 16K") x86_64 >> stack overflows are not an issue. But it's inefficient to use 4 bytes >> for priority. > > If you're looking to shave a few more bytes, allocation order can fit > in a u8 too (can't be more than 6 bits, and realistically won't be more > than 4 bits). reclaim_idx likewise will fit in a u8, and actually won't > be more than 3 bits. Nod. Good tip. Included in ("[PATCH v2] mm: condense scan_control"). > I am sceptical that nr_to_reclaim should really be an unsigned long; I > don't think we should be trying to free 4 billion pages in a single call. > nr_scanned might be over 4 billion (!) but nr_reclaimed can probably > shrink to unsigned int along with nr_to_reclaim. Agreed. For patch simplicity, I'll pass on this for now.
diff --git a/mm/vmscan.c b/mm/vmscan.c index 9b697323a88c..541c334bd176 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -83,9 +83,6 @@ struct scan_control { */ struct mem_cgroup *target_mem_cgroup; - /* Scan (total_size >> priority) pages at once */ - int priority; - /* The highest zone to isolate pages for reclaim from */ enum zone_type reclaim_idx; @@ -111,6 +108,9 @@ struct scan_control { /* One of the zones is ready for compaction */ unsigned int compaction_ready:1; + /* Scan (total_size >> priority) pages at once */ + s8 priority; + /* Incremented by the number of inactive pages that were scanned */ unsigned long nr_scanned;
Reclaim priorities range from 0..12(DEF_PRIORITY). scan_control.priority is a 4 byte int, which is overkill. Since commit 6538b8ea886e ("x86_64: expand kernel stack to 16K") x86_64 stack overflows are not an issue. But it's inefficient to use 4 bytes for priority. Use s8 (signed byte) rather than u8 to allow for loops like: do { ... } while (--sc.priority >= 0); This reduces sizeof(struct scan_control) from 96 => 88 bytes (x86_64), which saves some stack. scan_control.priority field order is changed to occupy otherwise unused padding. Signed-off-by: Greg Thelen <gthelen@google.com> --- mm/vmscan.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)