You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/05/16 21:02:55 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request, #6283: pthread/spinlock: Call up_testsest directly in the flat build
xiaoxiang781216 opened a new pull request, #6283:
URL: https://github.com/apache/incubator-nuttx/pull/6283
## Summary
- libc: Move the common implementation of up_testset to libc/machine
## Impact
spinlock
## Testing
Pass CI
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei merged pull request #6283: pthread/spinlock: Call up_testsest directly in the flat build
Posted by GitBox <gi...@apache.org>.
gustavonihei merged PR #6283:
URL: https://github.com/apache/incubator-nuttx/pull/6283
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #6283: pthread/spinlock: Call up_testsest directly in the flat build
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on PR #6283:
URL: https://github.com/apache/incubator-nuttx/pull/6283#issuecomment-1128624946
@xiaoxiang781216 a fix for the macOS failure on ESP32 configs is here: #6281 however I see other ARM related changes, seem to be build related.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6283: pthread/spinlock: Call up_testsest directly in the flat build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6283:
URL: https://github.com/apache/incubator-nuttx/pull/6283#issuecomment-1131929168
Ok, I drop up_testset from PR, let's create a new PR to remove up_testset and call atomic api instead.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6283: pthread/spinlock: Call up_testsest directly in the flat build
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6283:
URL: https://github.com/apache/incubator-nuttx/pull/6283#discussion_r876908191
##########
include/nuttx/spinlock.h:
##########
@@ -107,28 +107,7 @@ typedef struct
*
****************************************************************************/
-#if defined(CONFIG_ARCH_HAVE_TESTSET)
spinlock_t up_testset(volatile FAR spinlock_t *lock);
-#elif !defined(CONFIG_SMP)
Review Comment:
You’re right, thanks for the clarification @Ouss4!
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6283: pthread/spinlock: Call up_testsest directly in the flat build
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6283:
URL: https://github.com/apache/incubator-nuttx/pull/6283#discussion_r876901384
##########
include/nuttx/spinlock.h:
##########
@@ -107,28 +107,7 @@ typedef struct
*
****************************************************************************/
-#if defined(CONFIG_ARCH_HAVE_TESTSET)
spinlock_t up_testset(volatile FAR spinlock_t *lock);
-#elif !defined(CONFIG_SMP)
Review Comment:
There is a change in the logic here, now this function will be built for SMP targets. Is this really intended?
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a diff in pull request #6283: pthread/spinlock: Call up_testsest directly in the flat build
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on code in PR #6283:
URL: https://github.com/apache/incubator-nuttx/pull/6283#discussion_r876905523
##########
include/nuttx/spinlock.h:
##########
@@ -107,28 +107,7 @@ typedef struct
*
****************************************************************************/
-#if defined(CONFIG_ARCH_HAVE_TESTSET)
spinlock_t up_testset(volatile FAR spinlock_t *lock);
-#elif !defined(CONFIG_SMP)
Review Comment:
I don't think so. The condition is `#if defined(CONFIG_SPINLOCK) && !defined(CONFIG_ARCH_HAVE_TESTSET)` and SMP depends on `CONFIG_ARCH_HAVE_TESTSET`.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on pull request #6283: pthread/spinlock: Call up_testsest directly in the flat build
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on PR #6283:
URL: https://github.com/apache/incubator-nuttx/pull/6283#issuecomment-1131857075
> > Change looks good. I'm just wondering if `arch_atomic.c` is the best place for the `up_testset` function.
> > cc. @gustavonihei
>
> up_testset is one of atomic function.
The part that makes this new location a bit weird is that `arch_atomic.c` is under `libc`, and `up_testset` is specific for spinlocks.
> Actually, it isn't good to introduce up_testset since atomic is part of C/C++ standard. It's better to use the standard atomic api, > and implement them on the old toolchain by hand.
So another option would be to leave `up_testset` in its place and change it to use the C11 function. And then on `arch_atomic.c` we could provide the implementation for the GCC/Clang builtins for those archs that don't support.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6283: pthread/spinlock: Call up_testsest directly in the flat build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6283:
URL: https://github.com/apache/incubator-nuttx/pull/6283#issuecomment-1131842018
> Change looks good. I'm just wondering if `arch_atomic.c` is the best place for the `up_testset` function.
>
> cc. @gustavonihei
up_testset is one of atomic function. Actually, it isn't good to introduce up_testset since atomic is part of C/C++ standard. It's better to use the standard atomic api, and implement them on the old toolchain by hand.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #6283: pthread/spinlock: Call up_testsest directly in the flat build
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on PR #6283:
URL: https://github.com/apache/incubator-nuttx/pull/6283#issuecomment-1132218922
> Here is #6283
Recursion. :)
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6283: pthread/spinlock: Call up_testsest directly in the flat build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6283:
URL: https://github.com/apache/incubator-nuttx/pull/6283#issuecomment-1129137733
> @xiaoxiang781216 a fix for the macOS failure on ESP32 configs is here: #6281 however I see other ARM related changes, seem to be build related.
Done.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6283: pthread/spinlock: Call up_testsest directly in the flat build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6283:
URL: https://github.com/apache/incubator-nuttx/pull/6283#issuecomment-1131930868
Here is https://github.com/apache/incubator-nuttx/pull/6283
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #6283: pthread/spinlock: Call up_testsest directly in the flat build
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6283:
URL: https://github.com/apache/incubator-nuttx/pull/6283#issuecomment-1132421301
should be https://github.com/apache/incubator-nuttx/issues/6301 :)
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org