Message ID | 20221205155845.233018-1-maz@kernel.org |
---|---|
State | New |
Headers | show |
Series | [GIT,PULL] KVM/arm64 updates for 6.2 | expand |
On Tue, Dec 06, 2022, Paolo Bonzini wrote: > On 12/5/22 16:58, Marc Zyngier wrote: > > - There is a lot of selftest conflicts with your own branch, see: > > > > https://lore.kernel.org/r/20221201112432.4cb9ae42@canb.auug.org.au > > https://lore.kernel.org/r/20221201113626.438f13c5@canb.auug.org.au > > https://lore.kernel.org/r/20221201115741.7de32422@canb.auug.org.au > > https://lore.kernel.org/r/20221201120939.3c19f004@canb.auug.org.au > > https://lore.kernel.org/r/20221201131623.18ebc8d8@canb.auug.org.au > > > > for a rather exhaustive collection. > > Yeah, I saw them in Stephen's messages but missed your reply. > > In retrospect, at least Gavin's series for memslot_perf_test should have > been applied by both of us with a topic branch, but there's so many > conflicts all over the place that it's hard to single out one series. > It just happens. Alternatively, we could have a dedicated selftests/kvm tree (or branch)? I almost suggested doing that on multiple occasions this cycle, but ultimately decided not to because it would effectively mean splitting series that touch KVM and selftests into different trees, which would create a different kind of dependency hell. Or maybe a hybrid approach where series that only (or mostly?) touch selftests go into a dedicated tree? I get the feeling that I'm overthinking things though, this level of activity and conflicts should be relatively rare.
On Tue, Dec 06, 2022 at 06:10:32PM +0000, Sean Christopherson wrote: > Alternatively, we could have a dedicated selftests/kvm tree (or branch)? > I almost suggested doing that on multiple occasions this cycle, but ultimately > decided not to because it would effectively mean splitting series that touch KVM > and selftests into different trees, which would create a different kind of > dependency hell. Or maybe a hybrid approach where series that only (or mostly?) > touch selftests go into a dedicated tree? Some other subsystems do have a separate branch for kselftests. One fairly common occurrence is that the selftests branch ends up failing to build independently because someone adds new ABI together with a selftest but the patches adding the ABI don't end up on the same branch as the tests which try to use them. That is of course resolvable but it's a common friction point.
On Tue, 06 Dec 2022 17:41:21 +0000, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 12/5/22 16:58, Marc Zyngier wrote: > > - There is a lot of selftest conflicts with your own branch, see: > > > > https://lore.kernel.org/r/20221201112432.4cb9ae42@canb.auug.org.au > > https://lore.kernel.org/r/20221201113626.438f13c5@canb.auug.org.au > > https://lore.kernel.org/r/20221201115741.7de32422@canb.auug.org.au > > https://lore.kernel.org/r/20221201120939.3c19f004@canb.auug.org.au > > https://lore.kernel.org/r/20221201131623.18ebc8d8@canb.auug.org.au > > > > for a rather exhaustive collection. > > Yeah, I saw them in Stephen's messages but missed your reply. > > In retrospect, at least Gavin's series for memslot_perf_test should have > been applied by both of us with a topic branch, but there's so many > conflicts all over the place that it's hard to single out one series. > It just happens. I generally queue things on topic branches for my own sanity, happy to make them available in the future. > > The only conflict in non-x86 code is the following one, please check > if I got it right. > > diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c b/tools/testing/selftests/kvm/aarch64/page_fault_test.c > index 05bb6a6369c2..0cda70bef5d5 100644 > --- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c > +++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c > @@ -609,6 +609,8 @@ static void setup_memslots(struct kvm_vm *vm, struct test_params *p) > data_size / guest_page_size, > p->test_desc->data_memslot_flags); > vm->memslots[MEM_REGION_TEST_DATA] = TEST_DATA_MEMSLOT; > + > + ucall_init(vm, data_gpa + data_size); > } > static void setup_default_handlers(struct test_desc *test) > @@ -704,8 +706,6 @@ static void run_test(enum vm_guest_mode mode, void *arg) > setup_gva_maps(vm); > - ucall_init(vm, NULL); > - > reset_event_counts(); > /* > > > Special care is needed here because the test uses ____vm_create(). > > I haven't pushed to kvm/next yet to give you time to check, so the > merge is currently in kvm/queue only. There has been a couple of -next failures reported by broonie: https://lore.kernel.org/r/20221206175916.250104-1-broonie@kernel.org https://lore.kernel.org/r/20221206181506.252537-1-broonie@kernel.org which I think you've received as well. The second patch is definitely needed, but you've already solved the first one. At least things do build. > > > - For the 6.3 cycle, we are going to experiment with Oliver taking > > care of most of the patch herding. I'm sure he'll do a great job, > > but if there is the odd mistake, please cut him some slack and blame > > me instead. > > Absolutely - you both have all the slack you need, synchronization > is harder than it seems. Thanks, M.
On 12/6/22 19:20, Mark Brown wrote: >> I almost suggested doing that on multiple occasions this cycle, but ultimately >> decided not to because it would effectively mean splitting series that touch KVM >> and selftests into different trees, which would create a different kind of >> dependency hell. Or maybe a hybrid approach where series that only (or mostly?) >> touch selftests go into a dedicated tree? > > Some other subsystems do have a separate branch for kselftests. One > fairly common occurrence is that the selftests branch ends up failing to > build independently because someone adds new ABI together with a > selftest but the patches adding the ABI don't end up on the same branch > as the tests which try to use them. That is of course resolvable but > it's a common friction point. Yeah, the right solution is simply to merge selftests changes separately from the rest and use topic branches. We will have more friction of this kind if we succeed in making more KVM code multi-architecture, so let's just treat selftests as the more innocuous drill... Paolo
On Tue, 06 Dec 2022 21:43:43 +0000, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 12/6/22 19:20, Mark Brown wrote: > >> I almost suggested doing that on multiple occasions this cycle, but ultimately > >> decided not to because it would effectively mean splitting series that touch KVM > >> and selftests into different trees, which would create a different kind of > >> dependency hell. Or maybe a hybrid approach where series that only (or mostly?) > >> touch selftests go into a dedicated tree? > > > > Some other subsystems do have a separate branch for kselftests. One > > fairly common occurrence is that the selftests branch ends up failing to > > build independently because someone adds new ABI together with a > > selftest but the patches adding the ABI don't end up on the same branch > > as the tests which try to use them. That is of course resolvable but > > it's a common friction point. > > Yeah, the right solution is simply to merge selftests changes > separately from the rest and use topic branches. Don't know if this is what you have in mind, but I think that we should use topic branches for *everything*. The only things for which I don't use a separate branch are the odd drive-by patches, of the spelling fix persuasion. That's what we do for arm64 and the IRQ subsystem. It is a bit more involved at queuing time, but makes dropping series from -next extremely easy, without affecting the history. And crucially, it gives everyone a hint to base their stuff on a stable commit, not a random "tip of kvm/queue as of three days ago". M.
On Tue, Dec 06, 2022 at 06:41:21PM +0100, Paolo Bonzini wrote: > On 12/5/22 16:58, Marc Zyngier wrote: > > - There is a lot of selftest conflicts with your own branch, see: > > > > https://lore.kernel.org/r/20221201112432.4cb9ae42@canb.auug.org.au > > https://lore.kernel.org/r/20221201113626.438f13c5@canb.auug.org.au > > https://lore.kernel.org/r/20221201115741.7de32422@canb.auug.org.au > > https://lore.kernel.org/r/20221201120939.3c19f004@canb.auug.org.au > > https://lore.kernel.org/r/20221201131623.18ebc8d8@canb.auug.org.au > > > > for a rather exhaustive collection. > > Yeah, I saw them in Stephen's messages but missed your reply. > > In retrospect, at least Gavin's series for memslot_perf_test should have > been applied by both of us with a topic branch, but there's so many > conflicts all over the place that it's hard to single out one series. > It just happens. > > The only conflict in non-x86 code is the following one, please check > if I got it right. > > diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c b/tools/testing/selftests/kvm/aarch64/page_fault_test.c > index 05bb6a6369c2..0cda70bef5d5 100644 > --- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c > +++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c > @@ -609,6 +609,8 @@ static void setup_memslots(struct kvm_vm *vm, struct test_params *p) > data_size / guest_page_size, > p->test_desc->data_memslot_flags); > vm->memslots[MEM_REGION_TEST_DATA] = TEST_DATA_MEMSLOT; > + > + ucall_init(vm, data_gpa + data_size); > } > static void setup_default_handlers(struct test_desc *test) > @@ -704,8 +706,6 @@ static void run_test(enum vm_guest_mode mode, void *arg) > setup_gva_maps(vm); > - ucall_init(vm, NULL); > - > reset_event_counts(); > /* > > > Special care is needed here because the test uses ____vm_create(). > > I haven't pushed to kvm/next yet to give you time to check, so the > merge is currently in kvm/queue only. Have a look at this series, which gets things building and actually passing again: https://lore.kernel.org/kvm/20221207214809.489070-1-oliver.upton@linux.dev/ > > - For the 6.3 cycle, we are going to experiment with Oliver taking > > care of most of the patch herding. I'm sure he'll do a great job, > > but if there is the odd mistake, please cut him some slack and blame > > me instead. > > Absolutely - you both have all the slack you need, synchronization > is harder than it seems. Appreciated! -- Thanks, Oliver
On 12/7/22 08:49, Marc Zyngier wrote: > On Tue, 06 Dec 2022 21:43:43 +0000, > Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> On 12/6/22 19:20, Mark Brown wrote: >>>> I almost suggested doing that on multiple occasions this cycle, but ultimately >>>> decided not to because it would effectively mean splitting series that touch KVM >>>> and selftests into different trees, which would create a different kind of >>>> dependency hell. Or maybe a hybrid approach where series that only (or mostly?) >>>> touch selftests go into a dedicated tree? >>> >>> Some other subsystems do have a separate branch for kselftests. One >>> fairly common occurrence is that the selftests branch ends up failing to >>> build independently because someone adds new ABI together with a >>> selftest but the patches adding the ABI don't end up on the same branch >>> as the tests which try to use them. That is of course resolvable but >>> it's a common friction point. >> >> Yeah, the right solution is simply to merge selftests changes >> separately from the rest and use topic branches. > > Don't know if this is what you have in mind, but I think that we > should use topic branches for *everything*. The only things for which > I don't use a separate branch are the odd drive-by patches, of the > spelling fix persuasion. Yeah, I just wish we had better tools to manage them... Paolo > That's what we do for arm64 and the IRQ subsystem. It is a bit more > involved at queuing time, but makes dropping series from -next > extremely easy, without affecting the history. And crucially, it gives > everyone a hint to base their stuff on a stable commit, not a random > "tip of kvm/queue as of three days ago". > > M. >
On 12/7/22 22:51, Oliver Upton wrote: >> >> I haven't pushed to kvm/next yet to give you time to check, so the >> merge is currently in kvm/queue only. > Have a look at this series, which gets things building and actually > passing again: > > https://lore.kernel.org/kvm/20221207214809.489070-1-oliver.upton@linux.dev/ Thank you! Squashed 1-2 and applied 3-4. Paolo
Hey Paolo, On Fri, Dec 09, 2022 at 05:56:50PM +0100, Paolo Bonzini wrote: > On 12/7/22 22:51, Oliver Upton wrote: > > > > > > I haven't pushed to kvm/next yet to give you time to check, so the > > > merge is currently in kvm/queue only. > > Have a look at this series, which gets things building and actually > > passing again: > > > > https://lore.kernel.org/kvm/20221207214809.489070-1-oliver.upton@linux.dev/ > > Thank you! Squashed 1-2 and applied 3-4. I actually wound up sending a v2 for this :) 1-2 are the same, but I addressed some of Sean's feedback and also pulled in more fixes. In addition to the problems with page_fault_test, other KVM selftests were magically failing on arm64 because identity mapping ucall MMIO punched a hole straight through the program image. Mind dumping what I had in v1 and applying this instead? https://lore.kernel.org/kvm/20221209015307.1781352-1-oliver.upton@linux.dev/ Thanks! -- Best, Oliver
On Fri, Dec 9, 2022 at 6:05 PM Oliver Upton <oliver.upton@linux.dev> wrote: > Mind dumping what I had in v1 and applying this instead? > > https://lore.kernel.org/kvm/20221209015307.1781352-1-oliver.upton@linux.dev/ Ouch, five minutes too late... I can take care of the difference but it'll have to wait for Monday. Paolo