NAK: [SRU][F][PATCH 1/1] s390/qeth: use memory reserves to back RX buffers

[Krzysztof Kozlowski]

On 04/02/2022 15:29, Krzysztof Kozlowski wrote:
> On 04/02/2022 14:49, Thadeu Lima de Souza Cascardo wrote:
>> On Fri, Feb 04, 2022 at 10:29:21AM +0100, Krzysztof Kozlowski wrote:
>>> On 04/02/2022 07:47, frank.heimes at canonical.com wrote:
>>>> From: Julian Wiedmann <jwi at linux.ibm.com>
>>>>
>>>> BugLink: https://bugs.launchpad.net/bugs/1959529
>>>>
>>>> Use dev_alloc_page() for backing the RX buffers with pages. This way we
>>>> pick up __GFP_MEMALLOC.
>>>>
>>>> Signed-off-by: Julian Wiedmann <jwi at linux.ibm.com>
>>>> Signed-off-by: David S. Miller <davem at davemloft.net>
>>>> (backported from commit 714c9108851743bb718fbc1bfb81290f12a53854)
>>>> Signed-off-by: Alexandra Winter <wintera at linux.ibm.com>
>>>> Signed-off-by: Frank Heimes <frank.heimes at canonical.com>
>>>> ---
>>>>  drivers/s390/net/qeth_core_main.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
>>>> index ec8c7a640d9e..61372e5c279b 100644
>>>> --- a/drivers/s390/net/qeth_core_main.c
>>>> +++ b/drivers/s390/net/qeth_core_main.c
>>>> @@ -227,7 +227,7 @@ static int qeth_alloc_buffer_pool(struct qeth_card *card)
>>>>  			return -ENOMEM;
>>>>  		}
>>>>  		for (j = 0; j < QETH_MAX_BUFFER_ELEMENTS(card); ++j) {
>>>> -			ptr = (void *) __get_free_page(GFP_KERNEL);
>>>> +			ptr = (void *) __dev_alloc_page(GFP_KERNEL);
>>>
>>
>> Or since the whole point is to use __GPF_MEMALLOC, simply do:
>>
> 
> It is not exact equivalent of __dev_alloc_page (__get_free_page() picks
> closest NUMA node, __dev_alloc_page() loads entire NUMA policy and
> chooses node according to it) , but I guess the differences here do not
> matter.
> 
> The MEMALLOC, which allows using emergency memory pools and should not
> be used outside of memory management subsystem or outside paths freeing
> memory, seems a weird solution to reported problem of lack of memory.
> The Bug report is saying that memory allocation failed, without too many
> details so probably because of lack of memory in the system. :) This is
> a RX buffer. Allocating a generic network RX buffer in case of
> out-of-memory situation from emergency pools does not fit into MEMALLOC
> purpose...
> 
> However if IBM feels it is proper approach for their problem, let's go
> with simply MEMALLOC flag, like Thadeu suggested.
> 

Alexandra Winter from IBM proposed to pick up only second part of the
patch which looks correct. This fixes their issue, see:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1959529/comments/7

However picking up only half of a stable commit will confuse some stable
scripts (e.g. stable-commit-in-tree from stable-tools).

Since we skip parts of the commit, I propose to bring it as "UBUNTU:
SAUCE:" patch by us, with a similar title/explanation, but not exactly
the same.

Any comments on such approach?

Best regards,
Krzysztof