Re: __{read,write}_once

2019-11-06 Thread Mindaugas Rasiukevicius
Maxime Villard  wrote:
> > - If we do not want to stick with the C11 API (its emulation), then I
> > would suggest to use the similar terminology, e.g. atomic_load_relaxed()
> > and atomic_store_relaxed(), or Linux READ_ONCE()/WRITE_ONCE().  There is
> > no reason invent new terminology; it might just add more confusion.
> 
> But... Linux's READ_ONCE/WRITE_ONCE are not actually atomic, is that
> correct? So there is a significant semantic difference.

They are "atomic" in a sense that they prevent from tearing, fusing and
invented loads/stores.  Terms and conditions apply (e.g. they assume properly
aligned and word-sized accesses).  Additionally, READ_ONCE() provides a
data-dependency barrier, applicable only to DEC Alpha.  I think it was
the right decision on the Linux side (even though trying to get all the
data-dependency barriers right these days is kind of a lost cause, IMO).

So, READ_ONCE()/WRITE_ONCE() is more or less equivalent to the C11 atomic
load/stores routines with memory_order_relaxed (or memory_order_consume,
if you care about DEC Alpha).

> Also, in my original patch I marked two branches of subr_xcall.c, but it
> seems to me now they are actually races: ie xc_wait(), the read of
> 'xc_donep' could be made by two 4-byte fetches on 32bit arches (at
> least). Between the two, an xc thread could have updated the value. So
> there is an actual race which could possibly result in returning early
> while we shouldn't have. Does that look correct to you?

Correct.

There is more code in the NetBSD kernel which needs fixing.  I would say
pretty much all lock-free code should be audited.

Back in time, certain liberties compilers have were not fully understood or
were not considered as real problems (after all, C memory model concerning
many of these aspects was not defined).  Times changed, compilers became
much more aggressive and the old code needs to catch up.

-- 
Mindaugas


Re: __{read,write}_once

2019-11-06 Thread Kamil Rytarowski
On 06.11.2019 20:38, Mindaugas Rasiukevicius wrote:
> Maxime Villard  wrote:
>> There are cases in the kernel where we read/write global memory
>> locklessly, and accept the races either because it is part of the design
>> (eg low-level scheduling) or we simply don't care (eg global stats).
>>
>> In these cases, we want to access the memory only once, and need to ensure
>> the compiler does not split that access in several pieces, some of which
>> may be changed by concurrent accesses. There is a Linux article [1] about
>> this, and also [2]. I'd like to introduce the following macros:
>>
>> <...>
> 
> A few comments for everybody here:
> 
> - There is a general need in the NetBSD kernel for atomic load/store
> semantics.  This is because plain accesses (loads/stores) are subject
> to _tearing_, _fusing_ and _invented_ loads/stores.  I created a new
> thread which might help to clarify these and various other aspects:
> 
> http://mail-index.netbsd.org/tech-kern/2019/11/06/msg025664.html
> 

Thank you.

> - In C11, this can be handled with atomic_{load,store}_explicit() and
> memory_order_relaxed (or stronger memory order).
> 
> - If we do not want to stick with the C11 API (its emulation), then I
> would suggest to use the similar terminology, e.g. atomic_load_relaxed()
> and atomic_store_relaxed(), or Linux READ_ONCE()/WRITE_ONCE().  There is
> no reason invent new terminology; it might just add more confusion.
> 
> Thanks.
> 

I have no opinion here, just please do the right thing. Unless there are
any shortcomings it would be nice to follow closely C11.

If that information helps or not, we also need C11/C++14 libatomic-like
library in userland.



signature.asc
Description: OpenPGP digital signature


re: __{read,write}_once

2019-11-06 Thread matthew green
> - If we do not want to stick with the C11 API (its emulation), then I
> would suggest to use the similar terminology, e.g. atomic_load_relaxed()
> and atomic_store_relaxed(), or Linux READ_ONCE()/WRITE_ONCE().  There is
> no reason invent new terminology; it might just add more confusion.

i really do not like the "once" name.

even though i'm familiar with the actual desired semantics,
i have to ignore my builtin meaning that comes first.  to me,
"once" initially reads as compiler will only do it one time,
ever, not every time it is called.  like pthread "once".

can we make them __read_always() and __write_always()?


.mrg.


Re: __{read,write}_once

2019-11-06 Thread Maxime Villard

Le 06/11/2019 à 20:38, Mindaugas Rasiukevicius a écrit :

Maxime Villard  wrote:

There are cases in the kernel where we read/write global memory
locklessly, and accept the races either because it is part of the design
(eg low-level scheduling) or we simply don't care (eg global stats).

In these cases, we want to access the memory only once, and need to ensure
the compiler does not split that access in several pieces, some of which
may be changed by concurrent accesses. There is a Linux article [1] about
this, and also [2]. I'd like to introduce the following macros:

<...>


A few comments for everybody here:

- There is a general need in the NetBSD kernel for atomic load/store
semantics.  This is because plain accesses (loads/stores) are subject
to _tearing_, _fusing_ and _invented_ loads/stores.  I created a new
thread which might help to clarify these and various other aspects:

http://mail-index.netbsd.org/tech-kern/2019/11/06/msg025664.html


Thanks


- In C11, this can be handled with atomic_{load,store}_explicit() and
memory_order_relaxed (or stronger memory order).

- If we do not want to stick with the C11 API (its emulation), then I
would suggest to use the similar terminology, e.g. atomic_load_relaxed()
and atomic_store_relaxed(), or Linux READ_ONCE()/WRITE_ONCE().  There is
no reason invent new terminology; it might just add more confusion.


But... Linux's READ_ONCE/WRITE_ONCE are not actually atomic, is that correct?
So there is a significant semantic difference.

Also, in my original patch I marked two branches of subr_xcall.c, but it
seems to me now they are actually races: ie xc_wait(), the read of 'xc_donep'
could be made by two 4-byte fetches on 32bit arches (at least). Between the
two, an xc thread could have updated the value. So there is an actual race
which could possibly result in returning early while we shouldn't have. Does
that look correct to you?

Thanks


Re: __{read,write}_once

2019-11-06 Thread Mindaugas Rasiukevicius
Maxime Villard  wrote:
> There are cases in the kernel where we read/write global memory
> locklessly, and accept the races either because it is part of the design
> (eg low-level scheduling) or we simply don't care (eg global stats).
> 
> In these cases, we want to access the memory only once, and need to ensure
> the compiler does not split that access in several pieces, some of which
> may be changed by concurrent accesses. There is a Linux article [1] about
> this, and also [2]. I'd like to introduce the following macros:
>
> <...>

A few comments for everybody here:

- There is a general need in the NetBSD kernel for atomic load/store
semantics.  This is because plain accesses (loads/stores) are subject
to _tearing_, _fusing_ and _invented_ loads/stores.  I created a new
thread which might help to clarify these and various other aspects:

http://mail-index.netbsd.org/tech-kern/2019/11/06/msg025664.html

- In C11, this can be handled with atomic_{load,store}_explicit() and
memory_order_relaxed (or stronger memory order).

- If we do not want to stick with the C11 API (its emulation), then I
would suggest to use the similar terminology, e.g. atomic_load_relaxed()
and atomic_store_relaxed(), or Linux READ_ONCE()/WRITE_ONCE().  There is
no reason invent new terminology; it might just add more confusion.

Thanks.

-- 
Mindaugas


C11 memory fences, atomics, memory model

2019-11-06 Thread Mindaugas Rasiukevicius
Hi,

There have been some discussions amongst the developers about the C11
memory model.  It seemed that there was a lot of confusion and a general
lack of clarity in this subject.  I wrote a quick summary describing some
of the key points of the C11 memory model and key aspects of the memory
fence semantics as well as atomic operations.

This short document is supposed to help those who are familiar with the
lock-free programming in the pre-C11 world to better understand what
changed after C11.

https://github.com/rmind/stdc/blob/master/c11memfences.md
or txt: http://www.netbsd.org/~rmind/c11memfences.txt

Hope this helps.

Thanks.

-- 
Mindaugas


Re: __{read,write}_once

2019-11-06 Thread Marco Elver
On Wed, 6 Nov 2019 at 18:08, Maxime Villard  wrote:
>
> Le 06/11/2019 à 17:37, Marco Elver a écrit :
> > On Wed, 6 Nov 2019 at 16:51, Kamil Rytarowski  wrote:
> >>
> >> On 06.11.2019 16:44, Kamil Rytarowski wrote:
> >>> On 06.11.2019 15:57, Jason Thorpe wrote:
> 
> 
> > On Nov 6, 2019, at 5:41 AM, Kamil Rytarowski  wrote:
> >
> > On 06.11.2019 14:37, Jason Thorpe wrote:
> >>
> >>
> >>> On Nov 6, 2019, at 4:45 AM, Kamil Rytarowski  wrote:
> >>>
> >>> I propose __write_relaxed() / __read_relaxed().
> >>
> >> ...except that seems to imply the opposite of what these do.
> >>
> >> -- thorpej
> >>
> >
> > Rationale?
> >
> > This matches atomic_load_relaxed() / atomic_write_relaxed(), but we do
> > not deal with atomics here.
> 
>  Fair enough.  To me, the names suggest "compiler is allowed to apply 
>  relaxed constraints and tear the access if it wants" But apparently 
>  the common meaning is "relax, bro, I know what I'm doing".  If that's 
>  the case, I can roll with it.
> >
> > See below.
> >
>  -- thorpej
> 
> >>>
> >>> Unless I mean something this is exactly about relaxed constraints.
> >>
> >> miss*
> >>
> >>>
> >>> "Relaxed operation: there are no synchronization or ordering constraints
> >>> imposed on other reads or writes" and without "operation's atomicity is
> >>> guaranteed".
> >
> > In the memory consistency model world, "relaxed" has a very specific
> > meaning for loads/stores. It simply means that the compiler and
> > architecture is free to reorder the memory operation with respect to
> > previous or following memory operations in program order. But the
> > load/store still happens as one atomic operation.
> >
> > For programming-language memory models, "relaxed" appears in the
> > context of atomic memory operations, to define their ordering w.r.t.
> > other operations in program order. There is a distinction between
> > "relaxed atomic" and plain (unannotated) memory operations at the
> > programming language level.
> >
> > For plain operations, the compiler, in addition to the target
> > architecture, is free to reorder operations or even apply
> > optimizations that turn single plain operations into multiple
> > loads/stores [1, 2].
> > [1] https://lwn.net/Articles/793253/
> > [2] https://lwn.net/Articles/799218/
> >
> > In the Linux kernel READ_ONCE/WRITE_ONCE are one way to specify
> > "relaxed atomic read/write" for accesses that fit in a word (_ONCE
> > also works on things larger than a word, but probably aren't atomic
> > anymore). For most architectures the implementation is the same, and
> > merely avoids compiler optimizations; the exception is Alpha, where
> > READ_ONCE adds a memory barrier [3].
> > [3] https://www.kernel.org/doc/Documentation/memory-barriers.txt
> >
> >>> This is also similar to what suggested Google to apply to NetBSD in our
> >>> internal thread, but with a bit different naming.
> >
> > What you call the ops is entirely up to you. I would not use
> > '{read,write}_relaxed', since as you pointed out above, is probably
> > more confusing. This is why I suggested 'atomic_{read,write}_relaxed'.
> > If you do not like the 'atomic' in there, the next best thing is
> > probably '{read,write}_once'.
> >
> > Just note that you're now defining your own memory consistency model.
> > This is a complex topic that spans decades of work. The safest thing
> > is to stick as closely to the C11/C++11 memory model as possible [4].
> > The Linux kernel also has a memory model [5], but is significantly
> > more complex since it was defined after various primitives had been
> > introduced.
> > [4] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1479.htm
> > [5] 
> > https://github.com/torvalds/linux/blob/master/tools/memory-model/Documentation/explanation.txt
> >
> > Best Wishes,
> > -- Marco
>
> Thanks for the details.
>
> I must say I still have a doubt about the instructions actually emited
> by the compiler.
>
> Let's take READ_ONCE for example. What it does is basically just casting
> the pointer to volatile and doing the access. Two points of confusion:
>
>   - Is 'volatile' a guarantee that the compiler will emit only one
> instruction to fetch the value?

No, it depends on the access size among other things. 'volatile' just
communicates to the compiler that every access is a potentially
visible side-effect and avoid optimizations. 'volatile' on its own is
not at all sufficient for writing correct concurrent code.

READ_ONCE is not perfect in that sense, but it has been around for a
long time so it's pretty hard to change.

>   - Assuming there is only one instruction, strictly speaking the fetch is
> not guaranteed to be atomic, is that correct? Because the address may
> not be naturally aligned; or the cpu may not be x86 and thus may not
> provide automatic atomicity in such case.

Yes, this depends on the architecture. That's why C11/C++11 

Re: __{read,write}_once

2019-11-06 Thread Mouse
>   - Is 'volatile' a guarantee that the compiler will emit only one
> instruction to fetch the value?

No.  For some types, some compilers, and some architectures, it may be,
but it is implementation-dependent.  (For some types, on some
architectures, it cannot be; for example, consider a 64-bit volatile
value on an architecture without 64-bit memory access primitives.)

What volatile is designed for is to ensure that the generated code
actually accesses the value.  Consider

count = 0;
while (count < 10) {
if (*register & 0x0400) count ++;
}

If register is "uint32_t *", the compiler is permitted to - and some
will - hoist the load, effectively turning this into

count = 0;
temp = *register;
while (count < 10) {
if (temp & 0x0400) count ++;
}

which is unlikely to do what you want.  If register is instead declared
"volatile uint32_t *", then *register is volatile and thus must be
accessed each time through the loop.

volatile also compels such caching in some circumstances; for exmaple,
if the code is

uint32_t *register;
uint32_t v;
v = *register;
call((v+4)*v);

then, under certain circumstances (for example, v is auto and its
address is never taken) the compiler is permitted to turn that into

call(((*register)+4) * *register)

and, on some architectures, that would be a win.  (It's less likely to
happen than the other way around, but, if addressing modes are cheap,
registers are few, and memory is fast compared to CPU cycles, the
compiler may do that.)  But make it "volatile uint32_t *register;" and
such an optimization is not permitted.

>   - Assuming there is only one instruction, strictly speaking the
> fetch is not guaranteed to be atomic, is that correct?

Right.  What is or isn't atomic is architecture-dependent; there is, I
think, _nothing_ you can do in C that can guarantee atomicity (or, for
that matter, guarantee non-atomicity), since atomicity is fundamentally
an architecture-dependent notion.

/~\ The ASCII Mouse
\ / Ribbon Campaign
 X  Against HTMLmo...@rodents-montreal.org
/ \ Email!   7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B


Re: __{read,write}_once

2019-11-06 Thread Marco Elver
On Wed, 6 Nov 2019 at 16:51, Kamil Rytarowski  wrote:
>
> On 06.11.2019 16:44, Kamil Rytarowski wrote:
> > On 06.11.2019 15:57, Jason Thorpe wrote:
> >>
> >>
> >>> On Nov 6, 2019, at 5:41 AM, Kamil Rytarowski  wrote:
> >>>
> >>> On 06.11.2019 14:37, Jason Thorpe wrote:
> 
> 
> > On Nov 6, 2019, at 4:45 AM, Kamil Rytarowski  wrote:
> >
> > I propose __write_relaxed() / __read_relaxed().
> 
>  ...except that seems to imply the opposite of what these do.
> 
>  -- thorpej
> 
> >>>
> >>> Rationale?
> >>>
> >>> This matches atomic_load_relaxed() / atomic_write_relaxed(), but we do
> >>> not deal with atomics here.
> >>
> >> Fair enough.  To me, the names suggest "compiler is allowed to apply 
> >> relaxed constraints and tear the access if it wants" But apparently 
> >> the common meaning is "relax, bro, I know what I'm doing".  If that's the 
> >> case, I can roll with it.

See below.

> >> -- thorpej
> >>
> >
> > Unless I mean something this is exactly about relaxed constraints.
>
> miss*
>
> >
> > "Relaxed operation: there are no synchronization or ordering constraints
> > imposed on other reads or writes" and without "operation's atomicity is
> > guaranteed".

In the memory consistency model world, "relaxed" has a very specific
meaning for loads/stores. It simply means that the compiler and
architecture is free to reorder the memory operation with respect to
previous or following memory operations in program order. But the
load/store still happens as one atomic operation.

For programming-language memory models, "relaxed" appears in the
context of atomic memory operations, to define their ordering w.r.t.
other operations in program order. There is a distinction between
"relaxed atomic" and plain (unannotated) memory operations at the
programming language level.

For plain operations, the compiler, in addition to the target
architecture, is free to reorder operations or even apply
optimizations that turn single plain operations into multiple
loads/stores [1, 2].
[1] https://lwn.net/Articles/793253/
[2] https://lwn.net/Articles/799218/

In the Linux kernel READ_ONCE/WRITE_ONCE are one way to specify
"relaxed atomic read/write" for accesses that fit in a word (_ONCE
also works on things larger than a word, but probably aren't atomic
anymore). For most architectures the implementation is the same, and
merely avoids compiler optimizations; the exception is Alpha, where
READ_ONCE adds a memory barrier [3].
[3] https://www.kernel.org/doc/Documentation/memory-barriers.txt

> > This is also similar to what suggested Google to apply to NetBSD in our
> > internal thread, but with a bit different naming.

What you call the ops is entirely up to you. I would not use
'{read,write}_relaxed', since as you pointed out above, is probably
more confusing. This is why I suggested 'atomic_{read,write}_relaxed'.
If you do not like the 'atomic' in there, the next best thing is
probably '{read,write}_once'.

Just note that you're now defining your own memory consistency model.
This is a complex topic that spans decades of work. The safest thing
is to stick as closely to the C11/C++11 memory model as possible [4].
The Linux kernel also has a memory model [5], but is significantly
more complex since it was defined after various primitives had been
introduced.
[4] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1479.htm
[5] 
https://github.com/torvalds/linux/blob/master/tools/memory-model/Documentation/explanation.txt

Best Wishes,
-- Marco

> Adding Marco to this thread.
>



Re: __{read,write}_once

2019-11-06 Thread Kamil Rytarowski
On 06.11.2019 16:58, David Young wrote:
> I *think* the intention is for __read_once()/__write_once() to
> load/store the entire variable from/to memory precisely once.  They
> provide no guarantees about atomicity of the load/store.  Should
> something be said about ordering and visibility of stores?

The original intention is to mark reads and writes racy-OK.

once is a bad name as it suggests some variation of RUN_ONCE(). I am for
memory ordering name relaxed, borrowed from the terminology atomics.



signature.asc
Description: OpenPGP digital signature


Re: __{read,write}_once

2019-11-06 Thread Andrew Cagney
On Wed, 6 Nov 2019 at 09:57, Jason Thorpe  wrote:
>
>
>
> > On Nov 6, 2019, at 5:41 AM, Kamil Rytarowski  wrote:
> >
> > On 06.11.2019 14:37, Jason Thorpe wrote:
> >>
> >>
> >>> On Nov 6, 2019, at 4:45 AM, Kamil Rytarowski  wrote:
> >>>
> >>> I propose __write_relaxed() / __read_relaxed().
> >>
> >> ...except that seems to imply the opposite of what these do.
> >>
> >> -- thorpej
> >>
> >
> > Rationale?
> >
> > This matches atomic_load_relaxed() / atomic_write_relaxed(), but we do
> > not deal with atomics here.
>
> Fair enough.  To me, the names suggest "compiler is allowed to apply relaxed 
> constraints and tear the access if it wants" But apparently the common 
> meaning is "relax, bro, I know what I'm doing".  If that's the case, I can 
> roll with it.

Honestly, without reading any code, I interpretation is more inline
with the former:
  hey relax, maybe it happens, maybe it doesn't but well, nothing
matters so what ever


Re: __{read,write}_once

2019-11-06 Thread David Young
On Wed, Nov 06, 2019 at 06:57:07AM -0800, Jason Thorpe wrote:
> 
> 
> > On Nov 6, 2019, at 5:41 AM, Kamil Rytarowski  wrote:
> > 
> > On 06.11.2019 14:37, Jason Thorpe wrote:
> >> 
> >> 
> >>> On Nov 6, 2019, at 4:45 AM, Kamil Rytarowski  wrote:
> >>> 
> >>> I propose __write_relaxed() / __read_relaxed().
> >> 
> >> ...except that seems to imply the opposite of what these do.
> >> 
> >> -- thorpej
> >> 
> > 
> > Rationale?
> > 
> > This matches atomic_load_relaxed() / atomic_write_relaxed(), but we do
> > not deal with atomics here.
> 
> Fair enough.  To me, the names suggest "compiler is allowed to apply relaxed 
> constraints and tear the access if it wants" But apparently the common 
> meaning is "relax, bro, I know what I'm doing".  If that's the case, I can 
> roll with it.

After reading this conversation, I'm not sure of the semantics.

I *think* the intention is for __read_once()/__write_once() to
load/store the entire variable from/to memory precisely once.  They
provide no guarantees about atomicity of the load/store.  Should
something be said about ordering and visibility of stores?

If x is initialized to 0xf00dd00f, two threads start, and thread
1 performs __read_once(x) concurrently with thread 2 performing
__write_once(x, 0xfeedbeef), then what values can thread 1 read?

Do __read_once()/__write_once() have any semantics with respect to
interrupts?

Dave

-- 
David Young
dyo...@pobox.comUrbana, IL(217) 721-9981


Re: __{read,write}_once

2019-11-06 Thread Kamil Rytarowski
On 06.11.2019 16:44, Kamil Rytarowski wrote:
> On 06.11.2019 15:57, Jason Thorpe wrote:
>>
>>
>>> On Nov 6, 2019, at 5:41 AM, Kamil Rytarowski  wrote:
>>>
>>> On 06.11.2019 14:37, Jason Thorpe wrote:


> On Nov 6, 2019, at 4:45 AM, Kamil Rytarowski  wrote:
>
> I propose __write_relaxed() / __read_relaxed().

 ...except that seems to imply the opposite of what these do.

 -- thorpej

>>>
>>> Rationale?
>>>
>>> This matches atomic_load_relaxed() / atomic_write_relaxed(), but we do
>>> not deal with atomics here.
>>
>> Fair enough.  To me, the names suggest "compiler is allowed to apply relaxed 
>> constraints and tear the access if it wants" But apparently the common 
>> meaning is "relax, bro, I know what I'm doing".  If that's the case, I can 
>> roll with it.
>>
>> -- thorpej
>>
> 
> Unless I mean something this is exactly about relaxed constraints.

miss*

> 
> "Relaxed operation: there are no synchronization or ordering constraints
> imposed on other reads or writes" and without "operation's atomicity is
> guaranteed".
> 
> This is also similar to what suggested Google to apply to NetBSD in our
> internal thread, but with a bit different naming.
> 

Adding Marco to this thread.



signature.asc
Description: OpenPGP digital signature


Re: __{read,write}_once

2019-11-06 Thread Kamil Rytarowski
On 06.11.2019 15:57, Jason Thorpe wrote:
> 
> 
>> On Nov 6, 2019, at 5:41 AM, Kamil Rytarowski  wrote:
>>
>> On 06.11.2019 14:37, Jason Thorpe wrote:
>>>
>>>
 On Nov 6, 2019, at 4:45 AM, Kamil Rytarowski  wrote:

 I propose __write_relaxed() / __read_relaxed().
>>>
>>> ...except that seems to imply the opposite of what these do.
>>>
>>> -- thorpej
>>>
>>
>> Rationale?
>>
>> This matches atomic_load_relaxed() / atomic_write_relaxed(), but we do
>> not deal with atomics here.
> 
> Fair enough.  To me, the names suggest "compiler is allowed to apply relaxed 
> constraints and tear the access if it wants" But apparently the common 
> meaning is "relax, bro, I know what I'm doing".  If that's the case, I can 
> roll with it.
> 
> -- thorpej
> 

Unless I mean something this is exactly about relaxed constraints.

"Relaxed operation: there are no synchronization or ordering constraints
imposed on other reads or writes" and without "operation's atomicity is
guaranteed".

This is also similar to what suggested Google to apply to NetBSD in our
internal thread, but with a bit different naming.



signature.asc
Description: OpenPGP digital signature


Re: __{read,write}_once

2019-11-06 Thread Martin Husemann
On Wed, Nov 06, 2019 at 06:57:07AM -0800, Jason Thorpe wrote:
> > This matches atomic_load_relaxed() / atomic_write_relaxed(), but we do
> > not deal with atomics here.
> 
> Fair enough.  To me, the names suggest "compiler is allowed to apply
> relaxed constraints and tear the access if it wants" But apparently
> the common meaning is "relax, bro, I know what I'm doing".  If that's
> the case, I can roll with it.

What is the compiler / implementation supposed to do if there is no way
to do a single instruction load/store for the argument size?
Do we want sized versions instead and only provide the ones that are
available on a given architecture?

If both names and semantics are unclear, adding the macros maybe not a
good idea ;-)

Martin


Re: __{read,write}_once

2019-11-06 Thread Jason Thorpe



> On Nov 6, 2019, at 5:41 AM, Kamil Rytarowski  wrote:
> 
> On 06.11.2019 14:37, Jason Thorpe wrote:
>> 
>> 
>>> On Nov 6, 2019, at 4:45 AM, Kamil Rytarowski  wrote:
>>> 
>>> I propose __write_relaxed() / __read_relaxed().
>> 
>> ...except that seems to imply the opposite of what these do.
>> 
>> -- thorpej
>> 
> 
> Rationale?
> 
> This matches atomic_load_relaxed() / atomic_write_relaxed(), but we do
> not deal with atomics here.

Fair enough.  To me, the names suggest "compiler is allowed to apply relaxed 
constraints and tear the access if it wants" But apparently the common 
meaning is "relax, bro, I know what I'm doing".  If that's the case, I can roll 
with it.

-- thorpej



Re: __{read,write}_once

2019-11-06 Thread Jason Thorpe



> On Nov 6, 2019, at 4:45 AM, Kamil Rytarowski  wrote:
> 
> I propose __write_relaxed() / __read_relaxed().

...except that seems to imply the opposite of what these do.

-- thorpej



Re: __{read,write}_once

2019-11-06 Thread Kamil Rytarowski
On 06.11.2019 14:37, Jason Thorpe wrote:
> 
> 
>> On Nov 6, 2019, at 4:45 AM, Kamil Rytarowski  wrote:
>>
>> I propose __write_relaxed() / __read_relaxed().
> 
> ...except that seems to imply the opposite of what these do.
> 
> -- thorpej
> 

Rationale?

This matches atomic_load_relaxed() / atomic_write_relaxed(), but we do
not deal with atomics here.



signature.asc
Description: OpenPGP digital signature


Re: __{read,write}_once

2019-11-06 Thread Kamil Rytarowski
On 06.11.2019 12:51, Maxime Villard wrote:
> Le 06/11/2019 à 12:38, Martin Husemann a écrit :
>> On Wed, Nov 06, 2019 at 12:31:37PM +0100, Maxime Villard wrote:
>>> __read_once(x)
>>> __write_once(x, val)
>>
>> The names are really not helpfull for understanding what is supposed
>> to happen
>> here.
> 
> I don't know if you have a better suggestion, but it's basically the naming
> convention that seems to have been used for several years already. Maybe we
> could have something more explicit, like
> 
> __{read,write}_racy() or
> __{read,write}_parallel() or
> __{read,write}_concurrent()
> 
> But the last one is too long I think.

I propose __write_relaxed() / __read_relaxed().



signature.asc
Description: OpenPGP digital signature


Re: __{read,write}_once

2019-11-06 Thread Maxime Villard

Le 06/11/2019 à 12:38, Martin Husemann a écrit :

On Wed, Nov 06, 2019 at 12:31:37PM +0100, Maxime Villard wrote:

__read_once(x)
__write_once(x, val)


The names are really not helpfull for understanding what is supposed to happen
here.


I don't know if you have a better suggestion, but it's basically the naming
convention that seems to have been used for several years already. Maybe we
could have something more explicit, like

__{read,write}_racy() or
__{read,write}_parallel() or
__{read,write}_concurrent()

But the last one is too long I think.


Re: __{read,write}_once

2019-11-06 Thread Martin Husemann
On Wed, Nov 06, 2019 at 12:31:37PM +0100, Maxime Villard wrote:
>   __read_once(x)
>   __write_once(x, val)

The names are really not helpfull for understanding what is supposed to happen
here.

Martin


Re: adding linux syscall fallocate

2019-11-06 Thread r0ller
Hi Maciej,

Thanks for the detailed answer! Unfortunately, I don't think that I could 
accomplish this task in my spare time:(

Please, don't take this as an offence but as a fix for my case I thought of an 
utter hack:

I do know that writing a file by calling fallocate can be tricked and 
redirected into a named pipe of which the data can simply be pulled into a 
normal file. This is what I'm already doing in my project as a workaround when 
building it as 32bit arm lib:

mkfifo mypipe
cat mypipe > myfile


The problem with this is that it cannot be used when crosscompiling autoconf 
projects where a configure script starts creating many files as I'd need to 
edit the script at too many places to implement this trick.

However, if I could carry out this trick with the pipe when intercepting the 
linux fallocate call, it could work. Do you think it feasible?

Best regards,
r0ller

 Eredeti levél 
Feladó: Maciej < maciej.grochow...@protonmail.com (Link -> 
mailto:maciej.grochow...@protonmail.com) >
Dátum: 2019 november 4 23:32:56
Tárgy: Re: adding linux syscall fallocate
Címzett: r0ller < r0l...@freemail.hu (Link -> mailto:r0l...@freemail.hu) >
 
Hi r0ller,
 
A couple of weeks ago I also run to the issue when I found lack of fallocate or 
POSIX_FALLOCATE(2) (to be precise) a little bit sad.
>From the very first view typical usage of POSIX_FALLOCATE(2) seems straight 
>forward, comparing to the Linux fallocate(2) where different modes have to be 
>supported. However, things can go a little bit more complicated if you are 
>dealing with an existing file with a more complex structure.
Because of that, I found difficult to provide good quality implementation 
without a proper way to test it.
Before EuroBSD 2019 as a part of work about fuzzing the FFS, I decided to bring 
some well known FS test (namely speaking "XFS tests') suit to make sure that 
bugs that we fix did not introduce a regression.
The same thing applies to the new features of FS, is relatively easy to port 
implementation from other OS/FS, but without a proper way to test them, I would 
be very careful to introduce such things too quickly to the end-users.
 
One thing that I was missing for XFS tests, and going to publish part of it 
soon, is a way to view the internal structure of inodes and other metadata of 
Filesystem. My primary use case was to debug mount issues, in the example the 
issue that I showed during my presentation about the fuzzing. But also same 
apply to the code that manipulates inode blocks.
 
Hopefully, we can elaborate on that, and as I pointed earlier I would be 
careful with merging new FS features especially such wide used as 
POSIX_FALLOCATE(2) without a proper FS test suit or extensive enough testing 
that would require proper too like i.e. FSDB.
 
Thanks
Maciej
 
‐‐‐ Original Message ‐‐‐
On Sunday, November 3, 2019 6:06 PM, r0ller  wrote:
 
Hi Jaromir,
 
Indeed. That's bad news but thanks for your answer! I've even found this: 
https://wiki.netbsd.org/projects/project/ffs-fallocate/
Are there any details for this project besides that page? I don't know anything 
about NetBSD internals though if it's not meant for gurus, I'd have a look at 
it and give it a try.
 
Best regards,
r0ller
 
 Eredeti levél 
Feladó: Jaromír Doleček < jaromir.dole...@gmail.com (Link -> 
mailto:jaromir.dole...@gmail.com) >
Dátum: 2019 november 3 15:16:34
Tárgy: Re: adding linux syscall fallocate
Címzett: r0ller < r0l...@freemail.hu (Link -> mailto:r0l...@freemail.hu) >
Le dim. 3 nov. 2019 à 08:57, r0ller  a écrit :
 
> As you can see on the attached screenshot, "line 4741" gets printed out. So I 
> went on to check what happens in VOP_FALLOCATE but it gets really internal 
> there.
>
> Does anyone have any hint?
 
fallocate VOP is not implemented for FFS:
 
> grep fallocate *
ffs_vnops.c: { _fallocate_desc, genfs_eopnotsupp }, /* fallocate */
ffs_vnops.c: { _fallocate_desc, spec_fallocate }, /* fallocate */
ffs_vnops.c: { _fallocate_desc, vn_fifo_bypass }, /* fallocate */
 
Jaromir