2021-02-22 07:13:57

by Coiby Xu

[permalink] [raw]
Subject: [RFC PATCH 4/4] i40e: don't open i40iw client for kdump

i40iw consumes huge amounts of memory. For example, on a x86_64 machine,
i40iw consumed 1.5GB for Intel Corporation Ethernet Connection X722 for
for 1GbE while "craskernel=auto" only reserved 160M. With the module
parameter "resource_profile=2", we can reduce the memory usage of i40iw
to ~300M which is still too much for kdump.

Disabling the client registration would spare us the client interface
operation open , i.e., i40iw_open for iwarp/uda device. Thus memory is
saved for kdump.

Signed-off-by: Coiby Xu <[email protected]>
---
drivers/net/ethernet/intel/i40e/i40e_client.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
index a2dba32383f6..aafc2587f389 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -4,6 +4,7 @@
#include <linux/list.h>
#include <linux/errno.h>
#include <linux/net/intel/i40e_client.h>
+#include <linux/crash_dump.h>

#include "i40e.h"
#include "i40e_prototype.h"
@@ -741,6 +742,12 @@ int i40e_register_client(struct i40e_client *client)
{
int ret = 0;

+ /* Don't open i40iw client for kdump because i40iw will consume huge
+ * amounts of memory.
+ */
+ if (is_kdump_kernel())
+ return ret;
+
if (!client) {
ret = -EIO;
goto out;
--
2.30.1


2021-02-24 00:00:25

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [RFC PATCH 4/4] i40e: don't open i40iw client for kdump

On Mon, 22 Feb 2021 15:07:01 +0800 Coiby Xu wrote:
> i40iw consumes huge amounts of memory. For example, on a x86_64 machine,
> i40iw consumed 1.5GB for Intel Corporation Ethernet Connection X722 for
> for 1GbE while "craskernel=auto" only reserved 160M. With the module
> parameter "resource_profile=2", we can reduce the memory usage of i40iw
> to ~300M which is still too much for kdump.
>
> Disabling the client registration would spare us the client interface
> operation open , i.e., i40iw_open for iwarp/uda device. Thus memory is
> saved for kdump.
>
> Signed-off-by: Coiby Xu <[email protected]>

Is i40iw or whatever the client is not itself under a CONFIG which
kdump() kernels could be reasonably expected to disable?

2021-02-24 11:46:35

by Coiby Xu

[permalink] [raw]
Subject: Re: [RFC PATCH 4/4] i40e: don't open i40iw client for kdump

Hi Jakub,

Thank you for reviewing the patch!

On Tue, Feb 23, 2021 at 12:22:07PM -0800, Jakub Kicinski wrote:
>On Mon, 22 Feb 2021 15:07:01 +0800 Coiby Xu wrote:
>> i40iw consumes huge amounts of memory. For example, on a x86_64 machine,
>> i40iw consumed 1.5GB for Intel Corporation Ethernet Connection X722 for
>> for 1GbE while "craskernel=auto" only reserved 160M. With the module
>> parameter "resource_profile=2", we can reduce the memory usage of i40iw
>> to ~300M which is still too much for kdump.
>>
>> Disabling the client registration would spare us the client interface
>> operation open , i.e., i40iw_open for iwarp/uda device. Thus memory is
>> saved for kdump.
>>
>> Signed-off-by: Coiby Xu <[email protected]>
>
>Is i40iw or whatever the client is not itself under a CONFIG which
>kdump() kernels could be reasonably expected to disable?
>

I'm not sure if I understand you correctly. Do you mean we shouldn't
disable i40iw for kdump?

--
Best regards,
Coiby

2021-02-25 01:11:56

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [RFC PATCH 4/4] i40e: don't open i40iw client for kdump

On Wed, 24 Feb 2021 19:41:41 +0800 Coiby Xu wrote:
> On Tue, Feb 23, 2021 at 12:22:07PM -0800, Jakub Kicinski wrote:
> >On Mon, 22 Feb 2021 15:07:01 +0800 Coiby Xu wrote:
> >> i40iw consumes huge amounts of memory. For example, on a x86_64 machine,
> >> i40iw consumed 1.5GB for Intel Corporation Ethernet Connection X722 for
> >> for 1GbE while "craskernel=auto" only reserved 160M. With the module
> >> parameter "resource_profile=2", we can reduce the memory usage of i40iw
> >> to ~300M which is still too much for kdump.
> >>
> >> Disabling the client registration would spare us the client interface
> >> operation open , i.e., i40iw_open for iwarp/uda device. Thus memory is
> >> saved for kdump.
> >>
> >> Signed-off-by: Coiby Xu <[email protected]>
> >
> >Is i40iw or whatever the client is not itself under a CONFIG which
> >kdump() kernels could be reasonably expected to disable?
> >
>
> I'm not sure if I understand you correctly. Do you mean we shouldn't
> disable i40iw for kdump?

Forgive my ignorance - are the kdump kernels separate builds?

If they are it'd be better to leave the choice of enabling RDMA
to the user - through appropriate Kconfig options.

2021-02-25 05:00:27

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [RFC PATCH 4/4] i40e: don't open i40iw client for kdump

On Thu, 25 Feb 2021 08:21:01 +0800 Coiby Xu wrote:
> On Wed, Feb 24, 2021 at 08:48:41AM -0800, Jakub Kicinski wrote:
> >On Wed, 24 Feb 2021 19:41:41 +0800 Coiby Xu wrote:
> >> I'm not sure if I understand you correctly. Do you mean we shouldn't
> >> disable i40iw for kdump?
> >
> >Forgive my ignorance - are the kdump kernels separate builds?
>
> AFAIK we don't build a kernel exclusively for kdump.
>
> >If they are it'd be better to leave the choice of enabling RDMA
> >to the user - through appropriate Kconfig options.
>
> i40iw is usually built as a loadable module. So if we want to leave the
> choce of enabling RDMA to the user, we could exclude this driver when
> building the initramfs for kdump, for example, dracut provides the
> omit_drivers option for this purpose.
>
> On the other hand, the users expect "crashkernel=auto" to work out of
> the box. So i40iw defeats this purpose.
>
> I'll discuss with my Red Hat team and the Intel team about whether RDMA
> is needed for kdump. Thanks for bringing up this issue!

Great, talking to experts here at FB it seems that building a cut-down
kernel for kdump is easier than chasing all the drivers to react to
is_kdump_kernel(). But if you guys need it and Intel is fine with
the change I won't complain.

2021-02-25 08:16:35

by Coiby Xu

[permalink] [raw]
Subject: Re: [RFC PATCH 4/4] i40e: don't open i40iw client for kdump

On Wed, Feb 24, 2021 at 08:48:41AM -0800, Jakub Kicinski wrote:
>On Wed, 24 Feb 2021 19:41:41 +0800 Coiby Xu wrote:
>> On Tue, Feb 23, 2021 at 12:22:07PM -0800, Jakub Kicinski wrote:
>> >On Mon, 22 Feb 2021 15:07:01 +0800 Coiby Xu wrote:
>> >> i40iw consumes huge amounts of memory. For example, on a x86_64 machine,
>> >> i40iw consumed 1.5GB for Intel Corporation Ethernet Connection X722 for
>> >> for 1GbE while "craskernel=auto" only reserved 160M. With the module
>> >> parameter "resource_profile=2", we can reduce the memory usage of i40iw
>> >> to ~300M which is still too much for kdump.
>> >>
>> >> Disabling the client registration would spare us the client interface
>> >> operation open , i.e., i40iw_open for iwarp/uda device. Thus memory is
>> >> saved for kdump.
>> >>
>> >> Signed-off-by: Coiby Xu <[email protected]>
>> >
>> >Is i40iw or whatever the client is not itself under a CONFIG which
>> >kdump() kernels could be reasonably expected to disable?
>> >
>>
>> I'm not sure if I understand you correctly. Do you mean we shouldn't
>> disable i40iw for kdump?
>
>Forgive my ignorance - are the kdump kernels separate builds?
>

AFAIK we don't build a kernel exclusively for kdump.

>If they are it'd be better to leave the choice of enabling RDMA
>to the user - through appropriate Kconfig options.
>

i40iw is usually built as a loadable module. So if we want to leave the
choce of enabling RDMA to the user, we could exclude this driver when
building the initramfs for kdump, for example, dracut provides the
omit_drivers option for this purpose.

On the other hand, the users expect "crashkernel=auto" to work out of
the box. So i40iw defeats this purpose.

I'll discuss with my Red Hat team and the Intel team about whether RDMA
is needed for kdump. Thanks for bringing up this issue!

--
Best regards,
Coiby

2021-02-25 10:36:35

by Bhupesh Sharma

[permalink] [raw]
Subject: Re: [RFC PATCH 4/4] i40e: don't open i40iw client for kdump

Hello Coiby,

On Mon, Feb 22, 2021 at 12:40 PM Coiby Xu <[email protected]> wrote:
>
> i40iw consumes huge amounts of memory. For example, on a x86_64 machine,
> i40iw consumed 1.5GB for Intel Corporation Ethernet Connection X722 for
> for 1GbE while "craskernel=auto" only reserved 160M. With the module
> parameter "resource_profile=2", we can reduce the memory usage of i40iw
> to ~300M which is still too much for kdump.
>
> Disabling the client registration would spare us the client interface
> operation open , i.e., i40iw_open for iwarp/uda device. Thus memory is
> saved for kdump.
>
> Signed-off-by: Coiby Xu <[email protected]>
> ---
> drivers/net/ethernet/intel/i40e/i40e_client.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
> index a2dba32383f6..aafc2587f389 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_client.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
> @@ -4,6 +4,7 @@
> #include <linux/list.h>
> #include <linux/errno.h>
> #include <linux/net/intel/i40e_client.h>
> +#include <linux/crash_dump.h>
>
> #include "i40e.h"
> #include "i40e_prototype.h"
> @@ -741,6 +742,12 @@ int i40e_register_client(struct i40e_client *client)
> {
> int ret = 0;
>
> + /* Don't open i40iw client for kdump because i40iw will consume huge
> + * amounts of memory.
> + */
> + if (is_kdump_kernel())
> + return ret;
> +

Since crashkernel size can be manually set on the command line by a
user, and some users might be fine with a ~300M memory usage by i40iw
client [with resource_profile=2"], in my view, disabling the client
for all kdump cases seems too restrictive.

We can probably check the crash kernel size allocated (
$ cat /sys/kernel/kexec_crash_size) and then make a decision
accordingly, so for example something like:

+ if (is_kdump_kernel() && kexec_crash_size < 512M)
+ return ret;

What do you think?

Regards,
Bhupesh

> if (!client) {
> ret = -EIO;
> goto out;
> --
> 2.30.1
>
>
> _______________________________________________
> kexec mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/kexec

2021-03-04 09:48:20

by Coiby Xu

[permalink] [raw]
Subject: Re: [RFC PATCH 4/4] i40e: don't open i40iw client for kdump

Hi Bhupesh,

Glad to meet you here:)

On Thu, Feb 25, 2021 at 03:41:55PM +0530, Bhupesh SHARMA wrote:
>Hello Coiby,
>
>On Mon, Feb 22, 2021 at 12:40 PM Coiby Xu <[email protected]> wrote:
>>
>> i40iw consumes huge amounts of memory. For example, on a x86_64 machine,
>> i40iw consumed 1.5GB for Intel Corporation Ethernet Connection X722 for
>> for 1GbE while "craskernel=auto" only reserved 160M. With the module
>> parameter "resource_profile=2", we can reduce the memory usage of i40iw
>> to ~300M which is still too much for kdump.
>>
>> Disabling the client registration would spare us the client interface
>> operation open , i.e., i40iw_open for iwarp/uda device. Thus memory is
>> saved for kdump.
>>
>> Signed-off-by: Coiby Xu <[email protected]>
>> ---
>> drivers/net/ethernet/intel/i40e/i40e_client.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
>> index a2dba32383f6..aafc2587f389 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_client.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
>> @@ -4,6 +4,7 @@
>> #include <linux/list.h>
>> #include <linux/errno.h>
>> #include <linux/net/intel/i40e_client.h>
>> +#include <linux/crash_dump.h>
>>
>> #include "i40e.h"
>> #include "i40e_prototype.h"
>> @@ -741,6 +742,12 @@ int i40e_register_client(struct i40e_client *client)
>> {
>> int ret = 0;
>>
>> + /* Don't open i40iw client for kdump because i40iw will consume huge
>> + * amounts of memory.
>> + */
>> + if (is_kdump_kernel())
>> + return ret;
>> +
>
>Since crashkernel size can be manually set on the command line by a
>user, and some users might be fine with a ~300M memory usage by i40iw
>client [with resource_profile=2"], in my view, disabling the client
>for all kdump cases seems too restrictive.
>
>We can probably check the crash kernel size allocated (
>$ cat /sys/kernel/kexec_crash_size) and then make a decision
>accordingly, so for example something like:
>
> + if (is_kdump_kernel() && kexec_crash_size < 512M)
> + return ret;
>
>What do you think?
>

Thanks for the suggestion! After having a discussion with the team, we
think it's better to not intervene i40iw in the kernel space. Actually
when kexec-tools is building initramfs for kdump, i40iw is not included
by default unless a user explicitly asks to include i40iw by changing
/etc/kdump.conf, i.e., adding 'dracut_args --add-drivers "i40iw"'.


>Regards,
>Bhupesh
>
>> if (!client) {
>> ret = -EIO;
>> goto out;
>> --
>> 2.30.1
>>
>>
>> _______________________________________________
>> kexec mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/kexec
>

--
Best regards,
Coiby