Re: CVS commit: src/sys/arch/sparc64/sparc64

[Valery Ushakov]

On Mon, Feb 20, 2023 at 15:41:04 +0100, Martin Husemann wrote:

> On Mon, Feb 20, 2023 at 05:15:33PM +0300, Valery Ushakov wrote:
> > them up, b/c you cannot amend that comment.  To add to the fun, I
> > think releng scripts just clone the commit message on pull ups, so
> > that comment gets splattered all over the target branches too.
> 
> Yes - I try to manually remove them (which is easy if they are at the
> end of the last commit log in the batch) durint the pullup.

Data point:

  $ hg log -b netbsd-9 | grep 'XXX.*pullup.*9' | wc -l
  74

-uwe

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Valery Ushakov]

On Mon, Feb 20, 2023 at 22:35:40 +0700, Robert Elz wrote:

> Date:Mon, 20 Feb 2023 16:47:01 +0300
> From:Valery Ushakov 
> Message-ID:  
> 
>   | I wonder if we should stop abusing commit messages as pull-up
>   | reminders.  These XXX will not convey any useful information a few
>   | months down the line...
> 
> I think they're useful (if only I remembered to add them all the
> times I should) - it allows someone looking at the commit log to
> easily determine that the change is also intended for another branch.
> Then one can check and see if it happened or not, and if not, send
> a reminder if it is important/needed.
> 
> If the pullup annotation is missing, I tend to assume that the change
> is not intended to be pulled up - either it isn't applicable, or is
> something new that isn't appropriate for older releases.
> 
> That's why I will sometimes even include pullup annotations for ancient
> versions of NetBSD, even though I know they will never happen (never get
> submitted, much less acted upon) - just as an indication that the problem
> being fixed exists from long ago.

In that case they have no business being "XXX" :)

-uwe

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Robert Elz]

Date:Mon, 20 Feb 2023 16:47:01 +0300
From:Valery Ushakov 
Message-ID:  

  | I wonder if we should stop abusing commit messages as pull-up
  | reminders.  These XXX will not convey any useful information a few
  | months down the line...

I think they're useful (if only I remembered to add them all the
times I should) - it allows someone looking at the commit log to
easily determine that the change is also intended for another branch.
Then one can check and see if it happened or not, and if not, send
a reminder if it is important/needed.

If the pullup annotation is missing, I tend to assume that the change
is not intended to be pulled up - either it isn't applicable, or is
something new that isn't appropriate for older releases.

That's why I will sometimes even include pullup annotations for ancient
versions of NetBSD, even though I know they will never happen (never get
submitted, much less acted upon) - just as an indication that the problem
being fixed exists from long ago.

kre

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Martin Husemann]

On Mon, Feb 20, 2023 at 05:15:33PM +0300, Valery Ushakov wrote:
> them up, b/c you cannot amend that comment.  To add to the fun, I
> think releng scripts just clone the commit message on pull ups, so
> that comment gets splattered all over the target branches too.

Yes - I try to manually remove them (which is easy if they are at the
end of the last commit log in the batch) durint the pullup.

Martin

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Valery Ushakov]

On Mon, Feb 20, 2023 at 13:57:32 +, Taylor R Campbell wrote:

> > > XXX pullup-8
> > > XXX pullup-9
> > > XXX pullup-10
> > 
> > I wonder if we should stop abusing commit messages as pull-up
> > reminders.  These XXX will not convey any useful information a few
> > months down the line...
> 
> Happy to try a better mechanism if you have suggestions, but this is
> the best one I've found so far!  If I don't mark commits this way, I'm
> almost guaranteed not to pull them up.

I'm not sure any exists even with modern VCS.  May be something like a
branch that is not closed until it is merged (pulled-up) to all the
intended destination (but then the very notion of the closed branch
requires something more modern than CVS).  The problem with the XXX in
the commit message is that you don't easily know if you _did_ pull
them up, b/c you cannot amend that comment.  To add to the fun, I
think releng scripts just clone the commit message on pull ups, so
that comment gets splattered all over the target branches too.

-uwe

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Taylor R Campbell]

> Date: Mon, 20 Feb 2023 16:47:01 +0300
> From: Valery Ushakov 
> 
> On Mon, Feb 20, 2023 at 13:30:23 +, Taylor R Campbell wrote:
> 
> > Module Name:src
> > Committed By:   riastradh
> > Date:   Mon Feb 20 13:30:23 UTC 2023
> > 
> > Modified Files:
> > src/sys/arch/sparc64/sparc64: lock_stubs.s
> > 
> > Log Message:
> > sparc64: Add missing LoadStore ordering for mutex_enter stub.
> > 
> > XXX pullup-8
> > XXX pullup-9
> > XXX pullup-10
> 
> I wonder if we should stop abusing commit messages as pull-up
> reminders.  These XXX will not convey any useful information a few
> months down the line...

Happy to try a better mechanism if you have suggestions, but this is
the best one I've found so far!  If I don't mark commits this way, I'm
almost guaranteed not to pull them up.

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Valery Ushakov]

On Mon, Feb 20, 2023 at 13:30:23 +, Taylor R Campbell wrote:

> Module Name:  src
> Committed By: riastradh
> Date: Mon Feb 20 13:30:23 UTC 2023
> 
> Modified Files:
>   src/sys/arch/sparc64/sparc64: lock_stubs.s
> 
> Log Message:
> sparc64: Add missing LoadStore ordering for mutex_enter stub.
> 
> XXX pullup-8
> XXX pullup-9
> XXX pullup-10

I wonder if we should stop abusing commit messages as pull-up
reminders.  These XXX will not convey any useful information a few
months down the line...

-uwe

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Tobias Ulmer]

Oh, should've tested that. Survived kernels and distribution:

diff --git a/sys/arch/sparc64/sparc64/db_trace.c 
b/sys/arch/sparc64/sparc64/db_trace.c
index f5e35e79dd51..d94e5eb2d2ef 100644
--- a/sys/arch/sparc64/sparc64/db_trace.c
+++ b/sys/arch/sparc64/sparc64/db_trace.c
@@ -36,6 +36,7 @@ __KERNEL_RCSID(0, "$NetBSD: db_trace.c,v 1.52 2019/05/22 
07:40:09 martin Exp $")
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 


On Wed, May 22, 2019 at 03:02:13PM +0200, J. Hannken-Illjes wrote:
> This breaks the build of usr.sbin/crash:
> 
> /work/build/src/usr.sbin/crash/../../sys/arch/sparc64/sparc64/db_trace.c: In 
> function 'db_stack_trace_print':
> /work/build/src/usr.sbin/crash/../../sys/arch/sparc64/sparc64/db_trace.c:166:37:
>  error: 'VM_MAX_KERNEL_ADDRESS' undeclared (first use in this function); did 
> you mean 'VM_MAXADDRESS'?
> if (frame < KERNBASE || frame >= VM_MAX_KERNEL_ADDRESS)
>  ^
>  VM_MAXADDRESS
> /work/build/src/usr.sbin/crash/../../sys/arch/sparc64/sparc64/db_trace.c:166:37:
>  note: each undeclared identifier is reported only once for each function it 
> appears in
> 
> --
> J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig
> 
> > On 22. May 2019, at 09:40, Martin Husemann  wrote:
> > 
> > Module Name:src
> > Committed By:   martin
> > Date:   Wed May 22 07:40:09 UTC 2019
> > 
> > Modified Files:
> > src/sys/arch/sparc64/sparc64: db_trace.c
> > 
> > Log Message:
> > Fix previous and use the original patch from PR port-sparc64/54221
> > instead (XXX should fix comments in param.h)
> > 
> > 
> > To generate a diff of this commit:
> > cvs rdiff -u -r1.51 -r1.52 src/sys/arch/sparc64/sparc64/db_trace.c
> > 
> > Please note that diffs are not public domain; they are subject to the
> > copyright notices on the relevant files.
> > 
> 

Re: CVS commit: src/sys/arch/sparc64/sparc64

[J. Hannken-Illjes]

This breaks the build of usr.sbin/crash:

/work/build/src/usr.sbin/crash/../../sys/arch/sparc64/sparc64/db_trace.c: In 
function 'db_stack_trace_print':
/work/build/src/usr.sbin/crash/../../sys/arch/sparc64/sparc64/db_trace.c:166:37:
 error: 'VM_MAX_KERNEL_ADDRESS' undeclared (first use in this function); did 
you mean 'VM_MAXADDRESS'?
if (frame < KERNBASE || frame >= VM_MAX_KERNEL_ADDRESS)
 ^
 VM_MAXADDRESS
/work/build/src/usr.sbin/crash/../../sys/arch/sparc64/sparc64/db_trace.c:166:37:
 note: each undeclared identifier is reported only once for each function it 
appears in

--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig

> On 22. May 2019, at 09:40, Martin Husemann  wrote:
> 
> Module Name:  src
> Committed By: martin
> Date: Wed May 22 07:40:09 UTC 2019
> 
> Modified Files:
>   src/sys/arch/sparc64/sparc64: db_trace.c
> 
> Log Message:
> Fix previous and use the original patch from PR port-sparc64/54221
> instead (XXX should fix comments in param.h)
> 
> 
> To generate a diff of this commit:
> cvs rdiff -u -r1.51 -r1.52 src/sys/arch/sparc64/sparc64/db_trace.c
> 
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
> 



signature.asc
Description: Message signed with OpenPGP

re: CVS commit: src/sys/arch/sparc64/sparc64

[matthew green]

"Martin Husemann" writes:
> Module Name:  src
> Committed By: martin
> Date: Fri Jan  4 16:25:06 UTC 2019
> 
> Modified Files:
>   src/sys/arch/sparc64/sparc64: autoconf.c
> 
> Log Message:
> PR port-sparc64/53830: adapt QEMU workarounds to newer OpenBIOS device
> tree layout.

why this part?

+   regs[0] = 42;

it seems useless.

thanks.


.mrg.

re: CVS commit: src/sys/arch/sparc64/sparc64

[matthew green]

"Palle Lyckegaard" writes:
> Module Name:  src
> Committed By: palle
> Date: Sun Aug 27 19:31:44 UTC 2017
> 
> Modified Files:
>   src/sys/arch/sparc64/sparc64: cpu.c
> 
> Log Message:
> sun4v: Change clk and sclk variables to unsigned type so modern faster
> systems with CPU frequencies above 2 Ghz are shown correctly. Example
> is a 3599.910 MHz SPARC T5-2 system that otherwise is shown
> incorrectly as -695.-57 MHz. Based on code from OpenBSD cpu.c rev.
> 1.41. Verified on sun4u using qemu and sun4v on SPARC T5-2

cool :)

shouldn't we move to a 64 bit value?  4.3ghz isn't far away from 3.6..


.mrg.

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Palle Lyckegaard]

fixed thanks



On Sun, 12 Feb 2017, Takeshi Nakayama wrote:


Date: Sun, 12 Feb 2017 04:28:58
From: Takeshi Nakayama <nakay...@netbsd.org>
To: source-changes-d@NetBSD.org, pa...@netbsd.org
Subject: Re: CVS commit: src/sys/arch/sparc64/sparc64


"Palle Lyckegaard" <pa...@netbsd.org> wrote



Module Name:src
Committed By:   palle
Date:   Sat Feb 11 23:41:36 UTC 2017

Modified Files:
src/sys/arch/sparc64/sparc64: trap.c

Log Message:
sun4v: Fix calculation of mmu data fault address (pointer arithmetic)


paddr_t is "unsigned long int" or "unsigned long long int", so not
a pointer.  Why did you add "/ sizeof(paddr_t)" ?

-- Takeshi Nakayama

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Takeshi Nakayama]

>>> "Palle Lyckegaard"  wrote

> Module Name:  src
> Committed By: palle
> Date: Sat Feb 11 23:41:36 UTC 2017
> 
> Modified Files:
>   src/sys/arch/sparc64/sparc64: trap.c
> 
> Log Message:
> sun4v: Fix calculation of mmu data fault address (pointer arithmetic)

paddr_t is "unsigned long int" or "unsigned long long int", so not
a pointer.  Why did you add "/ sizeof(paddr_t)" ?

-- Takeshi Nakayama

re: CVS commit: src/sys/arch/sparc64/sparc64

[matthew green]

thanks for fixing these problems.  i was espcially amused by the
code that was if (copyout() || copyout() || suword()).

> Modified Files:
>   src/sys/arch/sparc64/sparc64: machdep.c netbsd32_machdep.c
>   sunos_machdep.c
> 
> Log Message:
> remove all MD uses of suword(), replace by copyout()

hmm..

+   sp = (register32_t)(uintptr_t)oldsp;

there are macros for doing this, aren't there?

i have an uncommited patch for netbsd32 that adds checks to ensure
that any such "truncation" actually doesn't modify the value if
DIAGNOSTIC, and it should apply to the above, but it's really hard
to find the above in other code...

thanks.


.mrg.

re: CVS commit: src/sys/arch/sparc64/sparc64

[matthew green]

> Module Name:  src
> Committed By: christos
> Date: Mon Nov  9 02:13:41 UTC 2015
> 
> Modified Files:
>   src/sys/arch/sparc64/sparc64: syscall.c
> 
> Log Message:
> fix printf formats.

yuck, can't you just use PRId64 instead of the casts?  these are
int64 members.  also, using %# vs 0x% :-)


.mrg.

re: CVS commit: src/sys/arch/sparc64/sparc64

[Palle Lyckegaard]

On Mon, 3 Nov 2014, matthew green wrote:


we don't need the #ifdef's here.  CPU_ISSUN4V is 0 for
normal kernels, so the above is compiled out anyway.
there's a bunch of other places this is done as well that
we don't need it.. could you this up at some point?




sure will do

re: CVS commit: src/sys/arch/sparc64/sparc64

[matthew green]

  we don't need the #ifdef's here.  CPU_ISSUN4V is 0 for
  normal kernels, so the above is compiled out anyway.
  there's a bunch of other places this is done as well that
  we don't need it.. could you this up at some point?

totally gimplished that up :-)  clean of course :-)

 sure will do

thanks.

re: CVS commit: src/sys/arch/sparc64/sparc64

[matthew green]

+ #ifdef SUN4V
+   if (CPU_ISSUN4V)
+   func = sparc64_ipi_dcache_flush_page_sun4v;
+   else if (CPU_IS_USIII_UP())
+ #else 
if (CPU_IS_USIII_UP())
+ #endif
func = sparc64_ipi_dcache_flush_page_usiii;

we don't need the #ifdef's here.  CPU_ISSUN4V is 0 for
normal kernels, so the above is compiled out anyway.
there's a bunch of other places this is done as well that
we don't need it.. could you this up at some point?

thanks.


.mrg.

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Martin Husemann]

On Thu, Sep 12, 2013 at 10:12:23PM +0900, Takeshi Nakayama wrote:
 This change provides a chance to select a prefer timecounter to
 users via sysctl kern.timecounter.hardware.
 
 stick-counter's quality is larger than tick-counter's one. The
 default choice becomes to stick-counter, so it's no problem.

Once we offer different cpu speeds we will have to stop adding %tick as
timesource option for that cpu, so this should automatically all work
well.

Martin

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Takeshi Nakayama]

 Michael macallan1...@gmail.com wrote

 Hello,
 
 on Thursday 22 August 2013 06:00:43 Takeshi Nakayama wrote:
  Module Name:src
  Committed By:   nakayama
  Date:   Thu Aug 22 10:00:43 UTC 2013
 
  Modified Files:
  src/sys/arch/sparc64/sparc64: clock.c
 
  Log Message:
  Make timecounter tick-counter mandatory.
 
 This is going to bite us when we actually mess with the CPU's clock speed ( 
 which is the whole point of using %stick and STICK )

This change provides a chance to select a prefer timecounter to
users via sysctl kern.timecounter.hardware.

stick-counter's quality is larger than tick-counter's one. The
default choice becomes to stick-counter, so it's no problem.

-- Takeshi Nakayama

Re: CVS commit: src/sys/arch/sparc64/sparc64

[David Laight]

On Sat, Mar 03, 2012 at 03:17:32AM +, Takeshi Nakayama wrote:
 Module Name:  src
 Committed By: nakayama
 Date: Sat Mar  3 03:17:32 UTC 2012
 
 Modified Files:
   src/sys/arch/sparc64/sparc64: locore.s
 
 Log Message:
 Fix the root cause of the hack disable optimizations for uvm_bio.c
 on 32 bit kernels.
 
 gcc converts a division in the calculation of UBC_UMAP_ADDR macro
 to multiplication (smul or combination of add/sll), and the
 register of its result contains a garbage in upper 32 bits (the
 upper 32 bits of smul/add/sll's result isn't zero cleared).
 
 Then it passes to pseg_get{,_real} through pmap_extract without the
 zero clear of upper 32 bits in the optimization case.  So the
 result of pseg_get and pmap_extact sometimes gets screwed up.

Is that a gcc bug?

Or are the high register bits usually undefined for 32bit values,
and this to do with using 64bit asm in a 32bit kernel?

David

-- 
David Laight: da...@l8s.co.uk

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Takeshi Nakayama]

 David Laight da...@l8s.co.uk wrote

 Is that a gcc bug?

I don't know.

 Or are the high register bits usually undefined for 32bit values,
 and this to do with using 64bit asm in a 32bit kernel?

But I guess it's undefined from looking at the generated codes.
Our kernel code is shared between 32-bit and 64-bit, and 64-bit asm
is usually used in locore.s.

-- Takeshi Nakayama

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Martin Husemann]

On Sat, Mar 03, 2012 at 08:50:50AM +, David Laight wrote:
 
 Is that a gcc bug?

No, gcc calls a function with 32bit abi and expects it to ignore the upper bits
in that register.

The patch makes it so. Good catch!

Martin

Re: CVS commit: src/sys/arch/sparc64/sparc64

[Alistair Crooks]

On Sat, Mar 03, 2012 at 03:17:32AM +, Takeshi Nakayama wrote:
 Module Name:  src
 Committed By: nakayama
 Date: Sat Mar  3 03:17:32 UTC 2012
 
 Modified Files:
   src/sys/arch/sparc64/sparc64: locore.s
 
 Log Message:
 Fix the root cause of the hack disable optimizations for uvm_bio.c
 on 32 bit kernels.
 
 gcc converts a division in the calculation of UBC_UMAP_ADDR macro
 to multiplication (smul or combination of add/sll), and the
 register of its result contains a garbage in upper 32 bits (the
 upper 32 bits of smul/add/sll's result isn't zero cleared).
 
 Then it passes to pseg_get{,_real} through pmap_extract without the
 zero clear of upper 32 bits in the optimization case.  So the
 result of pseg_get and pmap_extact sometimes gets screwed up.

Oh, good catch!

Thanks for fixing this one.

Best,
Al