Align WorkQueueBuilder with new workqueue work timeout feature

[bjarki-andreasen]

The feature comes with a new parameter to be initialized using the WorkQueueBuilder.

[d3zd3z]

Thank you for your contribution. I am actually in the midst of a bit of an overhaul of how the workqueue code works, so we'll have to rework this a bit at that point.

Probably my main concern is that with the current code, the work will actually get leaked if it is aborted due to a timeout. I'll try to think if there is a way to deal with this.

One thing I have planned to add is to allow my new struct Work to be passed as &'static Work, which would make the lifetime issues less of a concern.

[d3zd3z]

I think the only practical way to get this into both Zephyr and Rust is going to be to add the support based on some conditional (CONFIG_HAVE_...) so that the rust code will compile either way. It is too constraining to be practical to try and push these together lock step.

[bjarki-andreasen]

I think the only practical way to get this into both Zephyr and Rust is going to be to add the support based on some conditional (CONFIG_HAVE_...) so that the rust code will compile either way. It is too constraining to be practical to try and push these together lock step.

Modules in zephyr constantly need to be updated and synced, this is not an issue :) The process is this:

1. create PR which changes zephyr module (Rust)
2. create PR for zephyr which does two things:
   • update zephyr manifest to point to Rust PR
   • adjust zephyr to match changes in zephyr module (Rust)
3. Wait for green CI in zephyr, which means zephyr and the changes to the module work together.
4. Merge PR for zephyr module (Rust)
5. update zephyr PR to point to the new Rust sha
6. Wait for green CI in zephyr
7. Merge zephyr PR

The problem here is that the module itself is running its own CI, against an incorrect version of zephyr, IE, not the one being created in step 2.

The CI in this PR here should be ignored, as it is not tracking zephyr.

[bjarki-andreasen]

@d3zd3z Please revisit this, its been a month now...

[nashif]

The problem here is that the module itself is running its own CI, against an incorrect version of zephyr, IE, not the one being created in step 2.

this is a problem we also have with SOF for example where the dependency is circular with CI running in multiple places, if you merge in rust, rust CI will fail because the change is still not merged in zephyr, maybe you end up merging something in rust before getting the reviews/approvals on the zephyr side, i.e. some changes might be requested or someone might block the PR for a long period of time, so, that is probably what @d3zd3z means with lockstep.
@bjarki-andreasen I think you should get your PR reviewed and "almost" ready to merge, then we can do all of this in "lockstep". We need a better way to deal with this or we should prevent something like that to happen in the first place :)

[bjarki-andreasen]

The problem here is that the module itself is running its own CI, against an incorrect version of zephyr, IE, not the one being created in step 2.

this is a problem we also have with SOF for example where the dependency is circular with CI running in multiple places, if you merge in rust, rust CI will fail because the change is still not merged in zephyr, maybe you end up merging something in rust before getting the reviews/approvals on the zephyr side, i.e. some changes might be requested or someone might block the PR for a long period of time, so, that is probably what @d3zd3z means with lockstep. @bjarki-andreasen I think you should get your PR reviewed and "almost" ready to merge, then we can do all of this in "lockstep". We need a better way to deal with this or we should prevent something like that to happen in the first place :)

The PR was approved and ready to be merged a month ago, It was just blocked by the manifest DNM...

[d3zd3z]

@d3zd3z Please revisit this, its been a month now...

I'm just not sure what I'm supposed to do here. This doesn't compile without the upstream change, and the upstream change without this will break the rust CI build.

It does seem easier to make the upstream depend on something like a CONFIG_HAVE_... define, so this can then be conditionalized, and we don't have a lock-step dependency between them.

[bjarki-andreasen]

@d3zd3z Please revisit this, its been a month now...

I'm just not sure what I'm supposed to do here. This doesn't compile without the upstream change, and the upstream change without this will break the rust CI build.

It does seem easier to make the upstream depend on something like a CONFIG_HAVE_... define, so this can then be conditionalized, and we don't have a lock-step dependency between them.

You are supposed to merge the PR, this module is not standalone, its an optional module of zephyr. Once you merge this PR, nothing in zephyr breaks, since the optional module points to a sha before this PR is merged. Then, the zephyr PR is updated to point to the new sha here, and both changes to this submodule and the zephyr API changes go in together.

The only thing that matters is that zephyr builds with the version of this submodule that the zephyr manifest points to, CI in this module is not relevant.