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

[Björn Brandenburg]

> 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… 

Thanks,
Björn