>-----Original Message-----
>From: Mike Frysinger [mailto:[email protected]]
>Sent: Tuesday, June 22, 2010 12:53 AM
>To: Song, Barry
>Cc: [email protected];
>[email protected]
>Subject: Re: [Linux-kernel-commits] [8926]
>trunk/fs/binfmt_flat.c: [!no_src_qa!]
>
>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.
The fix was following the origin procedure. I have given a definite reply at
the first time that the current pointer and content are both in L1.
If you want to place contents to RAM, there will be a new little task.
>
>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
