| Message ID | 20221001002638.2881842-1-dlatypov@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | kunit: more assertion reworking | expand |
On Sat, Oct 1, 2022 at 2:26 AM Daniel Latypov <dlatypov@google.com> wrote: > > Note: this does change the function signature of > kunit_do_failed_assertion, so we'd need to update the rust wrapper in > https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/kunit.rs, > but hopefully it's just a simple change, e.g. maybe just like: Yeah, should be simple. Thanks for pointing it out! The series looks like a great cleanup on top of the stack reduction. Cheers, Miguel
On Sat, Oct 1, 2022 at 3:15 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Sat, Oct 1, 2022 at 2:26 AM Daniel Latypov <dlatypov@google.com> wrote: > > > > Note: this does change the function signature of > > kunit_do_failed_assertion, so we'd need to update the rust wrapper in > > https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/kunit.rs, > > but hopefully it's just a simple change, e.g. maybe just like: > > Yeah, should be simple. Thanks for pointing it out! > > The series looks like a great cleanup on top of the stack reduction. Thanks for taking a look at the rest of the series as well. While I have you here, any thoughts on how to coordinate the change? I made the breaking change patch#1 so it should be easier to pull out. One option I was thinking was: * wait till this lands in Shuah's tree * I create a Github PR that contains patch#1 + a patch for kunit.rs I was not clear on how the RfL Github pulls in upstream changes or how often. But my assumption is patch#1 would fall away naturally if rebasing onto 6.1 (and maybe we can squash the kunit.rs change). Thanks, Daniel
On Sat, Oct 1, 2022 at 8:00 PM Daniel Latypov <dlatypov@google.com> wrote: > > While I have you here, any thoughts on how to coordinate the change? My bad, I forgot to reply to this, sorry. I noticed it again when merging 6.1-rc1 into our branch. Normally I fix the issues when I merge back from Linus. Since we intend to keep the CI green on every PR, I added the fix for this in the merge itself. In any case, to clarify, the `rust` branch in GitHub is just where we placed a lot of things that we wanted to eventually submit (and some that should not, e.g. the GitHub CI files). We will be minimizing the differences w.r.t. upstream in that branch by preparing proper patches from scratch and submitting them through `rust-next` and other trees, and eventually remove it (or reusing the name for the fixes branch since that is the name other trees use, but we will see). Cheers, Miguel
On Tue, Oct 18, 2022 at 4:20 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Sat, Oct 1, 2022 at 8:00 PM Daniel Latypov <dlatypov@google.com> wrote: > > > > While I have you here, any thoughts on how to coordinate the change? > > My bad, I forgot to reply to this, sorry. I noticed it again when > merging 6.1-rc1 into our branch. No worries. You've had a very busy couple of weeks, I imagine. > > Normally I fix the issues when I merge back from Linus. Since we > intend to keep the CI green on every PR, I added the fix for this in > the merge itself. Thanks! Daniel
On Wed, Oct 19, 2022 at 1:26 AM Daniel Latypov <dlatypov@google.com> wrote: > > No worries. > You've had a very busy couple of weeks, I imagine. Just a bit :) Nevertheless, it was my intention to reply :( I have linked this thread from the PR noting that you warned me about the future conflict [1], thanks again! [1] https://github.com/Rust-for-Linux/linux/pull/915#issuecomment-1283138279 Cheers, Miguel
RFC: https://lore.kernel.org/linux-kselftest/20220525154442.1438081-1-dlatypov@google.com/ Changes since then: tweak commit messages, reformatting to make checkpatch.pl happy. Nothing substantial. Why send this out again now: the initial Rust patchset no longer contains the Kunit changes, so hopefully both series can go into 6.1 and later we can coordinate the update the kunit.rs wrapper. This is a follow up to these three series: https://lore.kernel.org/all/20220113165931.451305-1-dlatypov@google.com/ https://lore.kernel.org/all/20220118223506.1701553-1-dlatypov@google.com/ https://lore.kernel.org/all/20220125210011.3817742-1-dlatypov@google.com/ The two goals of those series were a) reduce the size of struct kunit_assert and friends. (struct kunit_assert went from 48 => 8 bytes on UML.) b) simplify the internal code, mostly by deleting macros This series goes further a) sizeof(struct kunit_assert) = 0 now b) e.g. we delete another class of macros (KUNIT_INIT_*_ASSERT_STRUCT) Note: this does change the function signature of kunit_do_failed_assertion, so we'd need to update the rust wrapper in https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/kunit.rs, but hopefully it's just a simple change, e.g. maybe just like: @@ -38,9 +38,7 @@ }); static CONDITION: &'static $crate::str::CStr = $crate::c_str!(stringify!($cond)); static ASSERTION: UnaryAssert = UnaryAssert($crate::bindings::kunit_unary_assert { - assert: $crate::bindings::kunit_assert { - format: Some($crate::bindings::kunit_unary_assert_format), - }, + assert: $crate::bindings::kunit_assert {}, condition: CONDITION.as_char_ptr(), expected_true: true, }); @@ -67,6 +65,7 @@ core::ptr::addr_of!(LOCATION.0), $crate::bindings::kunit_assert_type_KUNIT_ASSERTION, core::ptr::addr_of!(ASSERTION.0.assert), + Some($crate::bindings::kunit_unary_assert_format), core::ptr::null(), ); } Daniel Latypov (4): kunit: remove format func from struct kunit_assert, get it to 0 bytes kunit: rename base KUNIT_ASSERTION macro to _KUNIT_FAILED kunit: eliminate KUNIT_INIT_*_ASSERT_STRUCT macros kunit: declare kunit_assert structs as const include/kunit/assert.h | 74 ++---------------------- include/kunit/test.h | 127 +++++++++++++++++++++++------------------ lib/kunit/test.c | 7 ++- 3 files changed, 80 insertions(+), 128 deletions(-) base-commit: 511cce163b75bc3933fa3de769a82bb7e8663f2b