lkml.org 
[lkml]   [2013]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: pipe_release oops.
On Fri, Mar 08, 2013 at 01:26:49PM -0500, J??rn Engel wrote:
> On Fri, 8 March 2013 10:30:01 -0800, Linus Torvalds wrote:
> >
> > Hmm. So I've been trying to figure this out, and I really don't see
> > it. Every single pipe open routine *should* make sure that the inode
> > has an inode->i_pipe field. So if the open() has succeeded and you
> > have a valid file descriptor, the inode->i_pipe thing should be there.
>
> Ok, here is a wild idea that is very likely wrong. But some
> background first. I've had problems with process exit times and one
> of the culprits turned out to be exit_files() where one device driver
> went awol for several seconds. Fixing the device driver is hard, I
> didn't see a good reason not to call exit_files() earlier and
> exit_mm() was the other big offender, so the idea was to run both in
> parallel and I applied the patch below.
>
> As a result I've gotten a bunch of NULL pointer dereferences that only
> happen in virtual machines, never on real hardware. For example
> [<ffffffff81164bf8>] alloc_fd+0x38/0x130
> [<ffffffff8114857e>] do_sys_open+0xee/0x1f0
> [<ffffffff811486a1>] sys_open+0x21/0x30
> [<ffffffff815bea29>] system_call_fastpath+0x16/0x1b
>
> Now I can easily see how current->files being NULL will result in such
> backtraces. I can also see how my patch moves the NULLing of
> current->files a bit back in time. But I could never figure out how
> my patch could have introduced a race that didn't exist before.
>
> So the wild idea is that we have always had a very unlikely race with
> current->files being NULL and trinity happens to hit it somehow.
>
> J??rn

> + files_cookie = async_schedule(exit_files_async, tsk);
> exit_mm(tsk);
>
> if (group_dead)
> @@ -990,7 +998,7 @@ void do_exit(long code)
>
> exit_sem(tsk);
> exit_shm(tsk);
> - exit_files(tsk);
> + async_synchronize_cookie(files_cookie);

That doesn't do what you seem to think it's doing. It does *not* wait
for the completion of that sucker's execution - only the ones scheduled
before it. IOW, your exit_files_async() might very well be executed
*after* do_exit() completes and tsk gets reused.


\
 
 \ /
  Last update: 2013-03-11 01:21    [W:1.947 / U:0.620 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site