Ipmi modules and linux-4.19.1

Ipmi modules and linux-4.19.1

[Angel Shtilianov]

Hi everybody.
A couple of days I've decided to migrate several servers on
linux-4.19. What I've observed is that I have no /dev/ipmi. After
taking a look into the boot log I've found that ipmi modules are
complaining about percpu memory allocation failures:
https://pastebin.com/MCDssZzV
However, I've fixed my ipmi settings using older kernel, but I did
some research about the issue.
I had to increase the PERCPU_MODULE_RESERVE and PCPU_MIN_UNIT_SIZE in
order to get the ipmi modules loaded.
include/linux/percpu.h

-#define PERCPU_MODULE_RESERVE          (8 << 10)
+#define PERCPU_MODULE_RESERVE          (8 << 11)

-#define PCPU_MIN_UNIT_SIZE             PFN_ALIGN(32 << 10)
+#define PCPU_MIN_UNIT_SIZE             PFN_ALIGN(32 << 11)

-#define PERCPU_DYNAMIC_EARLY_SIZE      (12 << 10)
+#define PERCPU_DYNAMIC_EARLY_SIZE      (12 << 11)

-#define PERCPU_DYNAMIC_RESERVE         (28 << 10)
+#define PERCPU_DYNAMIC_RESERVE         (28 << 11)

Any suggestions ?
Is it a mm issue or this is a module subsystem issue ?
Shouldn't it fall back?

Best regards,
Angel Shtilianov

Re: Ipmi modules and linux-4.19.1

[Tejun Heo]

Hello, Angel.

(cc'ing Paul for SRCU)

On Thu, Dec 20, 2018 at 09:55:10AM +0200, Angel Shtilianov wrote:
> Hi everybody.
> A couple of days I've decided to migrate several servers on
> linux-4.19. What I've observed is that I have no /dev/ipmi. After
> taking a look into the boot log I've found that ipmi modules are
> complaining about percpu memory allocation failures:
> https://pastebin.com/MCDssZzV
...
> -#define PERCPU_DYNAMIC_RESERVE         (28 << 10)
> +#define PERCPU_DYNAMIC_RESERVE         (28 << 11)

So, you prolly just needed to bump this number.  The reserved percpu
area is used to accommodate static percpu variables used by modules.
They are special because code generation assumes static symbols aren't
too far from the program counter.  The usual dynamic percpu area is
way high up in vmalloc area, so if we put static percpu allocations
there, they go out of range for module symbol relocations.

The reserved area has some issues.

1. The area is not dynamically mapped, meaning that however much we
   reserve is hard allocated on boot for future module uses, so we
   don't can't increase it willy-nilly.

2. There is no mechanism to adjust the size dynamically.  28k is just
   a number I pulled out of my ass after looking at some common
   configs like a decade ago, so it being low now isn't too
   surprising.  Provided that we can't make it run-time dynamic (and I
   can't think of a way to do that), the right thing to do would be
   sizing it during build with some buffer and allow it to be
   overridden boot time.  This is definitely doable.

BTW, ipmi's extra usage, 8k, is coming from the use of static SRCU.
Paul, that's quite a bit of percpu memory to reserve statically.
Would it be possible to make srcu_struct init dynamic so that it can
use the normal percpu_alloc?  That way, this problem can be completely
side-stepped and it only occupies percpu memory which tends to be
pretty expensive unless ipmi is actually initialized.

Thanks.

-- 
tejun

Re: Ipmi modules and linux-4.19.1

[Tejun Heo]

(cc'ing Corey and quoting whole body)

On Thu, Dec 20, 2018 at 07:42:17AM -0800, Tejun Heo wrote:
> Hello, Angel.
> 
> (cc'ing Paul for SRCU)
> 
> On Thu, Dec 20, 2018 at 09:55:10AM +0200, Angel Shtilianov wrote:
> > Hi everybody.
> > A couple of days I've decided to migrate several servers on
> > linux-4.19. What I've observed is that I have no /dev/ipmi. After
> > taking a look into the boot log I've found that ipmi modules are
> > complaining about percpu memory allocation failures:
> > https://pastebin.com/MCDssZzV
> ...
> > -#define PERCPU_DYNAMIC_RESERVE         (28 << 10)
> > +#define PERCPU_DYNAMIC_RESERVE         (28 << 11)
> 
> So, you prolly just needed to bump this number.  The reserved percpu
> area is used to accommodate static percpu variables used by modules.
> They are special because code generation assumes static symbols aren't
> too far from the program counter.  The usual dynamic percpu area is
> way high up in vmalloc area, so if we put static percpu allocations
> there, they go out of range for module symbol relocations.
> 
> The reserved area has some issues.
> 
> 1. The area is not dynamically mapped, meaning that however much we
>    reserve is hard allocated on boot for future module uses, so we
>    don't can't increase it willy-nilly.
> 
> 2. There is no mechanism to adjust the size dynamically.  28k is just
>    a number I pulled out of my ass after looking at some common
>    configs like a decade ago, so it being low now isn't too
>    surprising.  Provided that we can't make it run-time dynamic (and I
>    can't think of a way to do that), the right thing to do would be
>    sizing it during build with some buffer and allow it to be
>    overridden boot time.  This is definitely doable.
> 
> BTW, ipmi's extra usage, 8k, is coming from the use of static SRCU.
> Paul, that's quite a bit of percpu memory to reserve statically.
> Would it be possible to make srcu_struct init dynamic so that it can
> use the normal percpu_alloc?  That way, this problem can be completely
> side-stepped and it only occupies percpu memory which tends to be
> pretty expensive unless ipmi is actually initialized.

So, the transition to SRCU was fairly recent and seems kinda overkill.
This code path isn't expected to be high frequency && concurrency.  Is
the SRCU usage justified here?  Looks like it could have trivially
used a little bit finer grained locking and/or straight-forward
reference count.

Thanks.

-- 
tejun

Re: Ipmi modules and linux-4.19.1

[Paul E. McKenney]

On Thu, Dec 20, 2018 at 07:42:17AM -0800, Tejun Heo wrote:
> Hello, Angel.
> 
> (cc'ing Paul for SRCU)
> 
> On Thu, Dec 20, 2018 at 09:55:10AM +0200, Angel Shtilianov wrote:
> > Hi everybody.
> > A couple of days I've decided to migrate several servers on
> > linux-4.19. What I've observed is that I have no /dev/ipmi. After
> > taking a look into the boot log I've found that ipmi modules are
> > complaining about percpu memory allocation failures:
> > https://pastebin.com/MCDssZzV
> ...
> > -#define PERCPU_DYNAMIC_RESERVE         (28 << 10)
> > +#define PERCPU_DYNAMIC_RESERVE         (28 << 11)
> 
> So, you prolly just needed to bump this number.  The reserved percpu
> area is used to accommodate static percpu variables used by modules.
> They are special because code generation assumes static symbols aren't
> too far from the program counter.  The usual dynamic percpu area is
> way high up in vmalloc area, so if we put static percpu allocations
> there, they go out of range for module symbol relocations.
> 
> The reserved area has some issues.
> 
> 1. The area is not dynamically mapped, meaning that however much we
>    reserve is hard allocated on boot for future module uses, so we
>    don't can't increase it willy-nilly.
> 
> 2. There is no mechanism to adjust the size dynamically.  28k is just
>    a number I pulled out of my ass after looking at some common
>    configs like a decade ago, so it being low now isn't too
>    surprising.  Provided that we can't make it run-time dynamic (and I
>    can't think of a way to do that), the right thing to do would be
>    sizing it during build with some buffer and allow it to be
>    overridden boot time.  This is definitely doable.
> 
> BTW, ipmi's extra usage, 8k, is coming from the use of static SRCU.
> Paul, that's quite a bit of percpu memory to reserve statically.
> Would it be possible to make srcu_struct init dynamic so that it can
> use the normal percpu_alloc?  That way, this problem can be completely
> side-stepped and it only occupies percpu memory which tends to be
> pretty expensive unless ipmi is actually initialized.

Yes, it is possible.  Just do something like this:

	struct srcu_struct my_srcu_struct;

And before the first use of my_srcu_struct, do this:

	init_srcu_struct(&my_srcu_struct);

This will result in alloc_percpu() being invoked to allocate the
needed per-CPU space.

If my_srcu_struct is used in a module or some such, then to avoid memory
leaks, after the last use of my_srcu_struct, do this:

	cleanup_srcu_struct(&my_srcu_struct);

There are several places in the kernel that take this approach.

							Thanx, Paul

Re: Ipmi modules and linux-4.19.1

[Paul E. McKenney]

Also adding Corey.  ;-)

On Thu, Dec 20, 2018 at 08:03:13AM -0800, Paul E. McKenney wrote:
> On Thu, Dec 20, 2018 at 07:42:17AM -0800, Tejun Heo wrote:
> > Hello, Angel.
> > 
> > (cc'ing Paul for SRCU)
> > 
> > On Thu, Dec 20, 2018 at 09:55:10AM +0200, Angel Shtilianov wrote:
> > > Hi everybody.
> > > A couple of days I've decided to migrate several servers on
> > > linux-4.19. What I've observed is that I have no /dev/ipmi. After
> > > taking a look into the boot log I've found that ipmi modules are
> > > complaining about percpu memory allocation failures:
> > > https://pastebin.com/MCDssZzV
> > ...
> > > -#define PERCPU_DYNAMIC_RESERVE         (28 << 10)
> > > +#define PERCPU_DYNAMIC_RESERVE         (28 << 11)
> > 
> > So, you prolly just needed to bump this number.  The reserved percpu
> > area is used to accommodate static percpu variables used by modules.
> > They are special because code generation assumes static symbols aren't
> > too far from the program counter.  The usual dynamic percpu area is
> > way high up in vmalloc area, so if we put static percpu allocations
> > there, they go out of range for module symbol relocations.
> > 
> > The reserved area has some issues.
> > 
> > 1. The area is not dynamically mapped, meaning that however much we
> >    reserve is hard allocated on boot for future module uses, so we
> >    don't can't increase it willy-nilly.
> > 
> > 2. There is no mechanism to adjust the size dynamically.  28k is just
> >    a number I pulled out of my ass after looking at some common
> >    configs like a decade ago, so it being low now isn't too
> >    surprising.  Provided that we can't make it run-time dynamic (and I
> >    can't think of a way to do that), the right thing to do would be
> >    sizing it during build with some buffer and allow it to be
> >    overridden boot time.  This is definitely doable.
> > 
> > BTW, ipmi's extra usage, 8k, is coming from the use of static SRCU.
> > Paul, that's quite a bit of percpu memory to reserve statically.
> > Would it be possible to make srcu_struct init dynamic so that it can
> > use the normal percpu_alloc?  That way, this problem can be completely
> > side-stepped and it only occupies percpu memory which tends to be
> > pretty expensive unless ipmi is actually initialized.
> 
> Yes, it is possible.  Just do something like this:
> 
> 	struct srcu_struct my_srcu_struct;
> 
> And before the first use of my_srcu_struct, do this:
> 
> 	init_srcu_struct(&my_srcu_struct);
> 
> This will result in alloc_percpu() being invoked to allocate the
> needed per-CPU space.
> 
> If my_srcu_struct is used in a module or some such, then to avoid memory
> leaks, after the last use of my_srcu_struct, do this:
> 
> 	cleanup_srcu_struct(&my_srcu_struct);
> 
> There are several places in the kernel that take this approach.
> 
> 							Thanx, Paul

Re: Ipmi modules and linux-4.19.1

[Tejun Heo]

Hello,

On Thu, Dec 20, 2018 at 08:04:08AM -0800, Paul E. McKenney wrote:
> > Yes, it is possible.  Just do something like this:
> > 
> > 	struct srcu_struct my_srcu_struct;
> > 
> > And before the first use of my_srcu_struct, do this:
> > 
> > 	init_srcu_struct(&my_srcu_struct);
> > 
> > This will result in alloc_percpu() being invoked to allocate the
> > needed per-CPU space.
> > 
> > If my_srcu_struct is used in a module or some such, then to avoid memory
> > leaks, after the last use of my_srcu_struct, do this:
> > 
> > 	cleanup_srcu_struct(&my_srcu_struct);
> > 
> > There are several places in the kernel that take this approach.

Oops, my bad.  Somehow I thought the dynamic init didn't exist (I
checked the header but somehow completely skipped over them).  Thanks
for the explanation!

-- 
tejun

Re: Ipmi modules and linux-4.19.1

[Paul E. McKenney]

On Thu, Dec 20, 2018 at 08:05:14AM -0800, Tejun Heo wrote:
> Hello,
> 
> On Thu, Dec 20, 2018 at 08:04:08AM -0800, Paul E. McKenney wrote:
> > > Yes, it is possible.  Just do something like this:
> > > 
> > > 	struct srcu_struct my_srcu_struct;
> > > 
> > > And before the first use of my_srcu_struct, do this:
> > > 
> > > 	init_srcu_struct(&my_srcu_struct);
> > > 
> > > This will result in alloc_percpu() being invoked to allocate the
> > > needed per-CPU space.
> > > 
> > > If my_srcu_struct is used in a module or some such, then to avoid memory
> > > leaks, after the last use of my_srcu_struct, do this:
> > > 
> > > 	cleanup_srcu_struct(&my_srcu_struct);
> > > 
> > > There are several places in the kernel that take this approach.
> 
> Oops, my bad.  Somehow I thought the dynamic init didn't exist (I
> checked the header but somehow completely skipped over them).  Thanks
> for the explanation!

No problem, especially given that if things go as they usually do, I
will provide you ample opportunity to return the favor.  ;-)

							Thanx, Paul

Re: Ipmi modules and linux-4.19.1

[Corey Minyard]

On 12/20/18 10:22 AM, Paul E. McKenney wrote:
> On Thu, Dec 20, 2018 at 08:05:14AM -0800, Tejun Heo wrote:
>> Hello,
>>
>> On Thu, Dec 20, 2018 at 08:04:08AM -0800, Paul E. McKenney wrote:
>>>> Yes, it is possible.  Just do something like this:
>>>>
>>>> 	struct srcu_struct my_srcu_struct;
>>>>
>>>> And before the first use of my_srcu_struct, do this:
>>>>
>>>> 	init_srcu_struct(&my_srcu_struct);
>>>>
>>>> This will result in alloc_percpu() being invoked to allocate the
>>>> needed per-CPU space.
>>>>
>>>> If my_srcu_struct is used in a module or some such, then to avoid memory
>>>> leaks, after the last use of my_srcu_struct, do this:
>>>>
>>>> 	cleanup_srcu_struct(&my_srcu_struct);
>>>>
>>>> There are several places in the kernel that take this approach.
>> Oops, my bad.  Somehow I thought the dynamic init didn't exist (I
>> checked the header but somehow completely skipped over them).  Thanks
>> for the explanation!
> No problem, especially given that if things go as they usually do, I
> will provide you ample opportunity to return the favor.  ;-)

Ok, I didn't realize that SRCU took up so much space.  It's true that 
this isn't
something that requires performance, but SRCU was awfully convenient to
use for this.

Unfortunately, it's not just a matter of adding the init_srcu_struct() 
to the
__init function.  I'm going to have to hunt down all the initial startup 
points
and add it there, and rework some of the other initialization code..  
But that's
something I can do.

Unless someone else would rather do it :-).

-corey