Messages in this thread |  | | Subject | Re: [PATCH] mm,madvise: bugfix of madvise systemcall infinite loop under special circumstances. | From | 郭雪楠 <> | Date | Mon, 27 Nov 2017 10:54:39 +0800 |
| |
Hi,Michal, Whether need me to modify according your modification and resubmit a new patch?
在 2017/11/25 9:52, 郭雪楠 写道: > Yes , your modification is much better! thanks. > > 在 2017/11/24 21:08, Michal Hocko 写道: >> On Fri 24-11-17 20:51:29, 郭雪楠 wrote: >>> Sorry,I explained wrong before. But,I've tested using trinity in DAX >>> mode,and I'am sure it has possibility of triggering an soft lockup. I >>> have >>> encountered the problem of endless loop here . >>> >>> I had a little problem here,I correct it . >>> under Initial state : >>> [ start = vam->vm_start < vam->vm_end < end ] >>> >>> When [start = vam->vm_start] the program enters for{;;} loop >>> ,find_vma_prev() will set the pointer vma and the pointer prev (prev = >>> vam->vm_prev ). Normally ,madvise_vma() will always move the pointer >>> prev >>> ,but when use DAX mode , it will never update . >> [...] >>> if (prev) // here prev not NULL,it will always enter this branch .. >>> vma = prev->vm_next; >>> else /* madvise_remove dropped mmap_sem */ >>> vma = find_vma(current->mm, start); >> >> You are right! My fault, I managed to confuse myself in the code flow. >> It really looks like this has been broken for more than 10 years since >> fe77ba6f4f97 ("[PATCH] xip: madvice/fadvice: execute in place"). >> >> Maybe the following would be more readable and less error prone? >> --- >> diff --git a/mm/madvise.c b/mm/madvise.c >> index 375cf32087e4..a631c414f915 100644 >> --- a/mm/madvise.c >> +++ b/mm/madvise.c >> @@ -276,30 +276,26 @@ static long madvise_willneed(struct >> vm_area_struct *vma, >> { >> struct file *file = vma->vm_file; >> + *prev = vma; >> #ifdef CONFIG_SWAP >> if (!file) { >> - *prev = vma; >> force_swapin_readahead(vma, start, end); >> return 0; >> } >> - if (shmem_mapping(file->f_mapping)) { >> - *prev = vma; >> + if (shmem_mapping(file->f_mapping)) >> force_shm_swapin_readahead(vma, start, end, >> file->f_mapping); >> return 0; >> - } >> #else >> if (!file) >> return -EBADF; >> #endif >> - if (IS_DAX(file_inode(file))) { >> + if (IS_DAX(file_inode(file))) >> /* no bad return value, but ignore advice */ >> return 0; >> - } >> - *prev = vma; >> start = ((start - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff; >> if (end > vma->vm_end) >> end = vma->vm_end; >>
|  |