On 11/4/20 1:36 PM, Matthew Wilcox wrote: > On Wed, Nov 04, 2020 at 09:50:30AM +0100, Eric Dumazet wrote: >> On 11/4/20 2:16 AM, Rama Nichanamatlu wrote: >>>> Thanks for providing the numbers. Do you think that dropping (up to) >>>> 7 packets is acceptable? >>> >>> net.ipv4.tcp_syn_retries = 6 >>> >>> tcp clients wouldn't even get that far leading to connect establish issues. >> >> This does not really matter. If host was under memory pressure, >> dropping a few packets is really not an issue. >> >> Please do not add expensive checks in fast path, just to "not drop a packet" >> even if the world is collapsing. > > Right, that was my first patch -- to only recheck if we're about to > reuse the page. Do you think that's acceptable, or is that still too > close to the fast path? I think it is totally acceptable. The same strategy is used in NIC drivers, before recycling a page. If page_is_pfmemalloc() returns true, they simply release the 'problematic'page and attempt a new allocation. ( git grep -n page_is_pfmemalloc -- drivers/net/ethernet/ ) > >> Also consider that NIC typically have thousands of pre-allocated page/frags >> for their RX ring buffers, they might all have pfmemalloc set, so we are speaking >> of thousands of packet drops before the RX-ring can be refilled with normal (non pfmemalloc) page/frags. >> >> If we want to solve this issue more generically, we would have to try >> to copy data into a non pfmemalloc frag instead of dropping skb that >> had frags allocated minutes ago under memory pressure. > > I don't think we need to copy anything. We need to figure out if the > system is still under memory pressure, and if not, we can clear the > pfmemalloc bit on the frag, as in my second patch. The 'least change' > way of doing that is to try to allocate a page, but the VM could export > a symbol that says "we're not under memory pressure any more". > > Did you want to move checking that into the networking layer, or do you > want to keep it in the pagefrag allocator? I think your proposal is fine, thanks !
