Changelog since V1
o Rebase on top of mmotm
o Use atomic_t for memalloc_socks (David Miller)
o Remove use of sk_memalloc_socks in vmscan (Neil Brown)
o Check throttle within prepare_to_wait (Neil Brown)
o Add statistics on throttling instead of printk
Swapping over NBD is something that is technically possible but not
often advised. While there are number of guides on the internet
on how to configure it and nbd-client supports a -swap switch to
"prevent deadlocks", the fact of the matter is a machine using NBD
for swap can be locked up within minutes if swap is used intensively.
The problem is that network block devices do not use mempools like
normal block devices do. As the host cannot control where they receive
packets from, they cannot reliably work out in advance how much memory
they might need.
Some years ago, Peter Ziljstra developed a series of patches that
supported swap over an NFS that some distributions are carrying in
their kernels. This patch series borrows very heavily from Peter's work
to support swapping over NBD (the relatively straight-forward case)
and uses throttling instead of dynamically resized memory reserves
so the series is not too unwieldy for review.
Patch 1 serialises access to min_free_kbytes. It's not strictly needed
by this series but as the series cares about watermarks in
general, it's a harmless fix. It could be merged independently.
Patch 2 adds knowledge of the PFMEMALLOC reserves to SLAB and SLUB to
preserve access to pages allocated under low memory situations
to callers that are freeying memory.
Patch 3 introduces __GFP_MEMALLOC to allow access to the PFMEMALLOC
reserves without setting PFMEMALLOC.
Patch 4 opens the possibility for softirqs to use PFMEMALLOC reserves
for later use by network packet processing.
Patch 5 ignores memory policies when ALLOC_NO_WATERMARKS is set.
Patches 6-9 allows network processing to use PFMEMALLOC reserves when
the socket has been marked as being used by the VM to clean
pages. If packets are received and stored in pages that were
allocated under low-memory situations and are unrelated to
the VM, the packets are dropped.
Patch 10 is a micro-optimisation to avoid a function call in the
common case.
Patch 11 tags NBD sockets as being SOCK_MEMALLOC so they can use
PFMEMALLOC if necessary.
Patch 12 notes that it is still possible for the PFMEMALLOC reserve
to be depleted. To prevent this, direct reclaimers get
throttled on a waitqueue if 50% of the PFMEMALLOC reserves are
depleted. It is expected that kswapd and the direct reclaimers
already running will clean enough pages for the low watermark
to be reached and the throttled processes are woken up.
Patch 13 adds a statistic to track how often processes get throttled
Some basic performance testing was run using kernel builds, netperf
on loopback for UDP and TCP, hackbench (pipes and sockets), iozone
and sysbench. Each of them were expected to use the sl*b allocators
reasonably heavily but there did not appear to be significant
performance variances. Here is the results from netperf using
slab as an example
NETPERF UDP
netperf-udp udp-swapnbd
vanilla-slab v1r17-slab
64 178.06 ( 0.00%)* 189.46 ( 6.02%)
1.02% 1.00%
128 355.06 ( 0.00%) 370.75 ( 4.23%)
256 662.47 ( 0.00%) 721.62 ( 8.20%)
1024 2229.39 ( 0.00%) 2567.04 (13.15%)
2048 3974.20 ( 0.00%) 4114.70 ( 3.41%)
3312 5619.89 ( 0.00%) 5800.09 ( 3.11%)
4096 6460.45 ( 0.00%) 6702.45 ( 3.61%)
8192 9580.24 ( 0.00%) 9927.97 ( 3.50%)
16384 13259.14 ( 0.00%) 13493.88 ( 1.74%)
MMTests Statistics: duration
User/Sys Time Running Test (seconds) 2960.17 2540.14
Total Elapsed Time (seconds) 3554.10 3050.10
NETPERF TCP
netperf-tcp tcp-swapnbd
vanilla-slab v1r17-slab
64 1230.29 ( 0.00%) 1273.17 ( 3.37%)
128 2309.97 ( 0.00%) 2375.22 ( 2.75%)
256 3659.32 ( 0.00%) 3704.87 ( 1.23%)
1024 7267.80 ( 0.00%) 7251.02 (-0.23%)
2048 8358.26 ( 0.00%) 8204.74 (-1.87%)
3312 8631.07 ( 0.00%) 8637.62 ( 0.08%)
4096 8770.95 ( 0.00%) 8704.08 (-0.77%)
8192 9749.33 ( 0.00%) 9769.06 ( 0.20%)
16384 11151.71 ( 0.00%) 11135.32 (-0.15%)
MMTests Statistics: duration
User/Sys Time Running Test (seconds) 1245.04 1619.89
Total Elapsed Time (seconds) 1250.66 1622.18
Here is the equivalent test for SLUB
NETPERF UDP
netperf-udp udp-swapnbd
vanilla-slub v1r17-slub
64 180.83 ( 0.00%) 183.68 ( 1.55%)
128 357.29 ( 0.00%) 367.11 ( 2.67%)
256 679.64 ( 0.00%)* 724.03 ( 6.13%)
1.15% 1.00%
1024 2343.40 ( 0.00%)* 2610.63 (10.24%)
1.68% 1.00%
2048 3971.53 ( 0.00%) 4102.21 ( 3.19%)*
1.00% 1.40%
3312 5677.04 ( 0.00%) 5748.69 ( 1.25%)
4096 6436.75 ( 0.00%) 6549.41 ( 1.72%)
8192 9698.56 ( 0.00%) 9808.84 ( 1.12%)
16384 13337.06 ( 0.00%) 13404.38 ( 0.50%)
MMTests Statistics: duration
User/Sys Time Running Test (seconds) 2880.15 2180.13
Total Elapsed Time (seconds) 3458.10 2618.09
NETPERF TCP
netperf-tcp tcp-swapnbd
vanilla-slub v1r17-slub
64 1256.79 ( 0.00%) 1287.32 ( 2.37%)
128 2308.71 ( 0.00%) 2371.09 ( 2.63%)
256 3672.03 ( 0.00%) 3771.05 ( 2.63%)
1024 7245.08 ( 0.00%) 7261.60 ( 0.23%)
2048 8315.17 ( 0.00%) 8244.14 (-0.86%)
3312 8611.43 ( 0.00%) 8616.90 ( 0.06%)
4096 8711.64 ( 0.00%) 8695.97 (-0.18%)
8192 9795.71 ( 0.00%) 9774.11 (-0.22%)
16384 11145.48 ( 0.00%) 11225.70 ( 0.71%)
MMTests Statistics: duration
User/Sys Time Running Test (seconds) 1345.05 1425.06
Total Elapsed Time (seconds) 1350.61 1430.66
Time to completion varied a lot but this can happen with netperf as
it tries to find results within a sufficiently high confidence. I
wouldn't read too much into the performance gains of netperf-udp
as it can sometimes be affected by code just shuffling around for
whatever reason.
For testing swap-over-NBD, a machine was booted with 2G of RAM with a
swapfile backed by NBD. 16*NUM_CPU processes were started that create
anonymous memory mappings and read them linearly in a loop. The total
size of the mappings were 4*PHYSICAL_MEMORY to use swap heavily under
memory pressure. Without the patches, the machine locks up within
minutes and runs to completion with them applied.
Comments?
drivers/block/nbd.c | 7 +-
include/linux/gfp.h | 7 +-
include/linux/mm_types.h | 8 ++
include/linux/mmzone.h | 1 +
include/linux/sched.h | 7 ++
include/linux/skbuff.h | 19 +++-
include/linux/slub_def.h | 1 +
include/linux/vm_event_item.h | 1 +
include/net/sock.h | 19 ++++
kernel/softirq.c | 3 +
mm/page_alloc.c | 57 ++++++++--
mm/slab.c | 240 +++++++++++++++++++++++++++++++++++------
mm/slub.c | 35 +++++-
mm/vmscan.c | 58 ++++++++++
mm/vmstat.c | 1 +
net/core/dev.c | 52 ++++++++-
net/core/filter.c | 8 ++
net/core/skbuff.c | 95 ++++++++++++++---
net/core/sock.c | 42 +++++++
net/ipv4/tcp.c | 3 +-
net/ipv4/tcp_output.c | 13 ++-
net/ipv6/tcp_ipv6.c | 12 ++-
22 files changed, 603 insertions(+), 86 deletions(-)
--
1.7.3.4
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
There is a race between the min_free_kbytes sysctl, memory hotplug
and transparent hugepage support enablement. Memory hotplug uses a
zonelists_mutex to avoid a race when building zonelists. Reuse it to
serialise watermark updates.
[a.p.zijlstra@chello.nl: Older patch fixed the race with spinlock]
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
mm/page_alloc.c | 23 +++++++++++++++--------
1 files changed, 15 insertions(+), 8 deletions(-)
--
1.7.3.4
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Getting and putting objects in SLAB currently requires a function call
but the bulk of the work is related to PFMEMALLOC reserves which are
only consumed when network-backed storage is critical. Use an inline
function to determine if the function call is required.
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
mm/slab.c | 28 ++++++++++++++++++++++++++--
1 files changed, 26 insertions(+), 2 deletions(-)
--
1.7.3.4
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
In order to make sure pfmemalloc packets receive all memory needed to proceed,
ensure processing of pfmemalloc SKBs happens under PF_MEMALLOC. This is
limited to a subset of protocols that are expected to be used for writing
to swap. Taps are not allowed to use PF_MEMALLOC as these are expected to
communicate with userspace processes which could be paged out.
[a.p.zijlstra@chello.nl: Ideas taken from various patches]
[jslaby@suse.cz: Lock imbalance fix]
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/net/sock.h | 5 +++++
net/core/dev.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++----
net/core/sock.c | 16 ++++++++++++++++
3 files changed, 69 insertions(+), 4 deletions(-)
@@ -3104,15 +3125,28 @@ static int __netif_receive_skb(struct sk_buff *skb)booldeliver_exact=false;intret=NET_RX_DROP;__be16type;+unsignedlongpflags=current->flags;if(!netdev_tstamp_prequeue)net_timestamp_check(skb);trace_netif_receive_skb(skb);+/*+*PFMEMALLOCskbsarespecial,theyshould+*-bedeliveredtoSOCK_MEMALLOCsocketsonly+*-stayawayfromuserspace+*-haveboundedmemoryusage+*+*UsePF_MEMALLOCasthissavesusfrompropagatingtheallocation+*contextdowntoallallocationsites.+*/+if(skb_pfmemalloc(skb))+current->flags|=PF_MEMALLOC;+/* if we've gotten here through NAPI, check netpoll */if(netpoll_receive_skb(skb))-returnNET_RX_DROP;+gotoout;if(!skb->skb_iif)skb->skb_iif=skb->dev->ifindex;
@@ -3210,6 +3251,7 @@ ncls:if(pt_prev){ret=pt_prev->func(skb,skb->dev,pt_prev,orig_dev);}else{+drop:atomic_long_inc(&skb->dev->rx_dropped);kfree_skb(skb);/* Jamal, now you will not able to escape explaining
@@ -245,6 +245,22 @@ void sk_clear_memalloc(struct sock *sk)}EXPORT_SYMBOL_GPL(sk_clear_memalloc);+int__sk_backlog_rcv(structsock*sk,structsk_buff*skb)+{+intret;+unsignedlongpflags=current->flags;++/* these should have been dropped before queueing */+BUG_ON(!sock_flag(sk,SOCK_MEMALLOC));++current->flags|=PF_MEMALLOC;+ret=sk->sk_backlog_rcv(sk,skb);+tsk_restore_flags(current,pflags,PF_MEMALLOC);++returnret;+}+EXPORT_SYMBOL(__sk_backlog_rcv);+#if defined(CONFIG_CGROUPS) && !defined(CONFIG_NET_CLS_CGROUP)intnet_cls_subsys_id=-1;EXPORT_SYMBOL_GPL(net_cls_subsys_id);
--
1.7.3.4
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Set SOCK_MEMALLOC on the NBD socket to allow access to PFMEMALLOC
reserves so pages backed by NBD, particularly if swap related,
can be cleaned to prevent the machine being deadlocked. It is
still possible that the PFMEMALLOC reserves get depleted resulting
in deadlock but this can be resolved by the administrator by
increasing min_free_kbytes.
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
drivers/block/nbd.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
@@ -156,6 +156,7 @@ static int sock_xmit(struct nbd_device *lo, int send, void *buf, int size,structmsghdrmsg;structkveciov;sigset_tblocked,oldset;+unsignedlongpflags=current->flags;if(unlikely(!sock)){printk(KERN_ERR"%s: Attempted %s on closed socket in sock_xmit\n",
@@ -168,8 +169,9 @@ static int sock_xmit(struct nbd_device *lo, int send, void *buf, int size,siginitsetinv(&blocked,sigmask(SIGKILL));sigprocmask(SIG_SETMASK,&blocked,&oldset);+current->flags|=PF_MEMALLOC;do{-sock->sk->sk_allocation=GFP_NOIO;+sock->sk->sk_allocation=GFP_NOIO|__GFP_MEMALLOC;iov.iov_base=buf;iov.iov_len=size;msg.msg_name=NULL;
@@ -214,6 +216,7 @@ static int sock_xmit(struct nbd_device *lo, int send, void *buf, int size,}while(size>0);sigprocmask(SIG_SETMASK,&oldset,NULL);+tsk_restore_flags(current,pflags,PF_MEMALLOC);returnresult;}
@@ -404,6 +407,8 @@ static int nbd_do_it(struct nbd_device *lo)BUG_ON(lo->magic!=LO_MAGIC);+sk_set_memalloc(lo->sock->sk);+lo->pid=current->pid;ret=sysfs_create_file(&disk_to_dev(lo->disk)->kobj,&pid_attr.attr);if(ret){
--
1.7.3.4
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
If swap is backed by network storage such as NBD, there is a risk that a
large number of reclaimers can hang the system by consuming all
PF_MEMALLOC reserves. To avoid these hangs, the administrator must tune
min_free_kbytes in advance. This patch will throttle direct reclaimers
if half the PF_MEMALLOC reserves are in use as the system is at risk of
hanging.
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/linux/mmzone.h | 1 +
mm/page_alloc.c | 1 +
mm/vmscan.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+), 0 deletions(-)
@@ -2117,6 +2117,55 @@ out:return0;}+staticboolpfmemalloc_watermark_ok(pg_data_t*pgdat,inthigh_zoneidx)+{+structzone*zone;+unsignedlongpfmemalloc_reserve=0;+unsignedlongfree_pages=0;+inti;++for(i=0;i<=high_zoneidx;i++){+zone=&pgdat->node_zones[i];+pfmemalloc_reserve+=min_wmark_pages(zone);+free_pages+=zone_page_state(zone,NR_FREE_PAGES);+}++return(free_pages>pfmemalloc_reserve/2)?true:false;+}++/*+*Throttledirectreclaimersifbackingstorageisbackedbythenetwork+*andthePFMEMALLOCreserveforthepreferrednodeisgettingdangerously+*depleted.kswapdwillcontinuetomakeprogressandwaketheprocesses+*whenthelowwatermarkisreached+*/+staticvoidthrottle_direct_reclaim(gfp_tgfp_mask,structzonelist*zonelist,+nodemask_t*nodemask)+{+structzone*zone;+inthigh_zoneidx=gfp_zone(gfp_mask);+DEFINE_WAIT(wait);++/* Check if the pfmemalloc reserves are ok */+first_zones_zonelist(zonelist,high_zoneidx,NULL,&zone);+prepare_to_wait(&zone->zone_pgdat->pfmemalloc_wait,&wait,+TASK_INTERRUPTIBLE);+if(pfmemalloc_watermark_ok(zone->zone_pgdat,high_zoneidx))+gotoout;++/* Throttle */+do{+schedule();+finish_wait(&zone->zone_pgdat->pfmemalloc_wait,&wait);+prepare_to_wait(&zone->zone_pgdat->pfmemalloc_wait,&wait,+TASK_INTERRUPTIBLE);+}while(!pfmemalloc_watermark_ok(zone->zone_pgdat,high_zoneidx)&&+!fatal_signal_pending(current));++out:+finish_wait(&zone->zone_pgdat->pfmemalloc_wait,&wait);+}+unsignedlongtry_to_free_pages(structzonelist*zonelist,intorder,gfp_tgfp_mask,nodemask_t*nodemask){
@@ -2133,6 +2182,8 @@ unsigned long try_to_free_pages(struct zonelist *zonelist, int order,.nodemask=nodemask,};+throttle_direct_reclaim(gfp_mask,zonelist,nodemask);+trace_mm_vmscan_direct_reclaim_begin(order,sc.may_writepage,gfp_mask);
@@ -2488,6 +2539,12 @@ loop_again:}}++/* Wake throttled direct reclaimers if low watermark is met */+if(waitqueue_active(&pgdat->pfmemalloc_wait)&&+pfmemalloc_watermark_ok(pgdat,MAX_NR_ZONES-1))+wake_up_interruptible(&pgdat->pfmemalloc_wait);+if(all_zones_ok||(order&&pgdat_balanced(pgdat,balanced,*classzone_idx)))break;/* kswapd: all done *//*
--
1.7.3.4
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Change the skb allocation API to indicate RX usage and use this to fall back
to the PFMEMALLOC reserve when needed. SKBs allocated from the reserve are
tagged in skb->pfmemalloc. If an SKB is allocated from the reserve and
the socket is later found to be unrelated to page reclaim, the packet is
dropped so that the memory remains available for page reclaim. Network
protocols are expected to recover from this packet loss.
[a.p.zijlstra@chello.nl: Ideas taken from various patches]
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/linux/gfp.h | 3 ++
include/linux/skbuff.h | 19 ++++++++--
include/net/sock.h | 6 +++
mm/internal.h | 3 --
net/core/filter.c | 8 ++++
net/core/skbuff.c | 95 ++++++++++++++++++++++++++++++++++++++++--------
net/core/sock.c | 4 ++
7 files changed, 116 insertions(+), 22 deletions(-)
@@ -376,6 +376,9 @@ void drain_local_pages(void *dummy);externgfp_tgfp_allowed_mask;+/* Returns true if the gfp_mask allows use of ALLOC_NO_WATERMARK */+boolgfp_pfmemalloc_allowed(gfp_tgfp_mask);+externvoidpm_restrict_gfp_mask(void);externvoidpm_restore_gfp_mask(void);
@@ -146,6 +146,43 @@ static void skb_under_panic(struct sk_buff *skb, int sz, void *here)BUG();}++/*+*kmalloc_reserveisawrapperaroundkmalloc_node_track_callerthattells+*thecallerifemergencypfmemallocreservesarebeingused.Ifitisand+*thesocketislaterfoundtobeSOCK_MEMALLOCthenPFMEMALLOCreserves+*maybeused.Otherwise,thepacketdatamaybediscardeduntilenough+*memoryisfree+*/+#define kmalloc_reserve(size, gfp, node, pfmemalloc) \+__kmalloc_reserve(size,gfp,node,_RET_IP_,pfmemalloc)+void*__kmalloc_reserve(size_tsize,gfp_tflags,intnode,unsignedlongip,+bool*pfmemalloc)+{+void*obj;+boolret_pfmemalloc=false;++/*+*Tryaregularallocation,whenthatfailsandwe'renotentitled+*tothereserves,fail.+*/+obj=kmalloc_node_track_caller(size,+flags|__GFP_NOMEMALLOC|__GFP_NOWARN,+node);+if(obj||!(gfp_pfmemalloc_allowed(flags)))+gotoout;++/* Try again but now we are using pfmemalloc reserves */+ret_pfmemalloc=true;+obj=kmalloc_node_track_caller(size,flags,node);++out:+if(pfmemalloc)+*pfmemalloc=ret_pfmemalloc;++returnobj;+}+/* Allocate a new skbuff. We do this ourselves so we can fill in a few*'private'fieldsandalsodomemorystatisticstofindallthe*[BEEP]leaks.
@@ -168,14 +207,19 @@ static void skb_under_panic(struct sk_buff *skb, int sz, void *here)*%GFP_ATOMIC.*/structsk_buff*__alloc_skb(unsignedintsize,gfp_tgfp_mask,-intfclone,intnode)+intflags,intnode){structkmem_cache*cache;structskb_shared_info*shinfo;structsk_buff*skb;u8*data;+boolpfmemalloc;++cache=(flags&SKB_ALLOC_FCLONE)+?skbuff_fclone_cache:skbuff_head_cache;-cache=fclone?skbuff_fclone_cache:skbuff_head_cache;+if(sk_memalloc_socks()&&(flags&SKB_ALLOC_RX))+gfp_mask|=__GFP_MEMALLOC;/* Get the HEAD */skb=kmem_cache_alloc_node(cache,gfp_mask&~__GFP_DMA,node);
@@ -802,7 +862,10 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,gotoadjust_others;}-data=kmalloc(size+sizeof(structskb_shared_info),gfp_mask);+if(skb_pfmemalloc(skb))+gfp_mask|=__GFP_MEMALLOC;+data=kmalloc_reserve(size+sizeof(structskb_shared_info),gfp_mask,+NUMA_NO_NODE,NULL);if(!data)gotonodata;
--
1.7.3.4
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Under significant pressure when writing back to network-backed storage,
direct reclaimers may get throttled. This is expected to be a
short-lived event and the processes get woken up again but processes do
get stalled. This patch counts how many times such stalling occurs. It's
up to the administrator whether to reduce these stalls by increasing
min_free_kbytes.
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/linux/vm_event_item.h | 1 +
mm/vmscan.c | 1 +
mm/vmstat.c | 1 +
3 files changed, 3 insertions(+), 0 deletions(-)
Allow specific sockets to be tagged SOCK_MEMALLOC and use __GFP_MEMALLOC
for their allocations. These sockets will be able to go below watermarks
and allocate from the emergency reserve. Such sockets are to be used
to service the VM (iow. to swap over). They must be handled kernel side,
exposing such a socket to user-space is a bug.
There is a risk that the reserves be depleted so for now, the administrator is
responsible for increasing min_free_kbytes as necessary to prevent deadlock
for their workloads.
[a.p.zijlstra@chello.nl: Original patches]
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/net/sock.h | 5 ++++-
net/core/sock.c | 22 ++++++++++++++++++++++
2 files changed, 26 insertions(+), 1 deletions(-)
--
1.7.3.4
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Introduce sk_allocation(), this function allows to inject sock specific
flags to each sock related allocation. It is only used on allocation
paths that may be required for writing pages back to network storage.
Signed-off-by: Peter Zijlstra <redacted>
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/net/sock.h | 5 +++++
net/ipv4/tcp.c | 3 ++-
net/ipv4/tcp_output.c | 13 +++++++------
net/ipv6/tcp_ipv6.c | 12 +++++++++---
4 files changed, 23 insertions(+), 10 deletions(-)
@@ -698,7 +698,8 @@ struct sk_buff *sk_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp)/* The TCP header must be at least 32-bit aligned. */size=ALIGN(size,4);-skb=alloc_skb_fclone(size+sk->sk_prot->max_header,gfp);+skb=alloc_skb_fclone(size+sk->sk_prot->max_header,+sk_allocation(sk,gfp));if(skb){if(sk_wmem_schedule(sk,skb->truesize)){/*
@@ -2734,7 +2735,7 @@ void tcp_send_ack(struct sock *sk)/* Send it off, this clears delayed acks for us. */TCP_SKB_CB(buff)->when=tcp_time_stamp;-tcp_transmit_skb(sk,buff,0,GFP_ATOMIC);+tcp_transmit_skb(sk,buff,0,sk_allocation(sk,GFP_ATOMIC));}/* This routine sends a packet with an out of date sequence
@@ -2754,7 +2755,7 @@ static int tcp_xmit_probe_skb(struct sock *sk, int urgent)structsk_buff*skb;/* We don't queue it, tcp_transmit_skb() sets ownership. */-skb=alloc_skb(MAX_TCP_HEADER,GFP_ATOMIC);+skb=alloc_skb(MAX_TCP_HEADER,sk_allocation(sk,GFP_ATOMIC));if(skb==NULL)return-1;
@@ -597,7 +597,8 @@ static int tcp_v6_md5_do_add(struct sock *sk, struct in6_addr *peer,}else{/* reallocate new list if current one is full. */if(!tp->md5sig_info){-tp->md5sig_info=kzalloc(sizeof(*tp->md5sig_info),GFP_ATOMIC);+tp->md5sig_info=kzalloc(sizeof(*tp->md5sig_info),+sk_allocation(sk,GFP_ATOMIC));if(!tp->md5sig_info){kfree(newkey);return-ENOMEM;
--
1.7.3.4
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
The reserve is proportionally distributed over all !highmem zones in the
system. So we need to allow an emergency allocation access to all zones.
In order to do that we need to break out of any mempolicy boundaries we
might have.
In my opinion that does not break mempolicies as those are user oriented
and not system oriented. That is, system allocations are not guaranteed to
be within mempolicy boundaries. For instance IRQs don't even have a mempolicy.
So breaking out of mempolicy boundaries for 'rare' emergency allocations,
which are always system allocations (as opposed to user) is ok.
Signed-off-by: Peter Zijlstra <redacted>
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
mm/page_alloc.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
@@ -2085,6 +2085,13 @@ restart:rebalance:/* Allocate without watermarks if the context allows */if(alloc_flags&ALLOC_NO_WATERMARKS){+/*+*IgnoremempoliciesifALLOC_NO_WATERMARKSonthegrounds+*theallocationishighpriorityandthesetypeof+*allocationsaresystemratherthanuserorientated+*/+zonelist=node_zonelist(numa_node_id(),gfp_mask);+page=__alloc_pages_high_priority(gfp_mask,order,zonelist,high_zoneidx,nodemask,preferred_zone,migratetype);
--
1.7.3.4
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
This is needed to allow network softirq packet processing to make use
of PF_MEMALLOC.
Currently softirq context cannot use PF_MEMALLOC due to it not being
associated with a task, and therefore not having task flags to fiddle with -
thus the gfp to alloc flag mapping ignores the task flags when in interrupts
(hard or soft) context.
Allowing softirqs to make use of PF_MEMALLOC therefore requires some trickery.
We basically borrow the task flags from whatever process happens to be
preempted by the softirq.
So we modify the gfp to alloc flags mapping to not exclude task flags in
softirq context, and modify the softirq code to save, clear and restore
the PF_MEMALLOC flag.
The save and clear, ensures the preempted task's PF_MEMALLOC flag doesn't
leak into the softirq. The restore ensures a softirq's PF_MEMALLOC flag
cannot leak back into the preempted process.
Signed-off-by: Peter Zijlstra <redacted>
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/linux/sched.h | 7 +++++++
kernel/softirq.c | 3 +++
mm/page_alloc.c | 5 ++++-
3 files changed, 14 insertions(+), 1 deletions(-)
--
1.7.3.4
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
__GFP_MEMALLOC will allow the allocation to disregard the watermarks,
much like PF_MEMALLOC. It allows one to pass along the memalloc state in
object related allocation flags as opposed to task related flags, such
as sk->sk_allocation. This removes the need for ALLOC_PFMEMALLOC as
callers using __GFP_MEMALLOC can get the ALLOC_NO_WATERMARK flag which
is now enough to identify allocations related to page reclaim.
Signed-off-by: Peter Zijlstra <redacted>
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/linux/gfp.h | 4 +++-
include/linux/mm_types.h | 2 +-
mm/page_alloc.c | 14 ++++++--------
mm/slab.c | 2 +-
4 files changed, 11 insertions(+), 11 deletions(-)
@@ -72,7 +72,7 @@ struct page {pgoff_tindex;/* Our offset within mapping. */void*freelist;/* SLUB: freelist req. slab lock */boolpfmemalloc;/* If set by the page allocator,-*ALLOC_PFMEMALLOCwassetandthe+*ALLOC_NO_WATERMARKSwassetandthe*lowwatermarkwasnotmetimplying*thatthesystemisundersome*pressure.Thecallershouldtry
@@ -2978,7 +2978,7 @@ static int cache_grow(struct kmem_cache *cachep,if(!slabp)gotoopps1;-/* Record if ALLOC_PFMEMALLOC was set when allocating the slab */+/* Record if ALLOC_NO_WATERMARKS was set when allocating the slab */if(pfmemalloc){structarray_cache*ac=cpu_cache_get(cachep);slabp->pfmemalloc=true;
--
1.7.3.4
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Allocations of pages below the min watermark run a risk of the machine
hanging due to lack of memory. To prevent this, only callers who
have PF_MEMALLOC or TIF_MEMDIE set and not processing an interrupt are
allowed to allocate with ALLOC_NO_WATERMARKS. Once they are allocated
to a slab though, nothing prevents other callers consuming free objects
within those slabs. This patch limits access to slab pages that were
alloced from the PFMEMALLOC reserves.
Pages allocated from the reserve are returned with page->pfmemalloc
set and it's up to the caller to determine how the page should be
protected. SLAB restricts access to any page with page->pfmemalloc set
to callers which are known to able to access the PFMEMALLOC reserve. If
one is not available, an attempt is made to allocate a new page rather
than use a reserve. SLUB is a bit more relaxed in that it only records
if the current per-CPU page was allocated from PFMEMALLOC reserve and
uses another partial slab if the caller does not have the necessary
GFP or process flags. This was found to be sufficient in tests to
avoid hangs due to SLUB generally maintaining smaller lists than SLAB.
In low-memory conditions it does mean that !PFMEMALLOC allocators
can fail a slab allocation even though free objects are available
because they are being preserved for callers that are freeing pages.
[a.p.zijlstra@chello.nl: Original implementation]
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/linux/mm_types.h | 8 ++
include/linux/slub_def.h | 1 +
mm/internal.h | 3 +
mm/page_alloc.c | 27 +++++-
mm/slab.c | 216 +++++++++++++++++++++++++++++++++++++++-------
mm/slub.c | 35 ++++++--
6 files changed, 246 insertions(+), 44 deletions(-)
@@ -42,6 +42,7 @@ struct kmem_cache_cpu {#endifstructpage*page;/* The slab from which we are allocating */intnode;/* The node of the page (or -1 for debug) */+boolpfmemalloc;/* Slab page had pfmemalloc set */#ifdef CONFIG_SLUB_STATSunsignedstat[NR_SLUB_STAT_ITEMS];#endif
@@ -226,6 +228,7 @@ struct slab {unsignedintinuse;/* num of objs active in slab */kmem_bufctl_tfree;unsignedshortnodeid;+boolpfmemalloc;/* Slab had pfmemalloc set */};structslab_rcu__slab_cover_slab_rcu;};
@@ -888,12 +913,100 @@ static struct array_cache *alloc_arraycache(int node, int entries,nc->avail=0;nc->limit=entries;nc->batchcount=batchcount;-nc->touched=0;+nc->touched=false;spin_lock_init(&nc->lock);}returnnc;}+/* Clears ac->pfmemalloc if no slabs have pfmalloc set */+staticvoidcheck_ac_pfmemalloc(structkmem_cache*cachep,+structarray_cache*ac)+{+structkmem_list3*l3=cachep->nodelists[numa_mem_id()];+structslab*slabp;++if(!ac->pfmemalloc)+return;++list_for_each_entry(slabp,&l3->slabs_full,list)+if(slabp->pfmemalloc)+return;++list_for_each_entry(slabp,&l3->slabs_partial,list)+if(slabp->pfmemalloc)+return;++list_for_each_entry(slabp,&l3->slabs_free,list)+if(slabp->pfmemalloc)+return;++ac->pfmemalloc=false;+}++staticvoid*ac_get_obj(structkmem_cache*cachep,structarray_cache*ac,+gfp_tflags,boolforce_refill)+{+inti;+void*objp=ac->entry[--ac->avail];++/* Ensure the caller is allowed to use objects from PFMEMALLOC slab */+if(unlikely(is_obj_pfmemalloc(objp))){+structkmem_list3*l3;++if(gfp_pfmemalloc_allowed(flags)){+clear_obj_pfmemalloc(&objp);+returnobjp;+}++/* The caller cannot use PFMEMALLOC objects, find another one */+for(i=1;i<ac->avail;i++){+/* If a !PFMEMALLOC object is found, swap them */+if(!is_obj_pfmemalloc(ac->entry[i])){+objp=ac->entry[i];+ac->entry[i]=ac->entry[ac->avail];+ac->entry[ac->avail]=objp;+returnobjp;+}+}++/*+*Iftherearefullemptyslabsandwewerenotforcedto+*allocateaslab,markthisone!pfmemalloc+*/+l3=cachep->nodelists[numa_mem_id()];+if(!list_empty(&l3->slabs_free)&&force_refill){+structslab*slabp=virt_to_slab(objp);+slabp->pfmemalloc=false;+clear_obj_pfmemalloc(&objp);+check_ac_pfmemalloc(cachep,ac);+returnobjp;+}++/* No !PFMEMALLOC objects available */+ac->avail++;+objp=NULL;+}++returnobjp;+}++staticvoidac_put_obj(structkmem_cache*cachep,structarray_cache*ac,+void*objp)+{+structslab*slabp;++/* If there are pfmemalloc slabs, check if the object is part of one */+if(unlikely(ac->pfmemalloc)){+slabp=virt_to_slab(objp);++if(slabp->pfmemalloc)+set_obj_pfmemalloc(&objp);+}++ac->entry[ac->avail++]=objp;+}+/**Transferobjectsinonearraycachetoanother.*Lockingmustbehandledbythecaller.
@@ -2852,7 +2968,7 @@ static int cache_grow(struct kmem_cache *cachep,*'nodeid'.*/if(!objp)-objp=kmem_getpages(cachep,local_flags,nodeid);+objp=kmem_getpages(cachep,local_flags,nodeid,&pfmemalloc);if(!objp)gotofailed;
@@ -2862,6 +2978,13 @@ static int cache_grow(struct kmem_cache *cachep,if(!slabp)gotoopps1;+/* Record if ALLOC_PFMEMALLOC was set when allocating the slab */+if(pfmemalloc){+structarray_cache*ac=cpu_cache_get(cachep);+slabp->pfmemalloc=true;+ac->pfmemalloc=1;+}+slab_map_pages(cachep,slabp,objp);cache_init_objs(cachep,slabp);
@@ -3003,16 +3126,19 @@ bad:#define check_slabp(x,y) do { } while(0)#endif-staticvoid*cache_alloc_refill(structkmem_cache*cachep,gfp_tflags)+staticvoid*cache_alloc_refill(structkmem_cache*cachep,gfp_tflags,+boolforce_refill){intbatchcount;structkmem_list3*l3;structarray_cache*ac;intnode;-retry:check_irq_off();node=numa_mem_id();+if(unlikely(force_refill))+gotoforce_grow;+retry:ac=cpu_cache_get(cachep);batchcount=ac->batchcount;if(!ac->touched&&batchcount>BATCHREFILL_LIMIT){
@@ -3030,7 +3156,7 @@ retry:/* See if we can refill from the shared array */if(l3->shared&&transfer_objects(ac,l3->shared,batchcount)){-l3->shared->touched=1;+l3->shared->touched=true;gotoalloc_done;}
@@ -3082,18 +3208,25 @@ alloc_done:if(unlikely(!ac->avail)){intx;-x=cache_grow(cachep,flags|GFP_THISNODE,node,NULL);+force_grow:+x=cache_grow(cachep,flags|GFP_THISNODE,node,NULL,false);/* cache_grow can reenable interrupts, then ac could change. */ac=cpu_cache_get(cachep);-if(!x&&ac->avail==0)/* no objects in sight? abort */++/* no objects in sight? abort */+if(!x&&(ac->avail==0||force_refill))returnNULL;-if(!ac->avail)/* objects refilled by interrupt? */+/* objects refilled by interrupt? */+if(!ac->avail){+node=numa_node_id();gotoretry;+}}-ac->touched=1;-returnac->entry[--ac->avail];+ac->touched=true;++returnac_get_obj(cachep,ac,flags,force_refill);}staticinlinevoidcache_alloc_debugcheck_before(structkmem_cache*cachep,
@@ -2389,10 +2411,11 @@ static void early_kmem_cache_node_alloc(int node)structpage*page;structkmem_cache_node*n;unsignedlongflags;+boolpfmemalloc;/* Ignore this early in boot */BUG_ON(kmem_cache_node->size<sizeof(structkmem_cache_node));-page=new_slab(kmem_cache_node,GFP_NOWAIT,node);+page=new_slab(kmem_cache_node,GFP_NOWAIT,node,&pfmemalloc);BUG_ON(!page);if(page_to_nid(page)!=node){
--
1.7.3.4
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
__GFP_MEMALLOC will allow the allocation to disregard the watermarks,
much like PF_MEMALLOC. It allows one to pass along the memalloc state in
object related allocation flags as opposed to task related flags, such
as sk->sk_allocation. This removes the need for ALLOC_PFMEMALLOC as
callers using __GFP_MEMALLOC can get the ALLOC_NO_WATERMARK flag which
is now enough to identify allocations related to page reclaim.
Signed-off-by: Peter Zijlstra <redacted>
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/linux/gfp.h | 4 +++-
include/linux/mm_types.h | 2 +-
mm/page_alloc.c | 14 ++++++--------
mm/slab.c | 2 +-
4 files changed, 11 insertions(+), 11 deletions(-)
Having both "MEMALLOC" and "NOMEMALLOC" seems ... unfortunate.
It appears that NOMEMALLOC over-rides MEMALLOC. It might be good to document
this
+#define __GFP_MEMALLOC ((__force gfp_t)___GFP_MEMALLOC)/* Allow access to emergency reserves
unless __GFP_NOMEMALLOC is set*/
#define __GFP_NOMEMALLOC ((__force gfp_t)___GFP_NOMEMALLOC) /* Don't use emergency reserves
Overrides __GFP_MEMALLOC */
I suspect that it is never valid to set both. So NOMEMALLOC is really
NO_PF_MEMALLOC, but making that change is probably just noise.
Maybe a
WARN_ON((gfp_mask & __GFP_MEMALLOC) && (gfp_mask & __GFP_NOMEMALLOC));
might be wise?
NeilBrown.
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
__GFP_MEMALLOC will allow the allocation to disregard the watermarks,
much like PF_MEMALLOC. It allows one to pass along the memalloc state in
object related allocation flags as opposed to task related flags, such
as sk->sk_allocation. This removes the need for ALLOC_PFMEMALLOC as
callers using __GFP_MEMALLOC can get the ALLOC_NO_WATERMARK flag which
is now enough to identify allocations related to page reclaim.
Signed-off-by: Peter Zijlstra <redacted>
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/linux/gfp.h | 4 +++-
include/linux/mm_types.h | 2 +-
mm/page_alloc.c | 14 ++++++--------
mm/slab.c | 2 +-
4 files changed, 11 insertions(+), 11 deletions(-)
Having both "MEMALLOC" and "NOMEMALLOC" seems ... unfortunate.
It appears that NOMEMALLOC over-rides MEMALLOC. It might be good to document
this
I can document it. Right now, a better name than NOMEMALLOC does not
spring to mind.
quoted
+#define __GFP_MEMALLOC ((__force gfp_t)___GFP_MEMALLOC)/* Allow access to emergency reserves
unless __GFP_NOMEMALLOC is set*/
quoted
#define __GFP_NOMEMALLOC ((__force gfp_t)___GFP_NOMEMALLOC) /* Don't use emergency reserves
Overrides __GFP_MEMALLOC */
I suspect that it is never valid to set both. So NOMEMALLOC is really
NO_PF_MEMALLOC, but making that change is probably just noise.
Maybe a
WARN_ON((gfp_mask & __GFP_MEMALLOC) && (gfp_mask & __GFP_NOMEMALLOC));
might be wise?
Both MEMALLOC and NOMEMALLOC are related to PFMEMALLOC reserves so
it's reasonable for them to have similar names. This warning will
also trigger because it's a combination of flags that does happen.
Consider for example
any interrupt
-> __netdev_alloc_skb (mask == GFP_ATOMIC)
-> __alloc_skb (mask == GFP_ATOMIC)
if (sk_memalloc_socks() && (flags & SKB_ALLOC_RX))
gfp_mask |= __GFP_MEMALLOC;
(mask == GFP_ATOMIC|__GFP_NOMEMALLOC)
-> __kmalloc_reserve
First attempt tries to avoid reserves so adds __GFP_MEMALLOC
(mask == GFP_ATOMIC|__GFP_NOMEMALLOC|__GFP_MEMALLOC)
You're right in that __GFP_NOMEMALLOC overrides __GFP_MEMALLOC so that
could do with a note.
--
Mel Gorman
SUSE Labs
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
On Tue, Apr 26, 2011 at 07:49:47PM +1000, NeilBrown wrote:
quoted
Maybe a
WARN_ON((gfp_mask & __GFP_MEMALLOC) && (gfp_mask & __GFP_NOMEMALLOC));
might be wise?
Both MEMALLOC and NOMEMALLOC are related to PFMEMALLOC reserves so
it's reasonable for them to have similar names. This warning will
also trigger because it's a combination of flags that does happen.
Consider for example
any interrupt
-> __netdev_alloc_skb (mask == GFP_ATOMIC)
-> __alloc_skb (mask == GFP_ATOMIC)
if (sk_memalloc_socks() && (flags & SKB_ALLOC_RX))
gfp_mask |= __GFP_MEMALLOC;
(mask == GFP_ATOMIC|__GFP_NOMEMALLOC)
-> __kmalloc_reserve
First attempt tries to avoid reserves so adds __GFP_MEMALLOC
(mask == GFP_ATOMIC|__GFP_NOMEMALLOC|__GFP_MEMALLOC)
You have the "NO"s mixed up a bit which confused me for a while :-)
But I see your point - I guess the WARN_ON isn't really needed.
You're right in that __GFP_NOMEMALLOC overrides __GFP_MEMALLOC so that
could do with a note.
Thanks,
NeilBrown
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
@@ -2202,8 +2211,16 @@ nopage: got_pg: if (kmemcheck_enabled) kmemcheck_pagealloc_alloc(page, order, gfp_mask);- return page;+ /*+ * page->pfmemalloc is set when the caller had PFMEMALLOC set or is+ * been OOM killed. The expectation is that the caller is taking+ * steps that will free more memory. The caller should avoid the+ * page being used for !PFMEMALLOC purposes.+ */+ page->pfmemalloc = (alloc_flags & ALLOC_PFMEMALLOC);++ return page;
Linus doesn't seem to be a fan of this construct:
https://lkml.org/lkml/2011/4/1/255
pfmemalloc is a bool, and the value on the right is either 0 or 0x1000.
If bool happens to be typedefed to 'char' or even 'short', pfmemalloc would
always be set to 0.
Ditto for the gfp_pfmemalloc_allowed function.
Prefixing with '!!' would make it safe.
NeilBrown
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
@@ -2202,8 +2211,16 @@ nopage: got_pg: if (kmemcheck_enabled) kmemcheck_pagealloc_alloc(page, order, gfp_mask);- return page;+ /*+ * page->pfmemalloc is set when the caller had PFMEMALLOC set or is+ * been OOM killed. The expectation is that the caller is taking+ * steps that will free more memory. The caller should avoid the+ * page being used for !PFMEMALLOC purposes.+ */+ page->pfmemalloc = (alloc_flags & ALLOC_PFMEMALLOC);++ return page;
There is confusion around this topic. Andrew prefers bool for true/false
values and it's self-documenting. I tend to prefer it myself for
readability and there is a slow conversion in the VM from
ints-used-as-bools to bools and my understanding of bool is that any
non-zero value will be treated as true (just as it is for int).
pfmemalloc is a bool, and the value on the right is either 0 or 0x1000.
If bool happens to be typedefed to 'char' or even 'short', pfmemalloc would
always be set to 0.
It is typedeffed as _Bool though which I thought was able to handle the
cast appropriately or is that wrong?
Ditto for the gfp_pfmemalloc_allowed function.
Prefixing with '!!' would make it safe.
Will do that to avoid any oddities. Thanks
--
Mel Gorman
SUSE Labs
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
+ /*+ * If there are full empty slabs and we were not forced to+ * allocate a slab, mark this one !pfmemalloc+ */+ l3 = cachep->nodelists[numa_mem_id()];+ if (!list_empty(&l3->slabs_free) && force_refill) {+ struct slab *slabp = virt_to_slab(objp);+ slabp->pfmemalloc = false;+ clear_obj_pfmemalloc(&objp);+ check_ac_pfmemalloc(cachep, ac);+ return objp;+ }
The comment doesn't match the code. I think you need to remove the words
"full" and "not" assuming the code is correct which it probably is...
But the code seems to be much more complex than Peter's original, and I don't
see the gain.
Peter's code had only one 'reserved' flag for each kmem_cache. You seem to
have one for every slab. I don't see the point.
It is true that yours is in some sense more fair - but I'm not sure the
complexity is worth it.
Was there some particular reason you made the change?
NeilBrown
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
@@ -2202,8 +2211,16 @@ nopage: got_pg: if (kmemcheck_enabled) kmemcheck_pagealloc_alloc(page, order, gfp_mask);- return page;+ /*+ * page->pfmemalloc is set when the caller had PFMEMALLOC set or is+ * been OOM killed. The expectation is that the caller is taking+ * steps that will free more memory. The caller should avoid the+ * page being used for !PFMEMALLOC purposes.+ */+ page->pfmemalloc = (alloc_flags & ALLOC_PFMEMALLOC);++ return page;
There is confusion around this topic. Andrew prefers bool for true/false
values and it's self-documenting. I tend to prefer it myself for
readability and there is a slow conversion in the VM from
ints-used-as-bools to bools and my understanding of bool is that any
non-zero value will be treated as true (just as it is for int).
quoted
pfmemalloc is a bool, and the value on the right is either 0 or 0x1000.
If bool happens to be typedefed to 'char' or even 'short', pfmemalloc would
always be set to 0.
It is typedeffed as _Bool though which I thought was able to handle the
cast appropriately or is that wrong?
Yes, I too believe that _Bool does the right thing, so this particular usage
does happen to be safe in the kernel. But there is a long tradition of
'bool' being typedefed to an int type so the usage can look wrong. So maybe
it is best avoided.
I have no strong feeling either way - I just thought I would highlight it.
In general, I like bool, but I don't like automatic casts (I often use
if (pointer != NULL) ...
because I think it reads better).
Thanks,
NeilBrown
quoted
Ditto for the gfp_pfmemalloc_allowed function.
Prefixing with '!!' would make it safe.
Will do that to avoid any oddities. Thanks
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
This sort of thing really bugs me :-)
Neither the comment nor the function name actually describe what the function
is doing. The function is checking *2* things.
is_pfmemalloc_skb_or_pfmemalloc_protocol()
might be a more correct name, but is too verbose.
I would prefer the skb_pfmemalloc test were removed from here and ....
quoted hunk
+ if (!skb_pfmemalloc_protocol(skb))+ goto drop;+
...added here so this becomes:
if (!skb_pfmemalloc(skb) && !skb_pfmemalloc_protocol(skb))
goto drop;
which actually makes sense.
Thanks,
NeilBrown
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
+/*+ * Throttle direct reclaimers if backing storage is backed by the network+ * and the PFMEMALLOC reserve for the preferred node is getting dangerously+ * depleted. kswapd will continue to make progress and wake the processes+ * when the low watermark is reached+ */+static void throttle_direct_reclaim(gfp_t gfp_mask, struct zonelist *zonelist,+ nodemask_t *nodemask)+{+ struct zone *zone;+ int high_zoneidx = gfp_zone(gfp_mask);+ DEFINE_WAIT(wait);++ /* Check if the pfmemalloc reserves are ok */+ first_zones_zonelist(zonelist, high_zoneidx, NULL, &zone);+ prepare_to_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait,+ TASK_INTERRUPTIBLE);+ if (pfmemalloc_watermark_ok(zone->zone_pgdat, high_zoneidx))+ goto out;++ /* Throttle */+ do {+ schedule();+ finish_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait);+ prepare_to_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait,+ TASK_INTERRUPTIBLE);+ } while (!pfmemalloc_watermark_ok(zone->zone_pgdat, high_zoneidx) &&+ !fatal_signal_pending(current));++out:+ finish_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait);+}
You are doing an interruptible wait, but only checking for fatal signals.
So if a non-fatal signal arrives, you will busy-wait.
So I suspect you want TASK_KILLABLE, so just use:
wait_event_killable(zone->zone_pgdat->pfmemalloc_wait,
pgmemalloc_watermark_ok(zone->zone_pgdata,
high_zoneidx));
(You also have an extraneous call to finish_wait)
NeilBrown
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Under significant pressure when writing back to network-backed storage,
direct reclaimers may get throttled. This is expected to be a
short-lived event and the processes get woken up again but processes do
get stalled. This patch counts how many times such stalling occurs. It's
up to the administrator whether to reduce these stalls by increasing
min_free_kbytes.
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/linux/vm_event_item.h | 1 +
mm/vmscan.c | 1 +
mm/vmstat.c | 1 +
3 files changed, 3 insertions(+), 0 deletions(-)
I like this approach. Make the information available, but don't make a fuss
about it.
Actually, I like the whole series - I'm really having to dig deep to find
anything to complain about :-)
Feel free to put
Reviewed-by: NeilBrown [off-list ref]
against anything that I haven't commented on.
Thanks,
NeilBrown
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
+ /*+ * If there are full empty slabs and we were not forced to+ * allocate a slab, mark this one !pfmemalloc+ */+ l3 = cachep->nodelists[numa_mem_id()];+ if (!list_empty(&l3->slabs_free) && force_refill) {+ struct slab *slabp = virt_to_slab(objp);+ slabp->pfmemalloc = false;+ clear_obj_pfmemalloc(&objp);+ check_ac_pfmemalloc(cachep, ac);+ return objp;+ }
The comment doesn't match the code. I think you need to remove the words
"full" and "not" assuming the code is correct which it probably is...
I'll fix up the comment, you're right, it's confusing.
But the code seems to be much more complex than Peter's original, and I don't
see the gain.
You're right, it is more complex.
Peter's code had only one 'reserved' flag for each kmem_cache.
The reserve was set in a per-cpu structure so there was a "lag" time
before that information was available to other CPUs. Fine on smaller
machines but a bit more of a problem today.
You seem to
have one for every slab. I don't see the point.
It is true that yours is in some sense more fair - but I'm not sure the
complexity is worth it.
More fairness was one of the objects.
Was there some particular reason you made the change?
This version survives under considerably more stress than Peter's
original version did without requiring the additional complexity of
memory reserves.
--
Mel Gorman
SUSE Labs
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
On Tue, Apr 26, 2011 at 07:49:47PM +1000, NeilBrown wrote:
quoted
quoted
Maybe a
WARN_ON((gfp_mask & __GFP_MEMALLOC) && (gfp_mask & __GFP_NOMEMALLOC));
might be wise?
Both MEMALLOC and NOMEMALLOC are related to PFMEMALLOC reserves so
it's reasonable for them to have similar names. This warning will
also trigger because it's a combination of flags that does happen.
Consider for example
any interrupt
-> __netdev_alloc_skb (mask == GFP_ATOMIC)
-> __alloc_skb (mask == GFP_ATOMIC)
if (sk_memalloc_socks() && (flags & SKB_ALLOC_RX))
gfp_mask |= __GFP_MEMALLOC;
(mask == GFP_ATOMIC|__GFP_NOMEMALLOC)
-> __kmalloc_reserve
First attempt tries to avoid reserves so adds __GFP_MEMALLOC
(mask == GFP_ATOMIC|__GFP_NOMEMALLOC|__GFP_MEMALLOC)
You have the "NO"s mixed up a bit which confused me for a while :-)
But I see your point - I guess the WARN_ON isn't really needed.
Bah, that was unhelpful on my part. I'm glad you saw the point anyway.
Sorry about that.
--
Mel Gorman
SUSE Labs
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
This sort of thing really bugs me :-)
Neither the comment nor the function name actually describe what the function
is doing. The function is checking *2* things.
is_pfmemalloc_skb_or_pfmemalloc_protocol()
might be a more correct name, but is too verbose.
I would prefer the skb_pfmemalloc test were removed from here and ....
quoted
+ if (!skb_pfmemalloc_protocol(skb))+ goto drop;+
...added here so this becomes:
if (!skb_pfmemalloc(skb) && !skb_pfmemalloc_protocol(skb))
goto drop;
which actually makes sense.
Moving the check is neater but that check should be
if (skb_pfmemalloc(skb) && !skb_pfmemalloc_protocol(skb))
? It's only if the skb was allocated from emergency reserves that we
need to consider dropping it to make way for other packets to be
received.
--
Mel Gorman
SUSE Labs
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Peter Zijlstra <hidden> Date: 2011-04-26 14:23:26
On Tue, 2011-04-26 at 08:36 +0100, Mel Gorman wrote:
Comments?
Last time I brought up the whole swap over network bits I was pointed
towards the generic skb recycling work:
http://lwn.net/Articles/332037/
as a means to pre-allocate memory, and it was suggested to simply pin
the few route-cache entries required to route these packets and
dis-allow swap packets to be fragmented (these last two avoid lots of
funny allocation cases in the network stack).
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
+/*+ * Throttle direct reclaimers if backing storage is backed by the network+ * and the PFMEMALLOC reserve for the preferred node is getting dangerously+ * depleted. kswapd will continue to make progress and wake the processes+ * when the low watermark is reached+ */+static void throttle_direct_reclaim(gfp_t gfp_mask, struct zonelist *zonelist,+ nodemask_t *nodemask)+{+ struct zone *zone;+ int high_zoneidx = gfp_zone(gfp_mask);+ DEFINE_WAIT(wait);++ /* Check if the pfmemalloc reserves are ok */+ first_zones_zonelist(zonelist, high_zoneidx, NULL, &zone);+ prepare_to_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait,+ TASK_INTERRUPTIBLE);+ if (pfmemalloc_watermark_ok(zone->zone_pgdat, high_zoneidx))+ goto out;++ /* Throttle */+ do {+ schedule();+ finish_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait);+ prepare_to_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait,+ TASK_INTERRUPTIBLE);+ } while (!pfmemalloc_watermark_ok(zone->zone_pgdat, high_zoneidx) &&+ !fatal_signal_pending(current));++out:+ finish_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait);+}
You are doing an interruptible wait, but only checking for fatal signals.
So if a non-fatal signal arrives, you will busy-wait.
So I suspect you want TASK_KILLABLE, so just use:
wait_event_killable(zone->zone_pgdat->pfmemalloc_wait,
pgmemalloc_watermark_ok(zone->zone_pgdata,
high_zoneidx));
Well, if a normal signal arrives, we do not necessarily want the
process to enter reclaim. For fatal signals, I allow it to continue
because it's not likely to be putting the system under more pressure
if it's exiting.
(You also have an extraneous call to finish_wait)
Which one? I'm not seeing a flow where finish_wait gets called twice
without a prepare_to_wait in between.
--
Mel Gorman
SUSE Labs
Under significant pressure when writing back to network-backed storage,
direct reclaimers may get throttled. This is expected to be a
short-lived event and the processes get woken up again but processes do
get stalled. This patch counts how many times such stalling occurs. It's
up to the administrator whether to reduce these stalls by increasing
min_free_kbytes.
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
include/linux/vm_event_item.h | 1 +
mm/vmscan.c | 1 +
mm/vmstat.c | 1 +
3 files changed, 3 insertions(+), 0 deletions(-)
I like this approach. Make the information available, but don't make a fuss
about it.
Actually, I like the whole series - I'm really having to dig deep to find
anything to complain about :-)
Feel free to put
Reviewed-by: NeilBrown [off-list ref]
against anything that I haven't commented on.
Thanks very much!
--
Mel Gorman
SUSE Labs
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
On Tue, Apr 26, 2011 at 04:23:05PM +0200, Peter Zijlstra wrote:
On Tue, 2011-04-26 at 08:36 +0100, Mel Gorman wrote:
quoted
Comments?
Last time I brought up the whole swap over network bits I was pointed
towards the generic skb recycling work:
http://lwn.net/Articles/332037/
as a means to pre-allocate memory,
I'd taken note of this to take a much closer look if it turned
out reservations were necessary and to find out what happened with
these patches. So far, bigger reservations have *not* been required
but I agree recycling SKBs may be a better alternative than large
reservations or preallocations if they are necessary.
and it was suggested to simply pin
the few route-cache entries required to route these packets and
dis-allow swap packets to be fragmented (these last two avoid lots of
funny allocation cases in the network stack).
I did find that only a few route-cache entries should be required. In
the original patches I worked with, there was a reservation for the
maximum possible number of route-cache entries. I thought this was
overkill and instead reserved 1-per-active-swapfile-backed-by-NFS.
--
Mel Gorman
SUSE Labs
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Peter Zijlstra <hidden> Date: 2011-04-26 14:51:08
On Tue, 2011-04-26 at 15:46 +0100, Mel Gorman wrote:
I did find that only a few route-cache entries should be required. In
the original patches I worked with, there was a reservation for the
maximum possible number of route-cache entries. I thought this was
overkill and instead reserved 1-per-active-swapfile-backed-by-NFS.
Right, so the thing I was worried about was a route-cache poison attack
where someone would spam the machine such that it would create a lot of
route cache entries and might flush the one we needed just as we needed
it.
Pinning the one entry we need would solve that (if possible).
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
+/*+ * Throttle direct reclaimers if backing storage is backed by the network+ * and the PFMEMALLOC reserve for the preferred node is getting dangerously+ * depleted. kswapd will continue to make progress and wake the processes+ * when the low watermark is reached+ */+static void throttle_direct_reclaim(gfp_t gfp_mask, struct zonelist *zonelist,+ nodemask_t *nodemask)+{+ struct zone *zone;+ int high_zoneidx = gfp_zone(gfp_mask);+ DEFINE_WAIT(wait);++ /* Check if the pfmemalloc reserves are ok */+ first_zones_zonelist(zonelist, high_zoneidx, NULL, &zone);+ prepare_to_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait,+ TASK_INTERRUPTIBLE);+ if (pfmemalloc_watermark_ok(zone->zone_pgdat, high_zoneidx))+ goto out;++ /* Throttle */+ do {+ schedule();+ finish_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait);+ prepare_to_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait,+ TASK_INTERRUPTIBLE);+ } while (!pfmemalloc_watermark_ok(zone->zone_pgdat, high_zoneidx) &&+ !fatal_signal_pending(current));++out:+ finish_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait);+}
You are doing an interruptible wait, but only checking for fatal signals.
So if a non-fatal signal arrives, you will busy-wait.
So I suspect you want TASK_KILLABLE, so just use:
wait_event_killable(zone->zone_pgdat->pfmemalloc_wait,
pgmemalloc_watermark_ok(zone->zone_pgdata,
high_zoneidx));
Well, if a normal signal arrives, we do not necessarily want the
process to enter reclaim. For fatal signals, I allow it to continue
because it's not likely to be putting the system under more pressure
if it's exiting.
Yep, I understand that and it doesn't seem unreasonable.
However I don't think the code implements that correctly.
If you get a non-fatal signal, schedule will exit immediately (because of the
TASK_INTERRUPTIBLE setting) and the 'while' clause will succeed because the
signal is not fatal, so it will loop around and try to schedule again, which
will again exit immediately - busy loop.
quoted
(You also have an extraneous call to finish_wait)
Which one? I'm not seeing a flow where finish_wait gets called twice
without a prepare_to_wait in between.
You don't need to call finish_wait immediately before prepare_to_wait.
It really is best to just use the appropriate 'wait_event*' macro....
NeilBrown
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
This sort of thing really bugs me :-)
Neither the comment nor the function name actually describe what the function
is doing. The function is checking *2* things.
is_pfmemalloc_skb_or_pfmemalloc_protocol()
might be a more correct name, but is too verbose.
I would prefer the skb_pfmemalloc test were removed from here and ....
quoted
+ if (!skb_pfmemalloc_protocol(skb))+ goto drop;+
...added here so this becomes:
if (!skb_pfmemalloc(skb) && !skb_pfmemalloc_protocol(skb))
goto drop;
which actually makes sense.
Moving the check is neater but that check should be
if (skb_pfmemalloc(skb) && !skb_pfmemalloc_protocol(skb))
? It's only if the skb was allocated from emergency reserves that we
need to consider dropping it to make way for other packets to be
received.
Correct. I got my Boolean algebra all confused. Sorry 'bout that.
NeilBrown
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
+/*+ * Throttle direct reclaimers if backing storage is backed by the network+ * and the PFMEMALLOC reserve for the preferred node is getting dangerously+ * depleted. kswapd will continue to make progress and wake the processes+ * when the low watermark is reached+ */+static void throttle_direct_reclaim(gfp_t gfp_mask, struct zonelist *zonelist,+ nodemask_t *nodemask)+{+ struct zone *zone;+ int high_zoneidx = gfp_zone(gfp_mask);+ DEFINE_WAIT(wait);++ /* Check if the pfmemalloc reserves are ok */+ first_zones_zonelist(zonelist, high_zoneidx, NULL, &zone);+ prepare_to_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait,+ TASK_INTERRUPTIBLE);+ if (pfmemalloc_watermark_ok(zone->zone_pgdat, high_zoneidx))+ goto out;++ /* Throttle */+ do {+ schedule();+ finish_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait);+ prepare_to_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait,+ TASK_INTERRUPTIBLE);+ } while (!pfmemalloc_watermark_ok(zone->zone_pgdat, high_zoneidx) &&+ !fatal_signal_pending(current));++out:+ finish_wait(&zone->zone_pgdat->pfmemalloc_wait, &wait);+}
You are doing an interruptible wait, but only checking for fatal signals.
So if a non-fatal signal arrives, you will busy-wait.
So I suspect you want TASK_KILLABLE, so just use:
wait_event_killable(zone->zone_pgdat->pfmemalloc_wait,
pgmemalloc_watermark_ok(zone->zone_pgdata,
high_zoneidx));
Well, if a normal signal arrives, we do not necessarily want the
process to enter reclaim. For fatal signals, I allow it to continue
because it's not likely to be putting the system under more pressure
if it's exiting.
Yep, I understand that and it doesn't seem unreasonable.
However I don't think the code implements that correctly.
If you get a non-fatal signal, schedule will exit immediately (because of the
TASK_INTERRUPTIBLE setting) and the 'while' clause will succeed because the
signal is not fatal, so it will loop around and try to schedule again, which
will again exit immediately - busy loop.
Ah, I see. Once again, well spotted.
quoted
quoted
(You also have an extraneous call to finish_wait)
Which one? I'm not seeing a flow where finish_wait gets called twice
without a prepare_to_wait in between.
You don't need to call finish_wait immediately before prepare_to_wait.
It really is best to just use the appropriate 'wait_event*' macro....
wait_event_interruptible it is. Thanks
--
Mel Gorman
SUSE Labs
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
On Tue, Apr 26, 2011 at 04:50:49PM +0200, Peter Zijlstra wrote:
On Tue, 2011-04-26 at 15:46 +0100, Mel Gorman wrote:
quoted
I did find that only a few route-cache entries should be required. In
the original patches I worked with, there was a reservation for the
maximum possible number of route-cache entries. I thought this was
overkill and instead reserved 1-per-active-swapfile-backed-by-NFS.
Right, so the thing I was worried about was a route-cache poison attack
where someone would spam the machine such that it would create a lot of
route cache entries and might flush the one we needed just as we needed
it.
Pinning the one entry we need would solve that (if possible).
That is a possibility all right, nice thoughts there. Ok, as I do
not want this series to grow to the point where it is unreviewable,
I'll mark pinning the routing cache entry for a follow-on series.
In this series, the throttling logic should allow a new routing cache
entry to be allocated by kswapd as it's immune to the throttle.
Thanks.
--
Mel Gorman
SUSE Labs
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
+ /*+ * If there are full empty slabs and we were not forced to+ * allocate a slab, mark this one !pfmemalloc+ */+ l3 = cachep->nodelists[numa_mem_id()];+ if (!list_empty(&l3->slabs_free) && force_refill) {+ struct slab *slabp = virt_to_slab(objp);+ slabp->pfmemalloc = false;+ clear_obj_pfmemalloc(&objp);+ check_ac_pfmemalloc(cachep, ac);+ return objp;+ }
The comment doesn't match the code. I think you need to remove the words
"full" and "not" assuming the code is correct which it probably is...
I'll fix up the comment, you're right, it's confusing.
quoted
But the code seems to be much more complex than Peter's original, and I don't
see the gain.
You're right, it is more complex.
quoted
Peter's code had only one 'reserved' flag for each kmem_cache.
The reserve was set in a per-cpu structure so there was a "lag" time
before that information was available to other CPUs. Fine on smaller
machines but a bit more of a problem today.
quoted
You seem to
have one for every slab. I don't see the point.
It is true that yours is in some sense more fair - but I'm not sure the
complexity is worth it.
More fairness was one of the objects.
quoted
Was there some particular reason you made the change?
This version survives under considerably more stress than Peter's
original version did without requiring the additional complexity of
memory reserves.
That is certainly a very compelling argument .... but I still don't get why.
I'm sorry if I'm being dense, but I still don't see why the complexity buys
us better stability and I really would like to understand.
You don't seem to need the same complexity for SLUB with the justification
of "SLUB generally maintaining smaller lists than SLAB".
Presumably these are per-CPU lists of free objects or slabs? If the things
on those lists could be used by anyone long lists couldn't hurt.
So the problem must be that the lists get long while the array_cache is still
marked as 'pfmemalloc' (or 'reserve' in Peter's patches).
Is that the problem? That reserve memory gets locked up in SLAB freelists?
If so - would that be more easily addressed by effectively reducing the
'batching' when the array_cache had dipped into reserves, so slabs are
returned to the VM more promptly?
Probably related, now that you've fixed the comment here (thanks):
+ /*
+ * If there are empty slabs on the slabs_free list and we are
+ * being forced to refill the cache, mark this one !pfmemalloc.
+ */
+ l3 = cachep->nodelists[numa_mem_id()];
+ if (!list_empty(&l3->slabs_free) && force_refill) {
+ struct slab *slabp = virt_to_slab(objp);
+ slabp->pfmemalloc = false;
+ clear_obj_pfmemalloc(&objp);
+ check_ac_pfmemalloc(cachep, ac);
+ return objp;
+ }
I'm trying to understand it...
The context is that a non-MEMALLOC allocation is happening and everything on
the free lists is reserved for MEMALLOC allocations.
So if in that case there is a completely free slab on the free list we decide
that it is OK to mark the current slab as non-MEMALLOC.
The logic seems to be that we could just release that free slab to the VM,
then an alloc_page would be able to get it back. But if we are still well
below the reserve watermark, then there might be some other allocation that
is more deserving of the page and we shouldn't just assume we can take
it with actually calling in to alloc_pages to check that we are no longer
running on reserves..
So this looks like an optimisation that is wrong.
BTW,
+ /* Record if ALLOC_PFMEMALLOC was set when allocating the slab */
+ if (pfmemalloc) {
+ struct array_cache *ac = cpu_cache_get(cachep);
+ slabp->pfmemalloc = true;
+ ac->pfmemalloc = 1;
+ }
+
I think that "= 1" should be "= true". :-)
NeilBrown
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
+ /*+ * If there are full empty slabs and we were not forced to+ * allocate a slab, mark this one !pfmemalloc+ */+ l3 = cachep->nodelists[numa_mem_id()];+ if (!list_empty(&l3->slabs_free) && force_refill) {+ struct slab *slabp = virt_to_slab(objp);+ slabp->pfmemalloc = false;+ clear_obj_pfmemalloc(&objp);+ check_ac_pfmemalloc(cachep, ac);+ return objp;+ }
The comment doesn't match the code. I think you need to remove the words
"full" and "not" assuming the code is correct which it probably is...
I'll fix up the comment, you're right, it's confusing.
quoted
But the code seems to be much more complex than Peter's original, and I don't
see the gain.
You're right, it is more complex.
quoted
Peter's code had only one 'reserved' flag for each kmem_cache.
The reserve was set in a per-cpu structure so there was a "lag" time
before that information was available to other CPUs. Fine on smaller
machines but a bit more of a problem today.
quoted
You seem to
have one for every slab. I don't see the point.
It is true that yours is in some sense more fair - but I'm not sure the
complexity is worth it.
More fairness was one of the objects.
quoted
Was there some particular reason you made the change?
This version survives under considerably more stress than Peter's
original version did without requiring the additional complexity of
memory reserves.
That is certainly a very compelling argument .... but I still don't get why.
I'm sorry if I'm being dense, but I still don't see why the complexity buys
us better stability and I really would like to understand.
You don't seem to need the same complexity for SLUB with the justification
of "SLUB generally maintaining smaller lists than SLAB".
It is an educated guess that the length of the lists was what was
relevant. Even without these patches, SLUB is harder to lockup (minutes
rather than seconds to halt the machine) than SLAB and I assumed it
was because there were fewer pages pinned on per-CPU lists with SLUB.
Presumably these are per-CPU lists of free objects or slabs?
The per-cpu lists are of objects (the entry[] array in struct
array_cache). The slab management structure is looked up much less
frequently (when a block of objects are being freed for example).
If the things
on those lists could be used by anyone long lists couldn't hurt.
Long lists can hurt in a few ways but I believe the two relevant reasons
for this series are;
1. A remote CPU could be holding the object on its free list
2. Multiple unnecessary caches could be pinning free memory with the
shrinkers not triggering because everything is waiting on IO to complete
So the problem must be that the lists get long while the array_cache is still
marked as 'pfmemalloc'
By marking the array_cache pfmemalloc we can have objects on the list
that are a mix of pfmemalloc and !pfmemalloc objects with very coarse
control over who is accessing them.
For example. In Peters patches, CPU A could allocate from pfmemalloc
reserves and mark its array_cache appropriately. CPU B could be freeing
the objects but not have its array_cache marked. PFMEMALLOC objects
are now available for !PFMEMALLOC uses on that CPU and we dip further
into our reserves. This was managed by the memory reservation patches
which meant that dipping further into the reserves was not that much
of a problem.
(or 'reserve' in Peter's patches).
Is that the problem? That reserve memory gets locked up in SLAB freelists?
If so - would that be more easily addressed by effectively reducing the
'batching' when the array_cache had dipped into reserves, so slabs are
returned to the VM more promptly?
Pinning objects on long list is one problem but I don't think it's
the most important problem. Indications were that the big problem
was insufficient control over who was accessing objects belonging to
slab pages allocated from the pfmemalloc reserves.
Probably related, now that you've fixed the comment here (thanks):
+ /*
+ * If there are empty slabs on the slabs_free list and we are
+ * being forced to refill the cache, mark this one !pfmemalloc.
+ */
+ l3 = cachep->nodelists[numa_mem_id()];
+ if (!list_empty(&l3->slabs_free) && force_refill) {
+ struct slab *slabp = virt_to_slab(objp);
+ slabp->pfmemalloc = false;
+ clear_obj_pfmemalloc(&objp);
+ check_ac_pfmemalloc(cachep, ac);
+ return objp;
+ }
I'm trying to understand it...
Thanks :)
The context is that a non-MEMALLOC allocation is happening and everything on
the free lists is reserved for MEMALLOC allocations.
Yep.
So if in that case there is a completely free slab on the free list we decide
that it is OK to mark the current slab as non-MEMALLOC.
Yep.
The logic seems to be that we could just release that free slab to the VM,
then an alloc_page would be able to get it back.
We could, it'd be slower and there would need to be some sort of
retry path in a hotter code path to catch the situation but we could.
But if we are still well
below the reserve watermark, then there might be some other allocation that
is more deserving of the page and we shouldn't just assume we can take
it with actually calling in to alloc_pages to check that we are no longer
running on reserves..
So this looks like an optimisation that is wrong.
The problem that is being addressed here is that pfmemalloc slabs
have to made available for general use at some point or slabs can
be artifically large and waste memory. I didn't do a free and retry
path because it'd be more expensive and I wanted to avoid hurting
the common slab paths.
The assumption is that if there are free slab pages while there are
pfmemalloc slabs in use then we cannot be under that much pressure
and the free slab page is safe to use. If we go well below the reserve
watermark as you are concerned about, the throttle logic will trigger
and we'll at least identify that the situation occured without the
system crashing.
BTW,
+ /* Record if ALLOC_PFMEMALLOC was set when allocating the slab */
+ if (pfmemalloc) {
+ struct array_cache *ac = cpu_cache_get(cachep);
+ slabp->pfmemalloc = true;
+ ac->pfmemalloc = 1;
+ }
+
I think that "= 1" should be "= true". :-)
/me slaps self
Thanks
--
Mel Gorman
SUSE Labs
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Pavel Machek <hidden> Date: 2011-04-28 13:32:09
Hi!
For testing swap-over-NBD, a machine was booted with 2G of RAM with a
swapfile backed by NBD. 16*NUM_CPU processes were started that create
anonymous memory mappings and read them linearly in a loop. The total
size of the mappings were 4*PHYSICAL_MEMORY to use swap heavily under
memory pressure. Without the patches, the machine locks up within
minutes and runs to completion with them applied.
Comments?
Nice!
It is easy to see why swapping needs these fixes, but... dirty memory
writeout is used for memory clearing, too. Are same changes neccessary
to make that safe?
(Perhaps raise 'max dirty %' for testing?)
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
On Thu, Apr 28, 2011 at 03:31:55PM +0200, Pavel Machek wrote:
Hi!
quoted
For testing swap-over-NBD, a machine was booted with 2G of RAM with a
swapfile backed by NBD. 16*NUM_CPU processes were started that create
anonymous memory mappings and read them linearly in a loop. The total
size of the mappings were 4*PHYSICAL_MEMORY to use swap heavily under
memory pressure. Without the patches, the machine locks up within
minutes and runs to completion with them applied.
Comments?
Nice!
It is easy to see why swapping needs these fixes, but... dirty memory
writeout is used for memory clearing, too. Are same changes neccessary
to make that safe?
Dirty page limiting covers the MAP_SHARED cases and are already
throttled approprately.
(Perhaps raise 'max dirty %' for testing?)
Stress testing passed for dirty ratios of 40% at least. Maybe it would
cause issues when raised to nearly 100% but I don't think that is a
particularly interesting use case.
--
Mel Gorman
SUSE Labs
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>