large frame size warning when compiling
Subsystems (2)
Hello list, Time for a noob discussion,
When I was building the kernel, I found a warning from drivers/mfd/abx500-core.c, that the Frame size is larger than 1024 bytes. Apparently the stack frame size can be changed from the config, but my question is, whether 1024 bytes low ? I am on an x86_64 (core i3).
abx500-core.c had an object of struct device being allocated on stack. So dynamically allocating it makes the warning go away. Are there any implications on using dynamic allocation on this particular code? I mean, could there be some reason why the original developer went with static option ?
--- drivers/mfd/abx500-core.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/mfd/abx500-core.c +5 -3
@@ -154,15 +154,17 @@ EXPORT_SYMBOL(abx500_startup_irq_enabled);void abx500_dump_all_banks (void ) {struct abx500_ops *ops; -struct device dummy_child = {NULL }; +struct device *dummy_child;struct abx500_device_entry *dev_entry; + dummy_child =kzalloc (sizeof (struct device ),GFP_KERNEL ); +list_for_each_entry (dev_entry, &abx500_list, list) { - dummy_child.parent = dev_entry->dev; + dummy_child->parent = dev_entry->dev; ops = &dev_entry->ops;if ((ops !=NULL ) && (ops->dump_all_banks !=NULL )) - ops->dump_all_banks (&dummy_child); + ops->dump_all_banks (dummy_child); } }EXPORT_SYMBOL (abx500_dump_all_banks);
Signature · 13 lines
1.9.1 -- Thanks and Regards, Jay Aurabind -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 213 bytes Desc: Digital signature Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140507/a7231430/attachment.bin
Quoted 4 lines · Jay Aurabind
When I was building the kernel, I found a warning from
drivers/mfd/abx500-core.c, that the Frame size is larger than 1024 bytes. Apparently the stack frame size can be changed from the config, but my question is, whether 1024 bytes low ? I am on anx86_64(core i3).
Allocating 1K on the stack is indeed evil.
Quoted 3 lines
abx500-core.chad an object of struct device being allocated on stack. So dynamically allocating it makes the warning go away. Are there any implications on using dynamic allocation on this particular code?
He probably didn't realize or didn't know better.
Having said that:
drivers/mfd/abx500-core.c EXPORT_SYMBOL(abx500_startup_irq_enabled); · line 160
Quoted 1 line
drivers/mfd/abx500-core.c +1
-0+ dummy_child =kzalloc (sizeof (struct device ),GFP_KERNEL );
There's no kfree() for this. So you introduced a memory leak.
drivers/mfd/abx500-core.c EXPORT_SYMBOL(abx500_startup_irq_enabled); · line 168
Quoted 9 lines
drivers/mfd/abx500-core.c +2
-2list_for_each_entry (dev_entry, &abx500_list, list) { - dummy_child.parent = dev_entry->dev; + dummy_child->parent = dev_entry->dev; ops = &dev_entry->ops;if ((ops !=NULL ) && (ops->dump_all_banks !=NULL )) - ops->dump_all_banks (&dummy_child); + ops->dump_all_banks (dummy_child); }
kfree(dummy_child); /* should go here... */
Quoted 2 lines
}
EXPORT_SYMBOL(abx500_dump_all_banks);
The weird part is that the entries on abx500_list apparently don't have valid ->parent pointers already, so we have have to invent dummy ones.
Anybody understand why that's the case? This smells like we're not fixing the actual problem here, just changing the way we paper it over to be a less ugly papering over....
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140507/de99ca63/attachment.bin
Quoted 3 lines
Allocating 1K on the stack is indeed evil.
Why would you say evil ? I didnt quite get why you meant by that. Is it at some extremes ? 1K is from the default ubuntu 14.04 config.
Quoted 5 lines
Quoted 4 lines
abx500-core.chad an object of struct device being allocated on stack. So dynamically allocating it makes the warning go away. Are there any implications on using dynamic allocation on this particular code?
Quoted 7 lines
He probably didn't realize or didn't know better.
Having said that:
drivers/mfd/abx500-core.c EXPORT_SYMBOL(abx500_startup_irq_enabled); · line 160
Quoted 1 line
drivers/mfd/abx500-core.c +1
-0+ dummy_child =kzalloc (sizeof (struct device ),GFP_KERNEL );There's no
kfree()for this. So you introduced a memory leak.
My bad! "I successfully operated the patient, but forgot the scissors inside", :D Thanks for pointing out! :)
Quoted 15 lines
drivers/mfd/abx500-core.c EXPORT_SYMBOL(abx500_startup_irq_enabled); · line 168
Quoted 10 lines
drivers/mfd/abx500-core.c +2
-2list_for_each_entry (dev_entry, &abx500_list, list) { - dummy_child.parent = dev_entry->dev; + dummy_child->parent = dev_entry->dev; ops = &dev_entry->ops;if ((ops !=NULL ) && (ops->dump_all_banks !=NULL )) - ops->dump_all_banks (&dummy_child); + ops->dump_all_banks (dummy_child); }kfree(dummy_child); /* should go here... */Quoted 2 lines
}
EXPORT_SYMBOL(abx500_dump_all_banks);
Quoted 8 lines
The weird part is that the entries on
abx500_listapparently don't have valid ->parent pointers already, so we have have to invent dummy ones.Anybody understand why that's the case? This smells like we're not fixing the actual problem here, just changing the way we paper it over to be a less ugly papering over....
Waiting to hear from the experts!
Quoted 6 lines
_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 278 bytes
Desc: OpenPGP digital signature
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140508/53b0070f/attachment.bin
Quoted 4 lines · Jay Aurabind
Quoted 1 line
Allocating 1K on the stack is indeed evil.
Why would you say evil ? I didnt quite get why you meant by that. Is it at some extremes ? 1K is from the default ubuntu 14.04 config.
Some paths in the kernel get very deep stacks (for instance, nfs reading an xfs file that's on an lvm partition on a dm-crypt target is famous for blowing the stack without any further help). On my laptop, the low-water mark is already pretty low:
% dmesg | grep 'stack depth'
[ 2.094319] cryptomgr_test (42) used greatest stack depth: 6456 bytes left [ 2.952135] modprobe (91) used greatest stack depth: 5904 bytes left [ 2.955593] modprobe (93) used greatest stack depth: 5848 bytes left [ 2.955953] cryptomgr_probe (94) used greatest stack depth: 5520 bytes left [ 4.145573] systemd-cgroups (135) used greatest stack depth: 5440 bytes left [ 4.859260] udevadm (255) used greatest stack depth: 4920 bytes left [ 4.966646] ata_id (268) used greatest stack depth: 4088 bytes left [ 145.528777] dhclient (1306) used greatest stack depth: 4072 bytes left [ 226.188576] ip (2232) used greatest stack depth: 3344 bytes left [ 300.303981] ip (2468) used greatest stack depth: 3152 bytes left
So in the first 5 minutes, I already was down to 3K of stack. All it takes is that code interacting with another code that allocates 1K popping when I was at that 3K low water mark, and my kernel is in deep juju.
The fact the 1K is in the default Ubuntu config just means that the kernel as distributed has bad code in it. :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140508/bea0e8aa/attachment.bin
Quoted 23 lines · Valdis.Kletnieks at vt.edu
Quoted 4 lines · Jay Aurabind
Quoted 1 line
Allocating 1K on the stack is indeed evil.
Why would you say evil ? I didnt quite get why you meant by that. Is it at some extremes ? 1K is from the default ubuntu 14.04 config.
Some paths in the kernel get very deep stacks (for instance, nfs reading an xfs file that's on an lvm partition on a dm-crypt target is famous for blowing the stack without any further help). On my laptop, the low-water mark is already pretty low:
% dmesg | grep 'stack depth'
[ 2.094319] cryptomgr_test (42) used greatest stack depth: 6456 bytes left [ 2.952135] modprobe (91) used greatest stack depth: 5904 bytes left [ 2.955593] modprobe (93) used greatest stack depth: 5848 bytes left [ 2.955953] cryptomgr_probe (94) used greatest stack depth: 5520 bytes left [ 4.145573] systemd-cgroups (135) used greatest stack depth: 5440 bytes left [ 4.859260] udevadm (255) used greatest stack depth: 4920 bytes left [ 4.966646] ata_id (268) used greatest stack depth: 4088 bytes left [ 145.528777] dhclient (1306) used greatest stack depth: 4072 bytes left [ 226.188576] ip (2232) used greatest stack depth: 3344 bytes left [ 300.303981] ip (2468) used greatest stack depth: 3152 bytes left
Quoted 7 lines
So in the first 5 minutes, I already was down to 3K of stack. All it takes is that code interacting with another code that allocates 1K popping when I was at that 3K low water mark, and my kernel is in deep juju.
The fact the 1K is in the default Ubuntu config just means that the kernel as distributed has bad code in it. :)
Thank you Valdis for the explaination. My stack depth is also similar to yours:
[ 1.567353] cryptomgr_test (52) used greatest stack depth: 6656 bytes left [ 2.871152] modprobe (85) used greatest stack depth: 5624 bytes left [ 2.894478] modprobe (93) used greatest stack depth: 5552 bytes left [ 4.212907] all_generic_ide (127) used greatest stack depth: 5448 bytes left [ 4.440752] udevadm (134) used greatest stack depth: 4488 bytes left [ 4.571276] ata_id (165) used greatest stack depth: 4448 bytes left [ 7.661284] btrfs (183) used greatest stack depth: 4424 bytes left [ 7.859625] systemd-udevd (133) used greatest stack depth: 4392 bytes left [ 8.898118] hostname (208) used greatest stack depth: 4208 bytes left [ 9.341028] sh (209) used greatest stack depth: 4088 bytes left [ 48.353719] systemd-udevd (506) used greatest stack depth: 3768 bytes left [ 304.162368] kworker/u8:0 (6) used greatest stack depth: 3656 bytes left
Apparently no other activity is happening which is decreasing the depth further, even after running some applications like firefox. So shouldnt we assume that initial value (somewhere around 6K) should be enough since the maximum it went down is only till its 50% mark ?
Quoted 2 lines · Jay Aurabind
So shouldnt we assume that initial value (somewhere around 6K) should be enough since the maximum it went down is only till its 50% mark ?
Depends. Are you of the "we haven't seen it before, so it can't happen" school of programming, or the "it could plausibly happen, so we should guard against it" school?
Consider you get down to that 6K mark - and now you hit that 1K allocation
that you didn't bother cleaning up because "we've never seen it before". And
then you hit a hardIRQ that *also* didn't bother cleaning up their 1K allocation
that *that* kernel hacker "had never seen it before". What happens to your
system? And how long is it going to take for you to figure out why every
several weeks, your system dies with a totally different random memory
overlay?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 848 bytes
Desc: not available
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140509/e2d5874a/attachment.bin
Quoted 9 lines · Valdis.Kletnieks at vt.edu
Quoted 2 lines · Jay Aurabind
So shouldnt we assume that initial value (somewhere around 6K) should be enough since the maximum it went down is only till its 50% mark ?
Depends. Are you of the "we haven't seen it before, so it can't happen" school of programming, or the "it could plausibly happen, so we should guard against it" school?
Quoted 8 lines
Consider you get down to that 6K mark - and now you hit that 1K allocation that you didn't bother cleaning up because "we've never seen it before". And then you hit a hardIRQ that *also* didn't bother cleaning up their 1K allocation that *that* kernel hacker "had never seen it before". What happens to your system? And how long is it going to take for you to figure out why every several weeks, your system dies with a totally different random memory overlay?
Point taken. Thank you Valdis, Martin, Paul and Frank for sharing your thoughts.
Cheers,
Jay
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 278 bytes
Desc: OpenPGP digital signature
Url : http://lists.kernelnewbies.org/pipermail/kernelnewbies/attachments/20140510/a8dc2df7/attachment.bin
Quoted 13 lines · Valdis.Kletnieks at vt.edu
[...]
Some paths in the kernel get very deep stacks (for instance, nfs reading an xfs file that's on an lvm partition on a dm-crypt target is famous for blowing the stack without any further help). On my laptop, the low-water mark is already pretty low:% dmesg | grep 'stack depth'
[ 2.094319] cryptomgr_test (42) used greatest stack depth: 6456 bytes left [...] [ 145.528777] dhclient (1306) used greatest stack depth: 4072 bytes left [ 226.188576] ip (2232) used greatest stack depth: 3344 bytes left [ 300.303981] ip (2468) used greatest stack depth: 3152 bytes left
Quoted 3 lines
So in the first 5 minutes, I already was down to 3K of stack. All it takes is that code interacting with another code that allocates 1K popping when I was at that 3K low water mark, and my kernel is in deep juju.
Note that the kernel stack is not a monotonically exhausted resource. Each task has its own. It's freed up gradually as internal functions return, and fully as control returns to the user thread.
The reported numbers decrease monotonically only to show "worst-case-until-now". It's not "only this much space remains, reboot soon" kind of thing.
- FChE
Am 2014-05-07 18:36, schrieb Jay Aurabind:
Quoted 8 lines
Hello list, Time for a noob discussion,
When I was building the kernel, I found a warning from
drivers/mfd/abx500-core.c, that the Frame size is larger than 1024 bytes. Apparently the stack frame size can be changed from the config, but my question is, whether 1024 bytes low ? I am on anx86_64(core i3).
abx500-core.chad an object of struct device being allocated on stack. So dynamically allocating it makes the warning go away. Are there any implications on using dynamic allocation on this particular code? I mean, could there be some reason why the original developer went with static option ?
kmalloc may sleep and is therefore sometimes not recommended to be used.
Quoted 39 lines
--- drivers/mfd/abx500-core.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)drivers/mfd/abx500-core.c +5
-3@@ -154,15 +154,17 @@ EXPORT_SYMBOL(abx500_startup_irq_enabled);void abx500_dump_all_banks (void ) {struct abx500_ops *ops; -struct device dummy_child = {NULL }; +struct device *dummy_child;struct abx500_device_entry *dev_entry; + dummy_child =kzalloc (sizeof (struct device ),GFP_KERNEL ); +list_for_each_entry (dev_entry, &abx500_list, list) { - dummy_child.parent = dev_entry->dev; + dummy_child->parent = dev_entry->dev; ops = &dev_entry->ops;if ((ops !=NULL ) && (ops->dump_all_banks !=NULL )) - ops->dump_all_banks (&dummy_child); + ops->dump_all_banks (dummy_child); } }EXPORT_SYMBOL (abx500_dump_all_banks);_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Quoted 12 lines · Martin Kepplinger
Am 2014-05-07 18:36, schrieb Jay Aurabind:
Quoted 8 lines
Hello list, Time for a noob discussion,
When I was building the kernel, I found a warning from
drivers/mfd/abx500-core.c, that the Frame size is larger than 1024 bytes. Apparently the stack frame size can be changed from the config, but my question is, whether 1024 bytes low ? I am on anx86_64(core i3).
abx500-core.chad an object of struct device being allocated on stack. So dynamically allocating it makes the warning go away. Are there any implications on using dynamic allocation on this particular code? I mean, could there be some reason why the original developer went with static option ?kmalloc may sleep and is therefore sometimes not recommended to be used.
Sure, but this code doesnt appear to be holding any locks. So it should be okay to call kmalloc here I guess.
Quoted 40 lines
Quoted 39 lines
--- drivers/mfd/abx500-core.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)drivers/mfd/abx500-core.c +5
-3@@ -154,15 +154,17 @@ EXPORT_SYMBOL(abx500_startup_irq_enabled);void abx500_dump_all_banks (void ) {struct abx500_ops *ops; -struct device dummy_child = {NULL }; +struct device *dummy_child;struct abx500_device_entry *dev_entry; + dummy_child =kzalloc (sizeof (struct device ),GFP_KERNEL ); +list_for_each_entry (dev_entry, &abx500_list, list) { - dummy_child.parent = dev_entry->dev; + dummy_child->parent = dev_entry->dev; ops = &dev_entry->ops;if ((ops !=NULL ) && (ops->dump_all_banks !=NULL )) - ops->dump_all_banks (&dummy_child); + ops->dump_all_banks (dummy_child); } }EXPORT_SYMBOL (abx500_dump_all_banks);_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Quoted 6 lines · Jay Aurabind
Quoted 5 lines · Martin Kepplinger
Am 2014-05-07 18:36, schrieb Jay Aurabind:
Quoted 4 lines
Hello list, Time for a noob discussion,
When I was building the kernel, I found a warning from
drivers/mfd/abx500-core.c, that the Frame size is larger than 1024 bytes. Apparently the stack frame size can be changed from the config, but my question is, whether 1024 bytes low ? I am on anx86_64(core i3).
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
abx500-core.chad an object of struct device being allocated on stack. So dynamically allocating it makes the warning go away. Are there any implications on using dynamic allocation on this particular code? I mean, could there be some reason why the original developer went with static option ?kmalloc may sleep and is therefore sometimes not recommended to be used.
Sure, but this code doesnt appear to be holding any locks. So it should be okay to call kmalloc here I guess.
You can use kmalloc even if you are holding a lock. Care must be taken to pass GFP_ATOMIC flag to the kmalloc, so that it does not sleep.
Quoted 42 lines
Quoted 33 lines
Quoted 33 lines
--- drivers/mfd/abx500-core.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)drivers/mfd/abx500-core.c +5
-3@@ -154,15 +154,17 @@ EXPORT_SYMBOL(abx500_startup_irq_enabled);void abx500_dump_all_banks (void ) {struct abx500_ops *ops; -struct device dummy_child = {NULL }; +struct device *dummy_child;struct abx500_device_entry *dev_entry; + dummy_child =kzalloc (sizeof (struct device ),GFP_KERNEL ); +list_for_each_entry (dev_entry, &abx500_list, list) { - dummy_child.parent = dev_entry->dev; + dummy_child->parent = dev_entry->dev; ops = &dev_entry->ops;if ((ops !=NULL ) && (ops->dump_all_banks !=NULL )) - ops->dump_all_banks (&dummy_child); + ops->dump_all_banks (dummy_child); } }EXPORT_SYMBOL (abx500_dump_all_banks);Quoted 5 lines
Quoted 5 lines
_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies_______________________________________________
Kernelnewbies mailing list
Kernelnewbies at kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Signature · 3 lines
Paul Davies C, CSE B.Tech. student, Govt. Engineering College, Thrissur.