I did not see this patch going any where? Looks like this patch needs Stephan's blessings. Stephan, can you ACK on this one. Thanks, Anil Keshavamurthy >-----Original Message----- >From: linux-ia64-owner@xxxxxxxxxxxxxxx >[mailto:linux-ia64-owner@xxxxxxxxxxxxxxx] On Behalf Of Cliff Wickman >Sent: Wednesday, February 01, 2006 11:57 AM >To: linux-ia64@xxxxxxxxxxxxxxx >Subject: [PATCH 1/1] ia64: perfmon.c trips BUG_ON in put_page_testzero > > > >The problem occurs during exit processing when a task has >done a pfm_context_create() (and pfm_smpl_buffer_alloc()). > >To trigger the problem requires: > - that the task be interrupted (so that it does not unmap its >smpl buffer) > - that there is another process accessing its mm_struct > >At exit: > - sets its task->mm to null > - calls mmput(). But the task's vmas are not unmapped >because the mm_users > count is still 1 > though pfm_flush() is unable to unmap the buffer vma, but it does > "put" the buffer pages when it frees the kernel mapping >(vm_struct). > - closes the pfm file with pfm_flush() and pfm_close(), >which un-reserve > the smpl buffer pages, but the buffer vma is not released. > >When that external task does the final mmput() on this mm_struct, > the sampling buffer's vma is unmapped. This does the second >put on its > pages and trips the BUG_ON(page_count(p) == 0) in >put_page_testzero(p) > >This patch maintains a list of active pfm tasks and their mm_structs. >The list allows pfm_flush() to locate the exiting task's >mm_struct so that >it can unmap the vma. >(this could be done without a list, but would require another >field in the > task_struct) >(this fix is not necessary for Stephane Eranian's perfmon2) > >Diffed against 2.6.15-rc6 > >Signed-off-by: Cliff Wickman <cpw@xxxxxxx> >--- > > arch/ia64/kernel/perfmon.c | 138 >+++++++++++++++++++++++++++++++++++++++++---- > kernel/exit.c | 10 +++ > 2 files changed, 137 insertions(+), 11 deletions(-) > >Index: linux-2.6.15-rc6/kernel/exit.c >=================================================================== >--- linux-2.6.15-rc6.orig/kernel/exit.c >+++ linux-2.6.15-rc6/kernel/exit.c >@@ -38,6 +38,11 @@ > extern void sem_exit (void); > extern struct task_struct *child_reaper; > >+#ifdef CONFIG_PERFMON >+void pfm_remove_task_from_list(struct task_struct *); >+extern int pfm_check_list; >+#endif >+ > int getrusage(struct task_struct *, int, struct rusage __user *); > > static void exit_mm(struct task_struct * tsk); >@@ -527,6 +532,11 @@ static void exit_mm(struct task_struct * > up_read(&mm->mmap_sem); > enter_lazy_tlb(mm, current); > task_unlock(tsk); >+#ifdef CONFIG_PERFMON >+ if (atomic_read(&mm->mm_users) == 1 && pfm_check_list) { >+ pfm_remove_task_from_list(current); >+ } >+#endif > mmput(mm); > } > >Index: linux-2.6.15-rc6/arch/ia64/kernel/perfmon.c >=================================================================== >--- linux-2.6.15-rc6.orig/arch/ia64/kernel/perfmon.c >+++ linux-2.6.15-rc6/arch/ia64/kernel/perfmon.c >@@ -535,6 +535,21 @@ static int pfm_flush(struct file *filp); > #define pfm_get_cpu_var(v) __ia64_per_cpu_var(v) > #define pfm_get_cpu_data(a,b) per_cpu(a, b) > >+struct mm_struct *pfm_lookup_mm(void); >+void pfm_remove_task_from_list(struct task_struct *); >+void pfm_record_tm(struct task_struct *); >+/* global: for exit_mmap() to call us back when the >mm_struct is removed */ >+int pfm_check_list=0; >+int pfm_num_in_list; >+EXPORT_SYMBOL(pfm_check_list); >+struct pfm_task_mm { >+ struct list_head pfm_tm_list; >+ struct task_struct *pfm_taskp; >+ struct mm_struct *pfm_mm; >+}; >+rwlock_t pfm_tmlist_lock; >+struct list_head pfm_tm_list_head; >+ > static inline void > pfm_put_task(struct task_struct *task) > { >@@ -1394,13 +1409,13 @@ pfm_unreserve_session(pfm_context_t *ctx > * a PROTECT_CTX() section. > */ > static int >-pfm_remove_smpl_mapping(struct task_struct *task, void >*vaddr, unsigned long size) >+pfm_remove_smpl_mapping(struct task_struct *task, struct >mm_struct *mm, void *vaddr, unsigned long size) > { > int r; > > /* sanity checks */ >- if (task->mm == NULL || size == 0UL || vaddr == NULL) { >- printk(KERN_ERR "perfmon: >pfm_remove_smpl_mapping [%d] invalid context mm=%p\n", >task->pid, task->mm); >+ if (mm == NULL || size == 0UL || vaddr == NULL) { >+ printk(KERN_ERR "perfmon: >pfm_remove_smpl_mapping [%d] invalid context mm=%p\n", task->pid, mm); > return -EINVAL; > } > >@@ -1409,13 +1424,13 @@ pfm_remove_smpl_mapping(struct task_stru > /* > * does the actual unmapping > */ >- down_write(&task->mm->mmap_sem); >+ down_write(&mm->mmap_sem); > > DPRINT(("down_write done smpl_vaddr=%p size=%lu\n", >vaddr, size)); > >- r = pfm_do_munmap(task->mm, (unsigned long)vaddr, size, 0); >+ r = pfm_do_munmap(mm, (unsigned long)vaddr, size, 0); > >- up_write(&task->mm->mmap_sem); >+ up_write(&mm->mmap_sem); > if (r !=0) { > printk(KERN_ERR "perfmon: [%d] unable to unmap >sampling buffer @%p size=%lu\n", task->pid, vaddr, size); > } >@@ -1493,6 +1508,12 @@ init_pfm_fs(void) > else > err = 0; > } >+ >+ /* initialize task/mm list */ >+ rwlock_init(&pfm_tmlist_lock); >+ INIT_LIST_HEAD(&pfm_tm_list_head); >+ pfm_num_in_list = 0; >+ > return err; > } > >@@ -1773,6 +1794,7 @@ pfm_flush(struct file *filp) > { > pfm_context_t *ctx; > struct task_struct *task; >+ struct mm_struct *mm; > struct pt_regs *regs; > unsigned long flags; > unsigned long smpl_buf_size = 0UL; >@@ -1876,11 +1898,14 @@ pfm_flush(struct file *filp) > * ctx_smpl_vaddr must never be cleared because it is needed > * by every task with access to the context > * >- * When called from do_exit(), the mm context is gone >already, therefore >- * mm is NULL, i.e., the VMA is already gone and we do >not have to >- * do anything here >+ * When called from do_exit(), the mm context may still >be present >+ * if mm_users is not zero. But the task mm pointer is >set to null >+ * at exit, so we have to use active_mm here. >+ * (we are depending on the assumption that active_mm >is not cleared >+ * at exit) > */ >- if (ctx->ctx_smpl_vaddr && current->mm) { >+ mm = pfm_lookup_mm(); >+ if (ctx->ctx_smpl_vaddr && mm && current->active_mm==mm) { > smpl_buf_vaddr = ctx->ctx_smpl_vaddr; > smpl_buf_size = ctx->ctx_smpl_size; > } >@@ -1893,7 +1918,7 @@ pfm_flush(struct file *filp) > * because some VM function reenables interrupts. > * > */ >- if (smpl_buf_vaddr) pfm_remove_smpl_mapping(current, >smpl_buf_vaddr, smpl_buf_size); >+ if (smpl_buf_vaddr) pfm_remove_smpl_mapping(current, >mm, smpl_buf_vaddr, smpl_buf_size); > > return 0; > } >@@ -1931,6 +1956,12 @@ pfm_close(struct inode *inode, struct fi > DPRINT(("bad magic\n")); > return -EBADF; > } >+ >+ /* It is possible that this task went thru do_exit with >+ mm_users > 1, in which case this task was not removed from >+ the list of pfm tasks with valid mm_struct's. So >make sure the >+ task is removed from the list. */ >+ pfm_remove_task_from_list(current); > > ctx = (pfm_context_t *)filp->private_data; > if (ctx == NULL) { >@@ -4333,6 +4364,9 @@ pfm_context_load(pfm_context_t *ctx, voi > */ > ctx->ctx_task = task; > >+ /* record the task/mm for this task, as it now has a context */ >+ pfm_record_tm(task); >+ > if (is_system) { > /* > * we load as stopped >@@ -6837,6 +6871,88 @@ pfm_inherit(struct task_struct *task, st > * the psr bits are already set properly in copy_threads() > */ > } >+ >+/* >+ * record task_struct and mm_struct in the list of valid pfm >mm_structs >+ */ >+void >+pfm_record_tm(struct task_struct *task) >+{ >+ struct pfm_task_mm *tm; >+ struct list_head *this, *next; >+ >+ read_lock(&pfm_tmlist_lock); >+ list_for_each_safe(this, next, &pfm_tm_list_head) { >+ tm = list_entry(this, struct pfm_task_mm, pfm_tm_list); >+ if (tm->pfm_taskp == task) { >+ /* don't record it twice */ >+ read_unlock(&pfm_tmlist_lock); >+ return; >+ } >+ } >+ >+ tm = kmalloc(sizeof(struct pfm_task_mm), GFP_KERNEL); >+ if (!tm) { >+ read_unlock(&pfm_tmlist_lock); >+ return; >+ } >+ tm->pfm_taskp = task; >+ tm->pfm_mm = task->mm; >+ list_add_tail(&tm->pfm_tm_list, &pfm_tm_list_head); >+ pfm_num_in_list++; >+ read_unlock(&pfm_tmlist_lock); >+ return; >+} >+ >+/* >+ * look up the current task in the list of valid pfm mm_structs >+ */ >+struct mm_struct * >+pfm_lookup_mm() >+{ >+ struct pfm_task_mm *tm; >+ struct list_head *this, *next; >+ >+ read_lock(&pfm_tmlist_lock); >+ list_for_each_safe(this, next, &pfm_tm_list_head) { >+ tm = list_entry(this, struct pfm_task_mm, pfm_tm_list); >+ if (current == tm->pfm_taskp) { >+ read_unlock(&pfm_tmlist_lock); >+ return tm->pfm_mm; >+ } >+ } >+ read_unlock(&pfm_tmlist_lock); >+ return NULL; >+} >+ >+/* >+ * called from exit_mmap when the task's mm_struct is removed >+ * (so that we do not use active_mm when it might point a >freed mm_struct >+ * (or another task's mm_struct)) >+ */ >+void >+pfm_remove_task_from_list(struct task_struct *task) >+{ >+ struct pfm_task_mm *tm; >+ struct list_head *this, *next; >+ >+ write_lock(&pfm_tmlist_lock); >+ list_for_each_safe(this, next, &pfm_tm_list_head) { >+ tm = list_entry(this, struct pfm_task_mm, pfm_tm_list); >+ if (current == tm->pfm_taskp) { >+ list_del(this); >+ kfree(tm); >+ pfm_num_in_list--; >+ /* clear the flag when all have been removed */ >+ if (pfm_num_in_list == 0) >+ pfm_check_list=0; >+ write_unlock(&pfm_tmlist_lock); >+ return; >+ } >+ } >+ write_unlock(&pfm_tmlist_lock); >+ return; >+} > #else /* !CONFIG_PERFMON */ > asmlinkage long > sys_perfmonctl (int fd, int cmd, void *arg, int count) >- >: send the line "unsubscribe >linux-ia64" in >the body of a message to majordomo@xxxxxxxxxxxxxxx >More majordomo info at http://vger.kernel.org/majordomo-info.html > - : send the line "unsubscribe linux-ia64" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
![]() |