[LITMUS^RT] LITMUS^RT on v4.9: call for testing

[Andrea Bastoni]

On Wed, Mar 8, 2017 at 9:08 AM, Björn Brandenburg <bbb at mpi-sws.org> wrote:
>
>> On 8 Mar 2017, at 09:02, Björn Brandenburg <bbb at mpi-sws.org> wrote:
>>
>>
>>> On 6 Mar 2017, at 18:06, Namhoon Kim <namhoonk at cs.unc.edu> wrote:
>>>
>>> Thanks for the patch! The patch fixes the compilation error. I ran ./runtests with PFAIR, P-FP, LINUX, GSN-EDF, and PSN-EDF. All tests are passed except SRP tests. I will do more tests such as running randomly generated tasksets or launching linux tasks with rt_launch.
>>>
>>
>> Hi Namhoon,
>>
>> I just had a look at Andrea’s compile fix. I think the use of atomic_add_return_relaxed() needs some more thought. It’s used to realize fetch_and_inc(), which is used in two places: once to get a sequence number (in litmus/trace.c), which I think is unproblematic, and once to mark a Feather-Buffer slot as free (in litmus/ftdev.c):
>>
>> static int ft_buffer_copy_to_user(struct ft_buffer* buf, char __user *dest)
>> {
>>       unsigned int idx;
>>       int err = 0;
>>       if (buf->free_count != buf->slot_count) {
>>               /* data available */
>>               idx = buf->read_idx % buf->slot_count;
>>               if (buf->slots[idx] == SLOT_READY) {
>>                       err = copy_to_user(dest, ((char*) buf->buffer_mem) +
>>                                          idx * buf->slot_size,
>>                                          buf->slot_size);
>>                       if (err == 0) {
>>                               /* copy ok */
>> (1) ->                                buf->slots[idx] = SLOT_FREE;
>>                               buf->read_idx++;
>> (2) —>                                fetch_and_inc(&buf->free_count);
>>                               err = 1;
>>                       }
>>               }
>>       }
>>       return err;
>> }
>>
>> Here, I think we probably need _release semantics (all prior stores visible) as otherwise  we might make a buffer available to another core @ (2) *before* the write of SLOT_FREE to buf->slots[idx] @ (1) has taken effect. It’s a small race window (the buffer needs to be completely full), but it’s incorrect nonetheless.
>>
>> However, we also actually don’t need atomic_add_return(); we only need atomic_inc(). But on ARM, atomic_inc() is just atomic_add().
>>
>>       http://lxr.free-electrons.com/source/arch/arm/include/asm/atomic.h?v=4.9#L257
>>
>> And ARM doesn’t seem to define atomic_add():
>>
>>       http://lxr.free-electrons.com/ident?v=4.9;i=atomic_add
>>
>> So it falls back to the default definition, which is atomic_add_return(); exactly where we started.
>>
>>       http://lxr.free-electrons.com/source/include/asm-generic/atomic.h?v=4.9#L196
>>
>> There are tons of users of atomic_inc() in kernel/ that compile just fine on ARM. So if all uses of atomic_inc() map to atomic_add_return() on ARM, why can’t we use atomic_add_return() in our code? What am I missing here?
>>
>> Bottomline: I don’t understand the problem, and if we really can’t keep using atomic_add_return() on ARM, I’m not convinced that just using atomic_add_return_relaxed() instead is right here.
>>
>> Namhoon, can you please investigate?
>
> By the way, while we are looking at this, one option would be to just remove the fetch_and_inc() wrapper (which comes from the use of Feather-Trace before inclusion in LITMUS^RT/Linux) and to switch all of Feather-Trace to use proper atomic_t variables and the regular accessor functions. Patches welcome…

There was an interesting article some time ago about the recent
changes in the atomic primitives in the linux kernel:
https://lwn.net/Articles/695257/

BR,
Andrea

>
> Thanks,
> Björn
>
>
>
> _______________________________________________
> litmus-dev mailing list
> litmus-dev at lists.litmus-rt.org
> https://lists.litmus-rt.org/listinfo/litmus-dev