On Sun, Jun 20, 2010 at 22:27, Song, Barry wrote: >From: Mike Frysinger [mailto:[email protected]] >>On Sun, Jun 20, 2010 at 06:12, Song, Barry wrote: >>>From: Mike Frysinger [mailto:[email protected]] >>>>On Fri, Jun 18, 2010 at 04:33, <[email protected]> wrote: >>>>> [#6034], fix argv and envp pointer while using L1 stack >>>>> >>>>> Modified: trunk/fs/binfmt_flat.c (8925 => 8926) >>>>> >>>>> put_user(argc, sp); >>>>> current->mm->arg_start = (unsigned long) p; >>>>> while (argc-->0) { >>>>> +#ifndef CONFIG_APP_STACK_L1 >>>>> put_user((unsigned long) p, argv++); >>>>> +#else >>>>> + put_user((unsigned long) p + l1_ram_offset, argv++); >>>>> +#endif >>>>> ... >>>>> while (envc-->0) { >>>>> - put_user((unsigned long)p, envp); envp++; >>>>> +#ifndef CONFIG_APP_STACK_L1 >>>>> + put_user((unsigned long)p, envp++); >>>>> +#else >>>>> + put_user((unsigned long)p + l1_ram_offset, envp++); >>>>> +#endif >>>> >>>>i dont think we want this behavior. the argv/envp can easily exceed >>>>the tiny 4k scratchpad, not to mention the app itself exceeding it. >>> >>> The origin codes actually have copied argv and env to L1 stack. >>> Following the old logic, adjusting SP+1 is the right way to >>a fast fix. >>> Then you mean we can place argv and env in ram and give a >>legal VMA to >>> cover it? That will make a difference and mean a new small task, >>> not a bug fix. >> >>the changes you made that were needed to make this work are overly >>invasive. if we cant support L1 stack in a simple way, then we need a >>different solution. perhaps the adding of all the #ifdef makes it >>worse than it should be. why are you checking CONFIG_APP_STACK_L1 at >>all ? the core logic already takes care of ignoring the L1 stack flag >>in the FLAT header when this define is enabled which means all those >>other #ifdefs are redundant. >> >>looking again, you're just putting the array of pointers onto the >>stack and not the strings they point to right ? i was thinking you >>were putting the strings themselves on there, but the arrays should be >>OK as those shouldnt take up too much space. > > Why not you debug and check more carefully to find what happen?
because you're already working on it > If the string is not in L1, how could helloworld can be printed in > application by argv[1]. because argv[1] is not the same thing as argv[1][...]. there is no requirement that the strings themselves be placed on the stack, only the pointers to the strings. > I have improved L1 test program to check this. right, that's why i was thinking you were putting the strings on the stack too. now that i look at it again though, i dont think your change there is correct. all you do is check the contents of argv[1]. you dont actually check to see where the pointer is. it should be looking at the address of argv, not the contents. as it stands, this commit needs to be cleaned up wrt the ifdef mess and whitespace reverting -mike _______________________________________________ Linux-kernel-commits mailing list [email protected] https://blackfin.uclinux.org/mailman/listinfo/linux-kernel-commits
