From 1b0403703465de3ed3a36b4c4d96bafa4aa7498a Mon Sep 17 00:00:00 2001 From: Bahadir Balban Date: Mon, 11 May 2009 12:10:12 +0300 Subject: [PATCH] Address space creation/deletion implemented - Proper releasing of user pmd and pgds when a space is not used. - Proper releasing of task, space ids. - At occasions a starting thread gets bogus SPSR, this needs investigating. - At a very rare occasion arch_setup_new_thread() had a kernel data abort during register copying from one task to another. Needs investigating. --- include/l4/arch/arm/v5/mm.h | 1 + include/l4/generic/tcb.h | 4 +- src/api/syscall.c | 2 +- src/api/thread.c | 88 ++++++++++++++++++------------------- src/arch/arm/v5/mm.c | 6 +++ src/generic/space.c | 24 +++++++--- src/generic/tcb.c | 25 ++++++++++- 7 files changed, 95 insertions(+), 55 deletions(-) diff --git a/include/l4/arch/arm/v5/mm.h b/include/l4/arch/arm/v5/mm.h index 80f2994..b7e948c 100644 --- a/include/l4/arch/arm/v5/mm.h +++ b/include/l4/arch/arm/v5/mm.h @@ -138,6 +138,7 @@ void add_section_mapping_init(unsigned int paddr, unsigned int vaddr, unsigned int size, unsigned int flags); struct address_space; +int delete_page_tables(struct address_space *space); int copy_user_tables(struct address_space *new, struct address_space *orig); pgd_table_t *copy_page_tables(pgd_table_t *from); void remap_as_pages(void *vstart, void *vend); diff --git a/include/l4/generic/tcb.h b/include/l4/generic/tcb.h index 427911a..3f62322 100644 --- a/include/l4/generic/tcb.h +++ b/include/l4/generic/tcb.h @@ -56,8 +56,8 @@ struct ktcb { /* Thread information */ l4id_t tid; /* Global thread id */ - l4id_t spid; /* Global space id */ l4id_t tgid; /* Global thread group id */ + /* See space for space id */ /* Flags to indicate various task status */ unsigned int flags; @@ -120,7 +120,6 @@ union ktcb_union { static inline void set_task_ids(struct ktcb *task, struct task_ids *ids) { task->tid = ids->tid; - task->spid = ids->spid; task->tgid = ids->tgid; } @@ -134,6 +133,7 @@ extern struct id_pool *space_id_pool; extern struct id_pool *tgroup_id_pool; struct ktcb *tcb_find(l4id_t tid); +struct ktcb *tcb_find_by_space(l4id_t tid); void tcb_add(struct ktcb *tcb); void tcb_remove(struct ktcb *tcb); diff --git a/src/api/syscall.c b/src/api/syscall.c index 2412f12..2bb05d6 100644 --- a/src/api/syscall.c +++ b/src/api/syscall.c @@ -167,7 +167,7 @@ int sys_getid(syscall_context_t *regs) struct ktcb *this = current; ids->tid = this->tid; - ids->spid = this->spid; + ids->spid = this->space->spid; ids->tgid = this->tgid; return 0; diff --git a/src/api/thread.c b/src/api/thread.c index 4367753..5d38411 100644 --- a/src/api/thread.c +++ b/src/api/thread.c @@ -172,7 +172,7 @@ int arch_setup_new_thread(struct ktcb *new, struct ktcb *orig, * We don't lock for context modification because the * thread is not known to the system yet. */ - new->context.spsr = orig->syscall_regs->spsr; /* User mode */ + BUG_ON(!(new->context.spsr = orig->syscall_regs->spsr)); /* User mode */ new->context.r0 = orig->syscall_regs->r0; new->context.r1 = orig->syscall_regs->r1; new->context.r2 = orig->syscall_regs->r2; @@ -213,32 +213,20 @@ int thread_setup_new_ids(struct task_ids *ids, unsigned int flags, /* * If thread space is new or copied, - * allocate a new space id and tgid + * thread gets same group id as its thread id */ if (flags == THREAD_NEW_SPACE || - flags == THREAD_COPY_SPACE) { - /* - * Allocate requested id if - * it's available, else a new one - */ - if ((ids->spid = id_get(space_id_pool, - ids->spid)) < 0) - ids->spid = id_new(space_id_pool); - - /* Thread gets same group id as its thread id */ + flags == THREAD_COPY_SPACE) ids->tgid = ids->tid; - } - if (flags == THREAD_SAME_SPACE) { - /* - * If the tgid of original thread is supplied, that - * implies the new thread wants to be in the same group, - * and we leave it as it is. Otherwise the thread gets - * the same group id as its unique thread id. - */ - if (ids->tgid != orig->tgid) - ids->tgid = ids->tid; - } + /* + * If the tgid of original thread is supplied, that implies the + * new thread wants to be in the same group, and we leave it as + * it is. Otherwise the thread gets the same group id as its + * unique thread id. + */ + if (flags == THREAD_SAME_SPACE && ids->tgid != orig->tgid) + ids->tgid = ids->tid; /* Set all ids */ set_task_ids(new, ids); @@ -249,31 +237,43 @@ int thread_setup_new_ids(struct task_ids *ids, unsigned int flags, int thread_setup_space(struct ktcb *tcb, struct task_ids *ids, unsigned int flags) { struct address_space *space, *new; + int ret = 0; address_space_reference_lock(); if (flags == THREAD_SAME_SPACE) { - if (!(space = address_space_find(ids->spid))) - return -ESRCH; + if (!(space = address_space_find(ids->spid))) { + ret = -ESRCH; + goto out; + } address_space_attach(tcb, space); } if (flags == THREAD_COPY_SPACE) { - if (!(space = address_space_find(ids->spid))) - return -ESRCH; - if (IS_ERR(new = address_space_create(space))) - return (int)new; + if (!(space = address_space_find(ids->spid))) { + ret = -ESRCH; + goto out; + } + if (IS_ERR(new = address_space_create(space))) { + ret = (int)new; + goto out; + } + ids->spid = new->spid; /* Returned back to caller */ address_space_attach(tcb, new); address_space_add(new); } if (flags == THREAD_NEW_SPACE) { - if (IS_ERR(new = address_space_create(0))) - return (int)new; + if (IS_ERR(new = address_space_create(0))) { + ret = (int)new; + goto out; + } + ids->spid = new->spid; /* Returned back to caller */ address_space_attach(tcb, new); address_space_add(new); } +out: address_space_reference_unlock(); - return 0; + return ret; } int thread_create(struct task_ids *ids, unsigned int flags) @@ -286,22 +286,20 @@ int thread_create(struct task_ids *ids, unsigned int flags) if (!(new = tcb_alloc_init())) return -ENOMEM; - if ((err = thread_setup_space(new, ids, flags))) { - tcb_delete(new); + if (flags != THREAD_NEW_SPACE) { + BUG_ON(!(orig_task = tcb_find_by_space(ids->spid))); + } else + orig_task = 0; + + if ((err = thread_setup_space(new, ids, flags)) < 0) { + /* Since it hasn't initialised maturely, we delete it this way */ + free_page(new); return err; } - if (flags != THREAD_NEW_SPACE) { - BUG_ON(!(orig_task = tcb_find(ids->tid))); - - /* Set up ids and context using original tcb */ - thread_setup_new_ids(ids, flags, new, orig_task); - arch_setup_new_thread(new, orig_task, flags); - } else { - /* Set up ids and context from scratch */ - thread_setup_new_ids(ids, flags, new, 0); - arch_setup_new_thread(new, 0, flags); - } + /* Set up ids and context using original tcb or from scratch */ + thread_setup_new_ids(ids, flags, new, (orig_task) ? orig_task : 0); + arch_setup_new_thread(new, (orig_task) ? orig_task : 0, flags); tcb_add(new); diff --git a/src/arch/arm/v5/mm.c b/src/arch/arm/v5/mm.c index 664a2f5..157f513 100644 --- a/src/arch/arm/v5/mm.c +++ b/src/arch/arm/v5/mm.c @@ -419,6 +419,12 @@ int remove_mapping(unsigned long vaddr) return remove_mapping_pgd(vaddr, TASK_PGD(current)); } +int delete_page_tables(struct address_space *space) +{ + remove_mapping_pgd_all_user(space->pgd); + free_pgd(space->pgd); + return 0; +} /* * Copies userspace entries of one task to another. In order to do that, diff --git a/src/generic/space.c b/src/generic/space.c index e8487e2..1128f7c 100644 --- a/src/generic/space.c +++ b/src/generic/space.c @@ -6,12 +6,14 @@ #include INC_GLUE(memory.h) #include INC_GLUE(memlayout.h) #include INC_ARCH(exception.h) +#include INC_SUBARCH(mm.h) #include #include #include #include #include #include +#include struct address_space_list { struct list_head list; @@ -69,7 +71,9 @@ struct address_space *address_space_find(l4id_t spid) void address_space_add(struct address_space *space) { spin_lock(&address_space_list.list_lock); + BUG_ON(!list_empty(&space->list)); list_add(&space->list, &address_space_list.list); + BUG_ON(!++address_space_list.count); spin_unlock(&address_space_list.list_lock); } @@ -77,23 +81,24 @@ void address_space_remove(struct address_space *space) { spin_lock(&address_space_list.list_lock); BUG_ON(list_empty(&space->list)); + BUG_ON(--address_space_list.count < 0); list_del_init(&space->list); spin_unlock(&address_space_list.list_lock); } +/* Assumes address space reflock is already held */ void address_space_delete(struct address_space *space) { - /* Address space refcount lock must be held */ - - /* Sanity checks ??? */ + BUG_ON(space->ktcb_refs); /* Traverse the page tables and delete private pmds */ + delete_page_tables(space); - /* Delete the top-level pgd */ - - /* Return the space id ??? */ + /* Return the space id */ + id_del(space_id_pool, space->spid); /* Deallocate the space structure */ + kfree(space); } struct address_space *address_space_create(struct address_space *orig) @@ -120,6 +125,13 @@ struct address_space *address_space_create(struct address_space *orig) /* Copy all kernel entries */ copy_pgd_kern_all(pgd); + /* + * Set up space id: Always allocate a new one. Specifying a space id + * is not allowed since spid field is used to indicate the space to + * copy from. + */ + space->spid = id_new(space_id_pool); + /* If an original space is supplied */ if (orig) { /* Copy its user entries/tables */ diff --git a/src/generic/tcb.c b/src/generic/tcb.c index ca4bac4..ae84c0e 100644 --- a/src/generic/tcb.c +++ b/src/generic/tcb.c @@ -75,6 +75,7 @@ void tcb_delete(struct ktcb *tcb) BUG_ON(tcb->wqh_recv.sleepers > 0); BUG_ON(!list_empty(&tcb->task_list)); BUG_ON(!list_empty(&tcb->rq_list)); + BUG_ON(tcb->rq); BUG_ON(tcb->nlocks); BUG_ON(tcb->waiting_on); BUG_ON(tcb->wq); @@ -87,15 +88,35 @@ void tcb_delete(struct ktcb *tcb) BUG_ON(--tcb->space->ktcb_refs < 0); /* No refs left for the space, delete it */ - if (tcb->space->ktcb_refs == 0) + if (tcb->space->ktcb_refs == 0) { + address_space_remove(tcb->space); address_space_delete(tcb->space); + } address_space_reference_unlock(); + /* Deallocate tcb ids */ + id_del(thread_id_pool, tcb->tid); + /* Free the tcb */ free_page(tcb); } +struct ktcb *tcb_find_by_space(l4id_t spid) +{ + struct ktcb *task; + + spin_lock(&ktcb_list.list_lock); + list_for_each_entry(task, &ktcb_list.list, task_list) { + if (task->space->spid == spid) { + spin_unlock(&ktcb_list.list_lock); + return task; + } + } + spin_unlock(&ktcb_list.list_lock); + return 0; +} + struct ktcb *tcb_find(l4id_t tid) { struct ktcb *task; @@ -115,6 +136,7 @@ void tcb_add(struct ktcb *new) { spin_lock(&ktcb_list.list_lock); BUG_ON(!list_empty(&new->task_list)); + BUG_ON(!++ktcb_list.count); list_add(&new->task_list, &ktcb_list.list); spin_unlock(&ktcb_list.list_lock); } @@ -123,6 +145,7 @@ void tcb_remove(struct ktcb *new) { spin_lock(&ktcb_list.list_lock); BUG_ON(list_empty(&new->task_list)); + BUG_ON(--ktcb_list.count < 0); list_del_init(&new->task_list); spin_unlock(&ktcb_list.list_lock); }