Re: [PATCH 1/4] net/skbuff: don't waste memory reserves

From: Andrey Ryabinin
Date: Fri Apr 19 2019 - 16:58:47 EST


On 4/19/19 4:41 PM, Eric Dumazet wrote:
> On 04/19/2019 06:17 AM, Andrey Ryabinin wrote:
>>
>
>> I don't see why that would be a problem. If refill failed because we didn't have
>> access to reserves, then there going to be an another refill attempt, right?
>> And the next refill attempt will be with access to the reserves if memalloc socket was created.
>> We can't predict the future, so until the memalloc socket appeared we must assume that those
>> RX ring buffers won't be used to reclaim memory (and that is actually true in 99% of cases).
>>
>
> I just said that the alloc might be attempted "in the past"
>
> Yes, we can not predict the future, this is why we need to access the reserve _now_ and not
> at the time the packet is received.
>
> The 'being true in 99% of cases' argument is not really convincing.
>
> You want the NIC to be ready to receive packets even before sk_memalloc_socks() becomes true.

The fact that we allocate pages before the socket created I completely understand.

But why that failed allocation is such a problem?

1. sk_memalloc_socks() false
2. NIC driver tries to allocate pages and fails
3. swapon /nfs/swap_file /* memalloc_socket created */
4. We may be loosing some packets because of 2. But there will be retransmit, I suppose NIC driver will
try to allocate pages again, and this time with access to reserves, so eventually we all good.

So what is the problem? Potential lost of some packets for some period of time after swapon?


> If a NIC driver has a bug, please fix it.
>

I don't follow, what kind of bug are you talking about?
The problem I'm trying to fix is entirely in __dev_alloc_pages():
1. sk_memalloc_socks() false all the time.
2. NIC takes pages from memory reserves
3. The fact that we occupying memory reserves increases the chances that the next page will be also taken from reserves as well,
which means more dropped packets than we would have without using the reserves.