Skip to content

Conversation

@wyr-7
Copy link
Contributor

@wyr-7 wyr-7 commented Feb 3, 2026

Add a comprehensive test suite to validate the reader-writer semaphore optimization that reduces unnecessary context switches. The optimization ensures up_wait() is only called when the lock is actually released (writer/reader count reaches 0), avoiding premature wake-ups.

Test Coverage:

  • Core Correctness Tests (6 tests):

    • Multiple readers with no writers (concurrent read access)
    • Multiple writers exclusive access verification
    • Mixed reader-writer access patterns
    • Waiter wake-up correctness
    • Lock holder tracking (recursive write lock support)
    • Context switch reduction verification
  • Performance Tests (3 tests):

    • Recursive write lock performance
    • Converted lock (read->write) performance
    • High contention multi-threaded performance (8 threads)
  • Basic Functionality Tests (1 test):

    • Basic read/write/recursive lock operations

Key APIs Tested:

  • init_rwsem() / destroy_rwsem()
  • down_read() / up_read()
  • down_write() / up_write()
  • Recursive locking behavior
  • Lock holder field tracking

This test suite validates commit Id3b49dda0309b098f04e8ab499c28c94fe1f77ce which optimizes rwsem to reduce context switches in NuttX.

@@ -0,0 +1,320 @@
# RWSem Test Coverage Summary
Copy link
Contributor

@linguini1 linguini1 Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't reviewed your code in detail but this documentation in its entirety is AI-generated slop. It's my opinion that it has no place in this PR or in the NuttX project.

…mization

VELAPLATFO-51602

Add a comprehensive test suite to validate the reader-writer semaphore
optimization that reduces unnecessary context switches. The optimization
ensures up_wait() is only called when the lock is actually released
(writer/reader count reaches 0), avoiding premature wake-ups.

Test Coverage:
- Core Correctness Tests (6 tests):
  * Multiple readers with no writers (concurrent read access)
  * Multiple writers exclusive access verification
  * Mixed reader-writer access patterns
  * Waiter wake-up correctness
  * Lock holder tracking (recursive write lock support)
  * Context switch reduction verification

- Performance Tests (3 tests):
  * Recursive write lock performance
  * Converted lock (read->write) performance
  * High contention multi-threaded performance (8 threads)

- Basic Functionality Tests (1 test):
  * Basic read/write/recursive lock operations

Key APIs Tested:
- init_rwsem() / destroy_rwsem()
- down_read() / up_read()
- down_write() / up_write()
- Recursive locking behavior
- Lock holder field tracking

This test suite validates commit Id3b49dda0309b098f04e8ab499c28c94fe1f77ce
which optimizes rwsem to reduce context switches in NuttX.

Change-Id: Icabbc9a65e879d6689529d0c8e3aecbdbd9bd6be
Signed-off-by: Yongrong Wang <wangyongrong@xiaomi.com>
Copy link
Member

@raiden00pl raiden00pl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI garbage

@wyr-7 wyr-7 closed this Feb 3, 2026
@GUIDINGLI
Copy link
Contributor

@raiden00pl @linguini1

Why this testcase is AI garbage ?
Have you seen the file:
testing/sched/rwsem_test/rwsem_comprehensive_test.c
in this PR ?

My view is that the API being tested is a common, POSIX-standard API. Therefore, it’s perfectly acceptable to use AI-generated tests or to reuse/adapt existing external test cases. There’s no need to reinvent the wheel by writing another brand-new test suite from scratch.

@GUIDINGLI GUIDINGLI reopened this Feb 3, 2026
@raiden00pl
Copy link
Member

raiden00pl commented Feb 3, 2026

@GUIDINGLI Did you even check the content of this commit?

image

Here's another example of AI garbage from Xiaomi: #3381
PRs like this are disrespectful to this community.

If you're going to use AI, at least do it right, and don't produce a wall of worthless text that no one reads.

@GUIDINGLI
Copy link
Contributor

@GUIDINGLI Did you even check the content of this commit?

image Here's another example of AI garbage from Xiaomi: #3381 PRs like this are disrespectful to this community.

If you're going to use AI, at least do it well, and don't produce a wall of worthless text that no one reads.

Yes, I have seen the external file, you can request the author to delete the intermediate, but can NOT say all the files are garbage.
And NOT close the PR.

@GUIDINGLI
Copy link
Contributor

@wyr-7
Please update this PR and remove the *.md

@raiden00pl
Copy link
Member

@GUIDINGLI The PR was closed by the author.

Another question: what license is this AI-generated code? Can you assure us that this is Apache compatible?

@linguini1
Copy link
Contributor

but can NOT say all the files are garbage.

Why not?

And NOT close the PR.

Closed by the author, not us.

@linguini1
Copy link
Contributor

but can NOT say all the files are garbage.

Why not?

And NOT close the PR.

Closed by the author, not us.

If a developer cannot take the time to write documentation for their tests, or even to write the tests, why should we bother reviewing it?

@wyr-7
Copy link
Contributor Author

wyr-7 commented Feb 3, 2026

@GUIDINGLI @raiden00pl @linguini1

@linguini1 wants rwsem test data. I used an ai tool to generate a relatively complete test case for testing. I think there's no problem with this. And @linguini1 wants me to share the test case with him, so I uploaded it here to give him a link. I didn't want to incorporate this pr into nuttx-apps. I think there's nothing wrong with using ai appropriately. What's wrong is that I shouldn't have pushed it here. It should only be pushed to my own homepage,so I closed this PR.

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Feb 3, 2026

but can NOT say all the files are garbage.

Why not?

It's time to learn how AI can help you development before you said the code generated by AI is garbage.

And NOT close the PR.

Closed by the author, not us.

If a developer cannot take the time to write documentation for their tests, or even to write the tests, why should we bother reviewing it?

All my team member require to use AI to assist him writing code/test, analysis bug and write document.

@xiaoxiang781216
Copy link
Contributor

@GUIDINGLI The PR was closed by the author.

Another question: what license is this AI-generated code? Can you assure us that this is Apache compatible?

@raiden00pl You need ask the lawyer, but ALL Xiaomi employer(include my team) do the development with AI everyday. At least, I don't hear any licence issue from Xiaomi legel team.

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Feb 3, 2026

@GUIDINGLI @raiden00pl @linguini1

@linguini1 wants rwsem test data. I used an ai tool to generate a relatively complete test case for testing. I think there's no problem with this. And @linguini1 wants me to share the test case with him, so I uploaded it here to give him a link. I didn't want to incorporate this pr into nuttx-apps. I think there's nothing wrong with using ai appropriately. What's wrong is that I shouldn't have pushed it here. It should only be pushed to my own homepage,so I closed this PR.

@wyr-7 BTW, please refuse the unreasonable requirement from @linguini1 in the future. If you want the new test, please develop it by yourself @linguini1 .

@raiden00pl
Copy link
Member

@wyr-7 If this PR wasn't supposed to be merged, then fine. But it would be worth mentioning that in the content :)

@raiden00pl You need ask the lawyer, but ALL Xiaomi employer(include my team) do the development with AI everyday. At least, I don't hear any licence issue from Xiaomi legel team.

@xiaoxiang781216 Can you ensure that AI-generated code doesn't contain copyleft licenses code like GPL?. It's up to humans to ensure that the code being generated has the correct license. Sorry, but I'm a little skeptical of your legal team statement, especially since the rest of the world hasn't really solved this problem yet, and copyright infringement by AI is a fact.

It's time to learn how AI can help you development before you said the code generated by AI is garbage.

Then you use this AI tools in a wrong way, because AI-generated content like in this PR #3381 is obvious garbage and shouldn't be merged in the first place. At least read if what the AI ​​has produced for you makes sense and add "Be concise" to your prompts

@linguini1
Copy link
Contributor

@linguini1 wants rwsem test data. I used an ai tool to generate a relatively complete test case for testing. I think there's no problem with this. And @linguini1 wants me to share the test case with him, so I uploaded it here to give him a link. I didn't want to incorporate this pr into nuttx-apps. I think there's nothing wrong with using ai appropriately. What's wrong is that I shouldn't have pushed it here. It should only be pushed to my own homepage,so I closed this PR.

You're right, thank you for providing the code you used to test. I did request on another PR (don't have the link off-hand) that you share the tests case you used to verify your change. I appreciate you sharing that information with me, AI-generated or not, because it gives me more information about your testing.

The reason I reviewed this negatively is as you said, it shouldn't be merged into NuttX. I appreciate you wanting to upstream your testing, but there is currently an ongoing discussion about admitting AI code into NuttX. Currently my stance is no. If you have a different opinion, please share it in the discussion on the mailing list so we can make an informed decision when/if we vote on it!

BTW, please refuse the unreasonable requirement from @linguini1 in the future. If you want the new test, please develop it by yourself @linguini1.

Appropriate PR testing information is a requirement for contributions to NuttX, in the contributing guidelines and voted for in the mailing least. I don't know why this is an unreasonable request. I will not be writing test cases for everyone's PRs: if you want it merged, prove that it works.

@xiaoxiang781216
Copy link
Contributor

@wyr-7 If this PR wasn't supposed to be merged, then fine. But it would be worth mentioning that in the content :)

@raiden00pl You need ask the lawyer, but ALL Xiaomi employer(include my team) do the development with AI everyday. At least, I don't hear any licence issue from Xiaomi legel team.

@xiaoxiang781216 Can you ensure that AI-generated code doesn't contain copyleft licenses code like GPL?. It's up to humans to ensure that the code being generated has the correct license. Sorry, but I'm a little skeptical of your legal team statement, especially since the rest of the world hasn't really solved this problem yet, and copyright infringement by AI is a fact.

Let's AI generate the right code isn't a simple task. You need tell him many detail steps and verify the generated code carefully(this proccess need iterate many times before getting the final result), so it's the developer and AI cowork generate the code. Anyway, this is out of topic, let's see what's happen in the next couple years.

@raiden00pl
Copy link
Member

@xiaoxiang781216 I'm not against AI. This is just tool like any other. I've been using AI for years for various tasks and I know it sucks for osdev tasks. Most important, anything AI produces must be checked and verified by a human so we can avoid what I call "AI garbage".
The longer the output from AI, the more difficult it is for a human to verify.

@wyr-7 wyr-7 deleted the rwsem branch February 4, 2026 03:20
@GUIDINGLI
Copy link
Contributor

@xiaoxiang781216 I'm not against AI. This is just tool like any other. I've been using AI for years for various tasks and I know it sucks for osdev tasks. Most important, anything AI produces must be checked and verified by a human so we can avoid what I call "AI garbage". The longer the output from AI, the more difficult it is for a human to verify.

First, Agree with your point:
AI produces must be checked and verified by a human.

Then,
That is right you said these .md are "garbage"
testing/sched/rwsem_test/RWSEM_TEST_COVERAGE.md
testing/sched/rwsem_test/RWSEM_TEST_SUMMARY.md
testing/sched/rwsem_test/RWSEM_TEST_SUMMARY.md

But not all of them.

Let's check the real code to say whether it is "garbage".
testing/sched/rwsem_test/rwsem_comprehensive_test.c
Use posix APIs and can pass on Linux, then this case can check NuttX API and can become the official testcase.
Then why it is "garbage" ?

The cases generated by AI are more rigorous than those written by engineers in some scenarios.

@GUIDINGLI
Copy link
Contributor

@GUIDINGLI The PR was closed by the author.

Another question: what license is this AI-generated code? Can you assure us that this is Apache compatible?

The copyright ownership of AI-generated code lacks a uniform global legal ruling and is a core contentious issue in the global intellectual property field.

I can't assure AI-generated code suit with Apache.
But you can't say it not suit with Apache either.

@GUIDINGLI
Copy link
Contributor

@raiden00pl

The main difference between us is our stance on AI tools.
I firmly believe the industry should be open and inclusive to AI as a new productive force. Despite its flaws like imprecise outputs and potential compliance issues, AI’s value in boosting R&D efficiency and cutting repetitive work costs is undeniable. Rather than writing it off entirely for individual problems, we should set up practical verification and optimization rules, making AI a helpful assistant for engineers and achieving better productivity via human-AI collaboration.

@raiden00pl
Copy link
Member

raiden00pl commented Feb 4, 2026

Let's check the real code to say whether it is "garbage".
testing/sched/rwsem_test/rwsem_comprehensive_test.c
Use posix APIs and can pass on Linux, then this case can check NuttX API and can become the official testcase.
Then why it is "garbage" ?

@GUIDINGLI Generated file is not even close to nuttx standard and should not be merged.
Of course, it can be used to verify functionality; I have nothing against it. It can even be included in a PR as proof of testing. But it's not suitable for an upstream repo in this stage, without fixes that most likely can only human do. "AI garbage" is just another term for "AI slop" https://en.wikipedia.org/wiki/AI_slop It may be more offensive, but I like it.

I can't assure AI-generated code suit with Apache.
But you can't say it not suit with Apache either.

I agree, but we have to be defensive when it comes to licensing. Copyleft licences are "viral" in nature and they can completely ruin your product. Keep in mind that the approach to copyright varies in different parts of the world.
While violating the GPL might not matter much in your country, in Europe it could destroy you and your company.

The main difference between us is our stance on AI tools.
I firmly believe the industry should be open and inclusive to AI as a new productive force. Despite its flaws like imprecise outputs and potential compliance issues, AI’s value in boosting R&D efficiency and cutting repetitive work costs is undeniable. Rather than writing it off entirely for individual problems, we should set up practical verification and optimization rules, making AI a helpful assistant for engineers and achieving better productivity via human-AI collaboration.

As I said earlier, AI is a tool like any other. It can be used in the right way or it can be used in the wrong way. You can boost your productivity, or you can destroy it (and introduce bugs and security holes).

Just remember that by boosting your team's productivity with AI, someone else must later review your changes. If you don't review your AI output, you're shifting that responsibility onto the community, which is not OK. You're boosting your team productivity at the expense of the community.

I certainly won't advocate for a complete AI ban, but some rules are essential. Besides, a complete AI ban is practically impossible, because how can it be verified? However, this isn't nuttx-specific problem, but a global one.
It would be best to adapt AI rules from another large project, but I don't know if anyone has already implemented something like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants