Messages in this thread Patch in this message | | | Date | Fri, 6 Mar 2020 14:03:03 +0100 | From | Oleksandr Natalenko <> | Subject | Re: [PATCH v7 6/7] mm/madvise: employ mmget_still_valid for write lock |
| |
Hello.
On Fri, Mar 06, 2020 at 01:52:07PM +0100, Vlastimil Babka wrote: > > diff --git a/mm/madvise.c b/mm/madvise.c > > index e794367f681e..e77c6c1fad34 100644 > > --- a/mm/madvise.c > > +++ b/mm/madvise.c > > @@ -1118,6 +1118,8 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm, > > if (write) { > > if (down_write_killable(&mm->mmap_sem)) > > return -EINTR; > > + if (current->mm != mm && !mmget_still_valid(mm)) > > + goto skip_mm; > > This will return 0, is that correct? Shoudln't there be a similar error e.g. as > when finding the task by pid fails (-ESRCH ?), because IIUC the task here is > going away and dumping the core?
Yeah.
Something like this then:
=== diff --git a/mm/madvise.c b/mm/madvise.c index 48d1da08c160..7ed2f4d13924 100644 --- a/mm/madvise.c +++ b/mm/madvise.c @@ -1122,6 +1122,10 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm, if (write) { if (down_write_killable(&mm->mmap_sem)) return -EINTR; + if (current->mm != mm && !mmget_still_valid(mm)) { + error = -ESRCH; + goto skip_mm; + } } else { down_read(&mm->mmap_sem); } @@ -1173,6 +1177,7 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm, } out: blk_finish_plug(&plug); +skip_mm: if (write) up_write(&mm->mmap_sem); else ===
?
> > > } else { > > down_read(&mm->mmap_sem); > > } > > @@ -1169,6 +1171,7 @@ int do_madvise(struct task_struct *target_task, struct mm_struct *mm, > > } > > out: > > blk_finish_plug(&plug); > > +skip_mm: > > if (write) > > up_write(&mm->mmap_sem); > > else > > >
-- Best regards, Oleksandr Natalenko (post-factum) Principal Software Maintenance Engineer
| |