On Fri, 21 Mar 2025, Frederic Weisbecker wrote: > > > diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S > > > index a572ce36a24f..4e012421d00f 100644 > > > --- a/arch/mips/kernel/genex.S > > > +++ b/arch/mips/kernel/genex.S > > > @@ -104,27 +104,30 @@ handle_vcei: > > > > > > __FINIT > > > > > > - .align 5 /* 32 byte rollback region */ > > > + .align 5 > > > LEAF(__r4k_wait) > > > .set push > > > .set noreorder > > > - /* start of rollback region */ > > > - LONG_L t0, TI_FLAGS($28) > > > - nop > > > - andi t0, _TIF_NEED_RESCHED > > > - bnez t0, 1f > > > - nop > > > - nop > > > - nop > > > -#ifdef CONFIG_CPU_MICROMIPS > > > - nop > > > - nop > > > - nop > > > - nop > > > -#endif > > > + /* Start of idle interrupt region. */ > > > + MFC0 t0, CP0_STATUS > > > + /* Enable interrupt. */ > > > + ori t0, 0x1f > > > > This instruction sequence still suffers from the coprocessor move delay > > hazard. How many times do I need to request to get it fixed (counting > > three so far)? > > This is because your request had follow-ups from Huacai and Marco that > were left unanswered: > > https://lore.kernel.org/all/CAAhV-H5ptAzHTPAr1bxrgByZrnFmMK8zJ68Z++RwC=NHWjqZmw@xxxxxxxxxxxxxx/ The conclusion made there is however wrong: `local_irq_enable' code plays no tricks with instruction scheduling and lets the toolchain resolve any pipeline hazards automatically, while `__r4k_wait' arranges for instructions to be scheduled by hand and any hazards resolved by the human writer of the code. There's even explicit `.set reorder' in `local_irq_enable', which is redundant, because it's the default mode for inline assembly. And I can't emphasise it enough: manual instruction scheduling is tough and ought to be restricted to cases where there is no other way really, such as for placing an instruction in a branch delay slot where there is a data antidependency between the branch and the delay-slot instruction. Yet this approach has often been used by code authors for other reasons (or I daresay no reason at all), leaving it up to the maintainers to keep the code working in the changing conditions while the submitter has long gone. I converted some of such code in the past, but it also takes time and effort that does not come for free. > https://lore.kernel.org/all/CAAofZF4HAczyRmuRe-JmQ2wcZatevLwGTOMLf1V1okGbj7q5Wg@xxxxxxxxxxxxxx/ I missed that one, sorry. A ping would have helped, and I never have an issue with being pinged. I do hope I have now addressed that concern with my other reply. Thank you for the pointers. > We have detected this longstanding architecture specific timer handling bug on > loongson and MIPS and we could have just dropped a report and let you guys deal with > it. Instead we decided to spend time ourselves (especially Marco) working on > fixes for these architectures we don't run and which we are not familiar with, > alongway taking reviews seriously and patiently re-iterating accordingly. Thank you for your effort, really appreciated. Any fixes need to be technically correct however, it makes no sense to get one bug replaced with another one. We've got enough technical debt accumulated already with a platform that no longer has any commercial support and relies solely on voluteers keeping it alive in their limited spare time. I do have a long list of outstanding issues to address and ever so little time to take care of them, with hardware problems additionally kicking in and distracting every so often too. > So please be gentle with us. As always, but also emphatic on this occasion. We're in the same boat really, striving against the lack of resources and issues piling, and now we've made some progress. Thank you for your understanding. Maciej
