Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lkl: let atomic ops outsourced #273

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions arch/lkl/include/uapi/asm/host_ops.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,13 @@ int lkl_is_running(void);
int lkl_printf(const char *, ...);
void lkl_bug(const char *, ...);

/* atomic ops */
int lkl__sync_fetch_and_sub(int *ptr, int value);
int lkl__sync_fetch_and_add(int *ptr, int value);
long lkl__sync_fetch_and_or(long *ptr, long value);
long lkl__sync_fetch_and_and(long *ptr, long value);
void lkl__sync_synchronize(void);
void atomic_ops_init(void);
void atomic_ops_cleanup(void);

#endif
2 changes: 1 addition & 1 deletion arch/lkl/kernel/Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
extra-y := vmlinux.lds

obj-y = setup.o threads.o irq.o time.o syscalls.o misc.o console.o \
syscalls_32.o cpu.o
syscalls_32.o cpu.o atomic.o
97 changes: 97 additions & 0 deletions arch/lkl/kernel/atomic.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#include <linux/kernel.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we move this to tools/lkl/lib?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree. I actually wanted to do it in toolks/lkl/lib but was reluctant to add new lkl_host_ops entries for various atomic ops. do you think it's nicer to do int host_ops or have better idea to do it in tools/lkl ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any nicer way to do it and we will need them anyway for SMP support. We may need a different set for SMP, but we can change that later.

#include <linux/init.h>
#include <asm/host_ops.h>

#if defined(__ARMEL__)
static void *atomic_lock;

long lkl__sync_fetch_and_or(long *ptr, long value)
{
lkl_ops->sem_down(atomic_lock);
*ptr = value;
lkl_ops->sem_up(atomic_lock);
return 0;
}

long lkl__sync_fetch_and_and(long *ptr, long value)
{
int tmp;

lkl_ops->sem_down(atomic_lock);
tmp = *ptr;
*ptr *= value;
lkl_ops->sem_up(atomic_lock);
return tmp;
}

int lkl__sync_fetch_and_add(int *ptr, int value)
{
int tmp;

lkl_ops->sem_down(atomic_lock);
tmp = *ptr;
*ptr += value;
lkl_ops->sem_up(atomic_lock);
return tmp;
}

int lkl__sync_fetch_and_sub(int *ptr, int value)
{
int tmp;

lkl_ops->sem_down(atomic_lock);
tmp = *ptr;
*ptr -= value;
lkl_ops->sem_up(atomic_lock);
return tmp;
}

void lkl__sync_synchronize(void)
{
}

void atomic_ops_init(void)
{
atomic_lock = lkl_ops->sem_alloc(1);
}

void atomic_ops_cleanup(void)
{
lkl_ops->sem_free(atomic_lock);
}

#else
long lkl__sync_fetch_and_or(long *ptr, long value)
{
return __sync_fetch_and_or(ptr, value);
}

long lkl__sync_fetch_and_and(long *ptr, long value)
{
return __sync_fetch_and_and(ptr, value);
}

int lkl__sync_fetch_and_add(int *ptr, int value)
{
return __sync_fetch_and_add(ptr, value);
}

int lkl__sync_fetch_and_sub(int *ptr, int value)
{
return __sync_fetch_and_sub(ptr, value);
}

void lkl__sync_synchronize(void)
{
return __sync_synchronize();
}

void atomic_ops_init(void)
{
}

void atomic_ops_cleanup(void)
{
}
#endif

8 changes: 4 additions & 4 deletions arch/lkl/kernel/cpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ static int __cpu_try_get_lock(int n)
{
lkl_thread_t self;

if (__sync_fetch_and_add(&cpu.shutdown_gate, n) >= MAX_THREADS)
if (lkl__sync_fetch_and_add(&cpu.shutdown_gate, n) >= MAX_THREADS)
return -2;

lkl_ops->mutex_lock(cpu.lock);
Expand All @@ -89,7 +89,7 @@ static void __cpu_try_get_unlock(int lock_ret, int n)
{
if (lock_ret >= -1)
lkl_ops->mutex_unlock(cpu.lock);
__sync_fetch_and_sub(&cpu.shutdown_gate, n);
lkl__sync_fetch_and_sub(&cpu.shutdown_gate, n);
}

void lkl_cpu_change_owner(lkl_thread_t owner)
Expand Down Expand Up @@ -173,7 +173,7 @@ int lkl_cpu_try_run_irq(int irq)

void lkl_cpu_shutdown(void)
{
__sync_fetch_and_add(&cpu.shutdown_gate, MAX_THREADS);
lkl__sync_fetch_and_add(&cpu.shutdown_gate, MAX_THREADS);
}

void lkl_cpu_wait_shutdown(void)
Expand All @@ -184,7 +184,7 @@ void lkl_cpu_wait_shutdown(void)

static void lkl_cpu_cleanup(bool shutdown)
{
while (__sync_fetch_and_add(&cpu.shutdown_gate, 0) > MAX_THREADS)
while (lkl__sync_fetch_and_add(&cpu.shutdown_gate, 0) > MAX_THREADS)
;

if (shutdown)
Expand Down
8 changes: 4 additions & 4 deletions arch/lkl/kernel/irq.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,23 @@ static inline unsigned long test_and_clear_irq_index_status(void)
{
if (!irq_index_status)
return 0;
return __sync_fetch_and_and(&irq_index_status, 0);
return lkl__sync_fetch_and_and(&irq_index_status, 0);
}

static inline unsigned long test_and_clear_irq_status(int index)
{
if (!&irq_status[index])
return 0;
return __sync_fetch_and_and(&irq_status[index], 0);
return lkl__sync_fetch_and_and(&irq_status[index], 0);
}

void set_irq_pending(int irq)
{
int index = irq / IRQ_STATUS_BITS;
int bit = irq % IRQ_STATUS_BITS;

__sync_fetch_and_or(&irq_status[index], BIT(bit));
__sync_fetch_and_or(&irq_index_status, BIT(index));
lkl__sync_fetch_and_or(&irq_status[index], BIT(bit));
lkl__sync_fetch_and_or(&irq_index_status, BIT(index));
}

static struct irq_info {
Expand Down
2 changes: 2 additions & 0 deletions arch/lkl/kernel/setup.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ void __init setup_arch(char **cl)

static void __init lkl_run_kernel(void *arg)
{
atomic_ops_init();
threads_init();
lkl_cpu_get();
start_kernel();
Expand Down Expand Up @@ -126,6 +127,7 @@ long lkl_sys_halt(void)

syscalls_cleanup();
threads_cleanup();
atomic_ops_cleanup();
/* Shutdown the clockevents source. */
tick_suspend_local();
free_mem();
Expand Down
8 changes: 4 additions & 4 deletions arch/lkl/kernel/threads.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include <asm/cpu.h>
#include <asm/sched.h>

static volatile int threads_counter;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange that no "threads_counter" is defined in my version ( based on a063e16 - Merge pull request #340 from libos-nuse/fix-checkpath-tak2).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR is a bit outdated: need to update/rebase in advance to apply the latest lkl.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added the code changes to the latest release, so we can see if the patch works for the ARM 32bit platforms. Will let you know if it works or not.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thehajime The patch doesn't work for me right now.
After changing to lkl__sync_ functions, the __sync_synchronize used in lib/virtio.c becomes undefined once again. Then, I tried to change it to lkl__sync_synchronize and compile, what I got is that there're two lkl__sync_synchronize symbols in liblkl-hijack.so:

00061454 l     F .text	00000044              lkl__sync_synchronize
00000000         *UND*	00000000              lkl__sync_synchronize

The undefined symbol comes from lib/lkl-in.o, the defined one comes from lib/lkl.o. When running AR to generate liblkl.a based on lkl-in.o and lkl.o, the two symbols are joint together.

What should I do to get rid of it? I am not professional on GCC toolchain....

static int threads_counter;

static int init_ti(struct thread_info *ti)
{
Expand Down Expand Up @@ -123,7 +123,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
}

if (_prev->dead) {
__sync_fetch_and_sub(&threads_counter, 1);
lkl__sync_fetch_and_sub(&threads_counter, 1);
lkl_ops->thread_exit();
}

Expand Down Expand Up @@ -193,7 +193,7 @@ int copy_thread(unsigned long clone_flags, unsigned long esp,
return -ENOMEM;
}

__sync_fetch_and_add(&threads_counter, 1);
lkl__sync_fetch_and_add(&threads_counter, 1);

return 0;
}
Expand All @@ -220,7 +220,7 @@ void threads_init(void)

void threads_cnt_dec(void)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No such one function is defined and used in the latest code base (a063e16).

{
__sync_fetch_and_sub(&threads_counter, 1);
lkl__sync_fetch_and_sub(&threads_counter, 1);
}

void threads_cleanup(void)
Expand Down