Re: __{read,write}_once
On Fri, Nov 22, 2019 at 08:42:19AM +0700, Robert Elz wrote: > Date:Fri, 22 Nov 2019 01:04:56 +0100 > From:Kamil Rytarowski > Message-ID: <1a9d9b40-42fe-be08-d7b3-e6ecead5b...@gmx.com> > > > | I think that picking C11 terminology is the way forward. > > Use a name like that iff the intent is to also exactly match the > semantics implied, otherwise it will only create more confusion. And if the implied semantics isn't even totally clear, do not hide it behind a macro at all? Martin
Re: PT32_[GS]ETXMMREGS for amd64 (Was: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h)
On Fri, 2019-11-22 at 10:10 +0900, Rin Okuyama wrote: > Hi, thank you for your review! > > On 2019/11/22 0:48, Kamil Rytarowski wrote: > > On 21.11.2019 10:49, Rin Okuyama wrote: > ... > > > I wrote a draft version of patch which adds PT32_[GS]ETXMMREGS support: > > > > > > http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch > > > > > > With this patch, XMM-related tests pass for COMPAT_NETBSD32 on amd64. > > > > > > Some remarks: > > > > > > (1) PT_[GS]ETXMMREGS ptrace(2) commands are added to . > > > These are only used for COMPAT_NETBSD32, and not exposed to userland. > > > > > > > This is correct. > > We don't want to export XMMREGS to amd64 users. > > > > Pleae remove /* */ from this part: > > > > +/* > > + "PT_GETXMMREGS", \ > > + "PT_SETXMMREGS" > > + */ > > Yes, I will. > > > This will allow ktruss and related software to have meaningful form for > > compat32 ptracing. > > > > > (2) COMPAT_NETBSD32 codes are called from process_machdep.c via > > > module_hook framework. This may be too much though... > > > > > > > I have no opinion here. > > Now, Paul and I are discussed how to improve the usage of module_hook. > I will update this part in accordance with his advice. > > > > Comments? > > > > > > > I will leave this to be reviewed by mgorny@. Adding him to CC. > > I see. Hi, mgorny@. Please look into it! > > > http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch > > > > It seems correct at a first glance. The hook logic is unknown to me, so I can't comment on that. If I was to be picky, I'd prefer if changes in existing code that are not exactly relevant to adding this were split into a separate patches (e.g. changing int to bool, renaming valid* func). -- Best regards, Michał Górny
re: __{read,write}_once
_always()?
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
On 2019/11/22 10:56, Christos Zoulas wrote: In article <679493cf-3e85-f56d-85e4-dfaf7958a...@gmail.com>, Rin Okuyama wrote: ... This was my misunderstanding. These codes are used when tracer is 64-bit and traced is 32-bit. Don't know whether this is useful though... Yes, and someone broke them because all the ptrace 64->32 calls for registers seem to return 0 now. Was that code changed recently? ... I fixed it: http://mail-index.netbsd.org/source-changes/2019/11/22/msg111068.html Now, gdb/amd64 seems to work for i386 binaries to some extent, at least. Thanks, rin
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
In article <679493cf-3e85-f56d-85e4-dfaf7958a...@gmail.com>,
Rin Okuyama wrote:
>On 2019/11/20 20:12, Rin Okuyama wrote:
>> On 2019/11/19 22:59, Kamil Rytarowski wrote:
>...
>>>
>>> We still miss compat32 support for PT_GETXMMREGS and PT_SETXMMREGS, at
>>> some point of time we shall add it for completeness.
>>
>> Thank you!
>>
>> With amd64/netbsd32_machdep.c rev 1.130, all tests in t_ptrace* pass in
>> COMPAT_NETBSD32 on amd64, except for that involved with XMM registers.
>> I will examine how to implement PT32_[GS]ETXMMREGS.
>
>I wrote a draft version of patch which adds PT32_[GS]ETXMMREGS support:
>
>http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch
>
>With this patch, XMM-related tests pass for COMPAT_NETBSD32 on amd64.
>
>Some remarks:
>
>(1) PT_[GS]ETXMMREGS ptrace(2) commands are added to .
> These are only used for COMPAT_NETBSD32, and not exposed to userland.
>
>(2) COMPAT_NETBSD32 codes are called from process_machdep.c via
> module_hook framework. This may be too much though...
>
>Comments?
>
>> Also, it seems that some COMPAT_NETBSD32 related codes for amd64 need to
>> be cleaned up. For example, there remain COMPAT_NETBSD32 codes in
>> amd64/process_machdep.c, that are no longer used:
>>
>> https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#129
>> https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#183
>> ...
>>
>> I will examine them too.
>
>This was my misunderstanding. These codes are used when tracer is 64-bit
>and traced is 32-bit. Don't know whether this is useful though...
Yes, and someone broke them because all the ptrace 64->32 calls for
registers seem to return 0 now. Was that code changed recently?
christos
[8:55pm] 1404>cat hello.c
#include
#include
int
main(void)
{
printf("hello world\n");
sleep(1);
return 0;
}
[8:56pm] 1405>cc -g -m32 hello.c
[8:56pm] 1406>gdb ./a.out
GNU gdb (GDB) 8.3
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64--netbsd".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./a.out...
(gdb) break main
Breakpoint 1 at 0x80488be: file hello.c, line 7.
(gdb) r
Starting program: /u/christos/a.out
Program received signal SIGTRAP, Trace/breakpoint trap.
0x in ?? ()
(gdb)
Re: __{read,write}_once
Date:Fri, 22 Nov 2019 01:04:56 +0100 From:Kamil Rytarowski Message-ID: <1a9d9b40-42fe-be08-d7b3-e6ecead5b...@gmx.com> | I think that picking C11 terminology is the way forward. Use a name like that iff the intent is to also exactly match the semantics implied, otherwise it will only create more confusion. kre
PT32_[GS]ETXMMREGS for amd64 (Was: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h)
Hi, thank you for your review! On 2019/11/22 0:48, Kamil Rytarowski wrote: On 21.11.2019 10:49, Rin Okuyama wrote: ... I wrote a draft version of patch which adds PT32_[GS]ETXMMREGS support: http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch With this patch, XMM-related tests pass for COMPAT_NETBSD32 on amd64. Some remarks: (1) PT_[GS]ETXMMREGS ptrace(2) commands are added to . These are only used for COMPAT_NETBSD32, and not exposed to userland. This is correct. We don't want to export XMMREGS to amd64 users. Pleae remove /* */ from this part: +/* + "PT_GETXMMREGS", \ + "PT_SETXMMREGS" + */ Yes, I will. This will allow ktruss and related software to have meaningful form for compat32 ptracing. (2) COMPAT_NETBSD32 codes are called from process_machdep.c via module_hook framework. This may be too much though... I have no opinion here. Now, Paul and I are discussed how to improve the usage of module_hook. I will update this part in accordance with his advice. Comments? I will leave this to be reviewed by mgorny@. Adding him to CC. I see. Hi, mgorny@. Please look into it! http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch Also, it seems that some COMPAT_NETBSD32 related codes for amd64 need to be cleaned up. For example, there remain COMPAT_NETBSD32 codes in amd64/process_machdep.c, that are no longer used: https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#129 https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#183 ... I will examine them too. This was my misunderstanding. These codes are used when tracer is 64-bit and traced is 32-bit. Don't know whether this is useful though... Thanks, rin Maxime added it. If you want to change it, please consult with maxv@. OK. It must be useful for a tracer capable for both 64- and 32-bit processes (although, we also need to provide x87-style fpregs for 64-bit tracer in some way). Thanks, rin
Re: __{read,write}_once
On Thu, Nov 21, 2019 at 07:19:51PM +0100, Maxime Villard wrote: > Le 18/11/2019 à 19:49, David Holland a écrit : > >On Sun, Nov 17, 2019 at 02:35:43PM +, Mindaugas Rasiukevicius wrote: > > > David Holland wrote: > > > > > I see the potential source of confusion, but just think about: what > > > > > could "atomic" possibly mean for loads or stores? A load is just a > > > > > load, there is no RMW cycle, conceptually; inter-locking the > > operation > > > > > does not make sense. For anybody who attempts to reason about this, > > > > > it should not be too confusing, plus there are man pages. > > > > > > > > ...that it's not torn. > > > > > > > > As far as names... there are increasingly many slightly different > > > > types of atomic and semiatomic operations. > > > > > > > > I think it would be helpful if someone came up with a comprehensive > > > > naming scheme for all of them (including ones we don't currently have > > > > that we're moderately likely to end up with later...) > > > > > > Yes, the meaning of "atomic" has different flavours and describes > > different > > > set of properties in different fields (operating systems vs databases vs > > > distributed systems vs ...) and, as we can see, even within the fields. > > > > > > Perhaps not ideal, but "atomic loads/stores" and "relaxed" are already > > the > > > dominant terms. > > > >Yes, but "relaxed" means something else... let me be clearer since I > >wasn't before: I would expect e.g. atomic_inc_32_relaxed() to be > >distinguished from atomic_inc_32() or maybe atomic_inc_32_ordered() by > >whether or not multiple instances of it are globally ordered, not by > >whether or not it's actually atomic relative to other cpus. > > > >Checking the prior messages indicates we aren't currently talking > >about atomic_inc_32_relaxed() but only about atomic_load_32_relaxed() > >and atomic_store_32_relaxed(), which would be used together to > >generate a local counter. This is less misleading, but I'm still not > >convinced it's a good choice of names given that we might reasonably > >later on want to have atomic_inc_32_relaxed() and > >atomic_inc_32_ordered() that differ as above. > > > > > I think it is pointless to attempt to reinvent the wheel > > > here. It is terminology used by C11 (and C++11) and accepted by various > > > technical literature and, at this point, by academia (if you look at the > > > recent papers on memory models -- it's pretty much settled). These terms > > > are not too bad; it could be worse; and this bit is certainly not the > > worst > > > part of C11. So, I would just move on. > > > >Is it settled? My experience with the academic literature has been > >that everyone uses their own terminology and the same words are > >routinely found to have subtly different meanings from one paper to > >the next and occasionally even within the same paper. :-/ But I > >haven't been paying much attention lately due to being preoccupied > >with other things. > > So in the end which name do we use? Are people really unhappy with _racy()? > At least it has a general meaning, and does not imply atomicity or ordering. _racy() doesn't really get at the intended meaning, and everything in C11 is racy unless you arrange otherwise by using mutexes, atomics, etc. The suffix has very little content. Names such as load_/store_fully() or load_/store_entire() or load_/store_completely() get to the actual semantics: at the program step implied by the load_/store_entire() expression, the memory constituting the named variable is loaded/stored in its entirety. In other words, the load cannot be drawn out over more than one local program step. Whether or not the load/store is performed in one step with respect to interrupts or other threads is not defined. I feel like load_entire() and store_entire() get to the heart of the matter while being easy to speak, but _fully() or _completely() seem fine. Dave -- David Young dyo...@pobox.comUrbana, IL(217) 721-9981
Re: __{read,write}_once
On 22.11.2019 00:53, Robert Elz wrote:
> Date:Thu, 21 Nov 2019 19:19:51 +0100
> From:Maxime Villard
> Message-ID:
>
> | So in the end which name do we use? Are people really unhappy with
> _racy()?
> | At least it has a general meaning, and does not imply atomicity or
> ordering.
>
> I dislike naming discussions, as in general, while there are often a
> bunch of obviously incorrect names (here for example, read_page_data() ...)
> there is very rarely one obviously right answer, and it all really just
> becomes
> a matter of choice for whoever is doing it.
>
> Nevertheless, perhaps something that says more what is actually happening,
> rather than mentions what doesn't matter (races here), so perhaps something
> like {read,write}_single_cycle() or {read,write}_1_bus_xact() or something
> along those lines ?
>
> kre
>
I think that picking C11 terminology is the way forward. It's settled
probably forever now and it will be simpler to find corresponding
documentation and specification of it.
signature.asc
Description: OpenPGP digital signature
Re: __{read,write}_once
Date:Thu, 21 Nov 2019 19:19:51 +0100
From:Maxime Villard
Message-ID:
| So in the end which name do we use? Are people really unhappy with _racy()?
| At least it has a general meaning, and does not imply atomicity or ordering.
I dislike naming discussions, as in general, while there are often a
bunch of obviously incorrect names (here for example, read_page_data() ...)
there is very rarely one obviously right answer, and it all really just becomes
a matter of choice for whoever is doing it.
Nevertheless, perhaps something that says more what is actually happening,
rather than mentions what doesn't matter (races here), so perhaps something
like {read,write}_single_cycle() or {read,write}_1_bus_xact() or something
along those lines ?
kre
Re: __{read,write}_once
Le 18/11/2019 à 19:49, David Holland a écrit : On Sun, Nov 17, 2019 at 02:35:43PM +, Mindaugas Rasiukevicius wrote: > David Holland wrote: > > > I see the potential source of confusion, but just think about: what > > > could "atomic" possibly mean for loads or stores? A load is just a > > > load, there is no RMW cycle, conceptually; inter-locking the operation > > > does not make sense. For anybody who attempts to reason about this, > > > it should not be too confusing, plus there are man pages. > > > > ...that it's not torn. > > > > As far as names... there are increasingly many slightly different > > types of atomic and semiatomic operations. > > > > I think it would be helpful if someone came up with a comprehensive > > naming scheme for all of them (including ones we don't currently have > > that we're moderately likely to end up with later...) > > Yes, the meaning of "atomic" has different flavours and describes different > set of properties in different fields (operating systems vs databases vs > distributed systems vs ...) and, as we can see, even within the fields. > > Perhaps not ideal, but "atomic loads/stores" and "relaxed" are already the > dominant terms. Yes, but "relaxed" means something else... let me be clearer since I wasn't before: I would expect e.g. atomic_inc_32_relaxed() to be distinguished from atomic_inc_32() or maybe atomic_inc_32_ordered() by whether or not multiple instances of it are globally ordered, not by whether or not it's actually atomic relative to other cpus. Checking the prior messages indicates we aren't currently talking about atomic_inc_32_relaxed() but only about atomic_load_32_relaxed() and atomic_store_32_relaxed(), which would be used together to generate a local counter. This is less misleading, but I'm still not convinced it's a good choice of names given that we might reasonably later on want to have atomic_inc_32_relaxed() and atomic_inc_32_ordered() that differ as above. > I think it is pointless to attempt to reinvent the wheel > here. It is terminology used by C11 (and C++11) and accepted by various > technical literature and, at this point, by academia (if you look at the > recent papers on memory models -- it's pretty much settled). These terms > are not too bad; it could be worse; and this bit is certainly not the worst > part of C11. So, I would just move on. Is it settled? My experience with the academic literature has been that everyone uses their own terminology and the same words are routinely found to have subtly different meanings from one paper to the next and occasionally even within the same paper. :-/ But I haven't been paying much attention lately due to being preoccupied with other things. So in the end which name do we use? Are people really unhappy with _racy()? At least it has a general meaning, and does not imply atomicity or ordering.
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
On 21.11.2019 10:49, Rin Okuyama wrote: > On 2019/11/20 20:12, Rin Okuyama wrote: >> On 2019/11/19 22:59, Kamil Rytarowski wrote: > ... >>> >>> We still miss compat32 support for PT_GETXMMREGS and PT_SETXMMREGS, at >>> some point of time we shall add it for completeness. >> >> Thank you! >> >> With amd64/netbsd32_machdep.c rev 1.130, all tests in t_ptrace* pass in >> COMPAT_NETBSD32 on amd64, except for that involved with XMM registers. >> I will examine how to implement PT32_[GS]ETXMMREGS. > > I wrote a draft version of patch which adds PT32_[GS]ETXMMREGS support: > > http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch > > With this patch, XMM-related tests pass for COMPAT_NETBSD32 on amd64. > > Some remarks: > > (1) PT_[GS]ETXMMREGS ptrace(2) commands are added to . > These are only used for COMPAT_NETBSD32, and not exposed to userland. > This is correct. We don't want to export XMMREGS to amd64 users. Pleae remove /* */ from this part: +/* + "PT_GETXMMREGS", \ + "PT_SETXMMREGS" + */ This will allow ktruss and related software to have meaningful form for compat32 ptracing. > (2) COMPAT_NETBSD32 codes are called from process_machdep.c via > module_hook framework. This may be too much though... > I have no opinion here. > Comments? > I will leave this to be reviewed by mgorny@. Adding him to CC. >> Also, it seems that some COMPAT_NETBSD32 related codes for amd64 need to >> be cleaned up. For example, there remain COMPAT_NETBSD32 codes in >> amd64/process_machdep.c, that are no longer used: >> >> https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#129 >> >> https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#183 >> >> ... >> >> I will examine them too. > > This was my misunderstanding. These codes are used when tracer is 64-bit > and traced is 32-bit. Don't know whether this is useful though... > > Thanks, > rin Maxime added it. If you want to change it, please consult with maxv@. signature.asc Description: OpenPGP digital signature
Re: netbsd32_{,u}int64 in sys/types.h for compat/sys/siginfo.h
On 2019/11/20 20:12, Rin Okuyama wrote: On 2019/11/19 22:59, Kamil Rytarowski wrote: ... We still miss compat32 support for PT_GETXMMREGS and PT_SETXMMREGS, at some point of time we shall add it for completeness. Thank you! With amd64/netbsd32_machdep.c rev 1.130, all tests in t_ptrace* pass in COMPAT_NETBSD32 on amd64, except for that involved with XMM registers. I will examine how to implement PT32_[GS]ETXMMREGS. I wrote a draft version of patch which adds PT32_[GS]ETXMMREGS support: http://www.netbsd.org/~rin/amd64-PT32_GSETXMMREGS-20191121.patch With this patch, XMM-related tests pass for COMPAT_NETBSD32 on amd64. Some remarks: (1) PT_[GS]ETXMMREGS ptrace(2) commands are added to . These are only used for COMPAT_NETBSD32, and not exposed to userland. (2) COMPAT_NETBSD32 codes are called from process_machdep.c via module_hook framework. This may be too much though... Comments? Also, it seems that some COMPAT_NETBSD32 related codes for amd64 need to be cleaned up. For example, there remain COMPAT_NETBSD32 codes in amd64/process_machdep.c, that are no longer used: https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#129 https://nxr.netbsd.org/xref/src/sys/arch/amd64/amd64/process_machdep.c#183 ... I will examine them too. This was my misunderstanding. These codes are used when tracer is 64-bit and traced is 32-bit. Don't know whether this is useful though... Thanks, rin
