Re: large frame size warning when compiling

2014-05-10 Thread Jay Aurabind


On Friday 09 May 2014 09:00 PM, valdis.kletni...@vt.edu wrote:
 On Fri, 09 May 2014 17:39:21 +0530, Jay Aurabind said:
 
 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?
 

Point taken. Thank you Valdis, Martin, Paul and Frank for sharing your
thoughts.

Cheers,
Jay



signature.asc
Description: OpenPGP digital signature
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


Re: large frame size warning when compiling

2014-05-09 Thread Jay Aurabind
On Thu, May 08, 2014 at 11:46:43AM -0400, valdis.kletni...@vt.edu wrote:
 On Thu, 08 May 2014 09:24:38 +0530, Jay Aurabind said:
   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. :)
 
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 ?



___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


Re: large frame size warning when compiling

2014-05-09 Thread Jay Aurabind
On Thu, May 08, 2014 at 02:38:24PM +0200, Martin Kepplinger wrote:
 Am 2014-05-07 18:36, schrieb Jay Aurabind:
  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 ?
 
 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.
  
  
  ---
   drivers/mfd/abx500-core.c | 8 +---
   1 file changed, 5 insertions(+), 3 deletions(-)
  
  diff --git a/drivers/mfd/abx500-core.c b/drivers/mfd/abx500-core.c
  index f3a15aa..709a84f 100644
  --- a/drivers/mfd/abx500-core.c
  +++ b/drivers/mfd/abx500-core.c
  @@ -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@kernelnewbies.org
  http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
  
 

___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


Re: large frame size warning when compiling

2014-05-09 Thread Paul Davies C
On Friday 09 May 2014 05:43 PM, Jay Aurabind wrote:
 On Thu, May 08, 2014 at 02:38:24PM +0200, Martin Kepplinger wrote:
 Am 2014-05-07 18:36, schrieb Jay Aurabind:
 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 ?
 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.

 ---
   drivers/mfd/abx500-core.c | 8 +---
   1 file changed, 5 insertions(+), 3 deletions(-)

 diff --git a/drivers/mfd/abx500-core.c b/drivers/mfd/abx500-core.c
 index f3a15aa..709a84f 100644
 --- a/drivers/mfd/abx500-core.c
 +++ b/drivers/mfd/abx500-core.c
 @@ -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@kernelnewbies.org
 http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

 ___
 Kernelnewbies mailing list
 Kernelnewbies@kernelnewbies.org
 http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


-- 
Paul Davies C,
CSE B.Tech. student,
Govt. Engineering College, Thrissur.


___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


Re: large frame size warning when compiling

2014-05-09 Thread Valdis . Kletnieks
On Fri, 09 May 2014 17:39:21 +0530, Jay Aurabind said:

 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?


pgpO_I31iCUCc.pgp
Description: PGP signature
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


Re: large frame size warning when compiling

2014-05-09 Thread Frank Ch. Eigler
valdis.kletni...@vt.edu writes:

 [...]
 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

 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

___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


Re: large frame size warning when compiling

2014-05-08 Thread Martin Kepplinger
Am 2014-05-07 18:36, schrieb Jay Aurabind:
 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 ?

kmalloc may sleep and is therefore sometimes not recommended to be used.

 
 
 ---
  drivers/mfd/abx500-core.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/mfd/abx500-core.c b/drivers/mfd/abx500-core.c
 index f3a15aa..709a84f 100644
 --- a/drivers/mfd/abx500-core.c
 +++ b/drivers/mfd/abx500-core.c
 @@ -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@kernelnewbies.org
 http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
 


___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


Re: large frame size warning when compiling

2014-05-08 Thread Valdis . Kletnieks
On Thu, 08 May 2014 09:24:38 +0530, Jay Aurabind said:
  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. :)



pgpQEzLwLXPCb.pgp
Description: PGP signature
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


Re: large frame size warning when compiling

2014-05-07 Thread Jay Aurabind

 
 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.
 
 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?
 
 He probably didn't realize or didn't know better.
 
 Having said that:
 
 +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! :)
 
  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);
  }
 
   kfree(dummy_child); /* should go here... */
  }
  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
 
 
Waiting to hear from the experts!
 
 ___
 Kernelnewbies mailing list
 Kernelnewbies@kernelnewbies.org
 http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
 



signature.asc
Description: OpenPGP digital signature
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies