lkl: turn on CONFIG_EVENTFD

[xjia1]

Signed-off-by: Xiao Jia [email protected]

[thehajime]

it would be good to do like same as other hijacked calls to handle host fds (below is the example of setsockopt():

    CHECK_HOST_CALL(getsockopt);
    if (!is_lklfd(fd))
        return host_setsockopt(fd, level, optname, optval, optlen);

[xjia1]

What's the purpose of CHECK_HOST_CALL? Isn't the host_xxx already initialized in init2_host_##name?

[pscollins]

I was going to make a PR for this, but I think that either the __attribute__((constructor))s or the CHECK_HOST_CALLs should be removed --- preferably the former because afaik constructor functions are run once per thread rather than once per process (so spawning a new worker thread in hijacked code will trigger a wave of dlsyms).

[xjia1]

What about concurrent executions of CHECK_HOST_CALLs?

[pscollins]

@stfairy That should be safe, right? Worst comes to worst, two threads try to dlsym it at once and it gets set twice, but the setting is atomic so it doesn't break anything.

[xjia1]

dlsym may not be atomic. It is MT safe according to POSIX, but that doesn't provide much guarantee: http://man7.org/linux/man-pages/man7/attributes.7.html

[xjia1]

btw setting a non-atomic variable is also not guaranteed to be atomic on some architectures.

From my POV the CHECK_HOST_CALL should be changed to something like:

if (!acquire_read(host_calls...)) {
lock; local_var = dlsym...; unlock;
release_write(host_calls..., local_var);
}

[pscollins]

So MT safe != reentrant? I figured dlsym was thread safe and then setting a pointer to the result of dlsym would be one movl (i.e. a word-sized write), I'm surprised that there are architectures that don't guarantee that to be atomic.

Depending on what we believe to be atomic I think we can get away with less locking, anyway, something like:

if(!host_foo) {
   acquire(host_foo_lock);
       if(!host_foo) {
         local_foo = dlsym('foo');
         host_foo = local_foo;
      }
  release(host_foo_lock);
}

[pscollins]

Oh, but regardless --- eventfd will never need to touch the host, since it creates a new fd; there's no way for the user to ask it to hit the host (except perhaps by adding our own flag). This is a useful discussion but it doesn't seem to apply here.

[thehajime]

@pscollins you're right. eventfd will never touch the host.

epoll_create also has the same issue: no way to distinguish the destination without additional information. I currently have no idea how to deal with it.

This is a useful discussion but it doesn't seem to apply here.

right . my bad.
my comment only applies to eventfd_read and wirte.

[thehajime]

@stfairy @pscollins __attribute__((constructor)) is somehow useless.

we need to take care for applications who have own constructor calls hijacked funcs of lkl.
see related discussion here.

4c57f95#diff-6754865b3e4a1cc9d24ba88bd5fddfb1R465

[ghost]

@stfairy can you please add the checks @thehajime mentioned, so I can merge this?

We can do the constructor cleanup in a separate patch.

[xjia1]

Pushed a new version. Please take a look.