yamt commented on pull request #4977: URL: https://github.com/apache/incubator-nuttx/pull/4977#issuecomment-1025254774
> > > > > > i'm not sure if this is a good idea because > > > > > > > > > > > > * this breaks existing configurations > > > > > > > > > > > > > > > Could you explain more? > > > > > > > > > > > > if you have a config with stack sizes adjusted carefully, it won't work as expected anymore. > > > > > > > > > > > > * it makes stack-related api very confusing (eg. pthread_attr_setstack and pthread_attr_setstacksize) > > > > > > > > > > > > > > > SIM_STACKSIZE_ADJUSTMENT is used only for sim, the stack on sim already has huge difference to other arch. This patch is try to fix the program hard code the stack size in code or config with a fixed default value. > > > > > > > > > > > > the difference from other arch is not a problem as it's arch-dependent in the first place. > > > > my concern is inconsistency within sim. with this change, an app using pthread_attr_setstack and an app using pthread_attr_setstacksize need to have different ideas of stack size. > > > > > > > > > If the user tune the stack size by pthread_attr_setstacksize for real device, it normally stop work on sim, so do you prefer the user define the different size between sim and other arch. But, is it good to let user take care about the stack size on sim? > > > > > > yes. it's something only the app can deal with some #ifdef. > > Why do you want to see a general application add ifdef CONFIG_ARCH_SIM? i don't. but sometimes it's necessary. > As I said before, sim is just for developing, nobody want to fight the stack size issue on the simulator. We have 50+ developers use simulator daily who doesn't have the deep knowledge how sim work internally. The top 1 issue(complaint) is that the application run correctly on the real device, but crash on sim. After investigating by NuttX expert, 90+% is just because they copy the stack size config to sim. interesting. thank you for the info. what's your suggestion to them when their app was using pthread_attr_setstack-like api? > > > > > > > * stacksize is inherently arch-dependent > > > > > > > > > > > > > > > DEFAULT_TASK_STACKSIZE could cover the most arch difference. > > > > > > > > > > > > the stack usage is actually different. it's more natural to use different values to reflect the reality than trying to maintain the illusion of "one value fit all". > > > > > > > > > I just want to illusion of the sim(that's why we name it CONFIG_SIM_STACKSIZE_ADJUSTMENT) not other real arch. sim is just a test platform for functional development, nobody expect to tune the stack size on it, so it's boring to enforce the user to define the different stack size for sim either by defconfig or source code. > > > We have found many people just turn on some application under apps/, and crash immediately on sim just because that application tune the stack size to a particular value. > > > > > > how those applications tune the stack size? if they hardcode values for a specific arch, it's their problem, not sim. > > If you don't like this patch, please revert DEFAULT_STACK_SIZE too. From my review, DEFAULT_STACK_SIZE provided by you also try to fix the same stack size. And let's modify all the hardcode stack size in application or add CONFIG_xxx_STACK_SIZE=65536 in all defconfig under boards/sim. replied in the other pr. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
