[PATCH] GSOC2012 project atomic support for RTEMS
wei.a.yang at gmail.com
wei.a.yang at gmail.com
Thu Sep 20 14:32:26 UTC 2012
Hi Joel, thank you very much for review my patch. I will correct the patch according to your comments ASAP. And I have already updated the wiki for this project. See: http://wiki.rtems.org/wiki/index.php/Atomic_Operations . If there is any lost I can add it. Sorry for reply this so later.
在 2012-9-19,19:47,Joel Sherrill <joel.sherrill at oarcorp.com> 写道:
> The first thing I noticed was that git complained about white
> space issues. I have attached the output from that.
>
> Here are random comments as I spotted them beyond that.
>
> + I ran all tests on powerpc/psim and the atomic tests passed.
> + Check the copyright year to 2012 everywhere.
> + rtems/score/atomic.h
> - Capitalize the first letter in sentences in comments.
> - }Atomic_Memory_barrier needs a space after }
> - Each Atomic Load, Store, etc method needs its own
> Doxygen comment block. There is a Doxygen command
> to reuse another comment (I think) which might be useful
> here.
> - line up parameter names in prototypes.
> + atomic.inl
> - Copyright and file description go in separate comment blocks
> - Doxygen file header goes first
> - Stay < 80 columns
> - acq_XXX? Can we spell that out completely, please? In RTEMS
> we do not abbreviate.
> - Ditto for "rel"
> - Ditto for "ptr"
> - Ditto on others I might have missed
> - if( should be if (
> - normally ...
> if X
> return Y
> else
> return X
> is written
> if X
> return Y
> return X
> Notice that the else is not needed and we avoid indenting more.
> This will really clean up a method like _Atomic_Fetch_add_int
> - Again no Doxygen comments on methods
> - Single line only -- no need for > 1 blank line
> - line up arguments
> + i386 file
> - spacing, abbreviation
> - watch \ at end of line to make sure they line up. They don't
> + powerpc
> - Same as i386
>
> I didn't have as much time to spend on the test code.
> - no intermediate blank lines in output.
> - copyright year
> - no double "*" lines in comments
> - What's the purpose of TASK_NUMS? Better name?
> - Why is the CONFIGURE_MAXIMUM_TASKS not near the rest of the
> parameters? Comment it in the "odd" place and explain near the
> rest of the parameters where it is. If it is needed to do this by the
> test design (e.g. file reuse, etc). Try to avoid it.
> - ATOMIC_STORE_NO_BARRIER > 80 columns
>
> Please also take the time to update the Wiki page for this as a project.
> It clearly has made a lot of progress but since it only supports 2 target
> architectures, there is work left.
>
> Mostly just clean up. The technical side of this got beaten mercilessly
> during the summer. :)
>
> Thanks.
>
> --joel
> On 09/18/2012 10:50 AM, yangwei weiyang wrote:
>> Hi
>>
>> There are some patches to RTEMS which need review.
>>
>> It is the final version of GSOC2012 project atomic support for RTEMS.
>> This series include six patches:
>>
>> -The first patch includes all the modifies to the related *.am and *.ac files.
>> -The second patch includes the atomic operations API definition for
>> all the architectures.
>> -The third patch includes the implementation of atomic operations
>> for i386 architecture.
>> -The fourth patch includes the implementation of atomic operations
>> for PowerPC architecture.
>> -The fifth patch includes seven test cases of atomic operations for UP system.
>> -The sixth patch includes seven test cases of atomic operations for
>> SMP system.
>>
>> Any suggestion is welcome!
>>
>
>
> --
> Joel Sherrill, Ph.D. Director of Research& Development
> joel.sherrill at OARcorp.com On-Line Applications Research
> Ask me about RTEMS: a free RTOS Huntsville AL 35805
> Support Available (256) 722-9985
>
>
> <white.txt>
More information about the devel
mailing list