On Mon, 11 Jun 2007, Russell King wrote:
>
> However, I still question whether those down_read_trylock games are
> correct
That is certainly a valid question. It may well be that we should *always*
do the "search_exception_tables()" for kernel accesses. As it is, we
aren't doing all the validity checks that we *could* do.
However, the deadlock is real, and concerns kernel *bugs*:
- if the kernel takes a (wild) page fault while holding the mmap_sem
semaphore, we at one point just deadlocked instead of printign a nice
oops message. Some people (including me) spent way too much time
debugging silent and very-hard-to-debug lockups that should have just
caused a nice oops and made the bug obvious.
However, I agree that the check itself conceptually should be done
regardless of whether the mmap_sem is taken or not.
One solution migt be to just move the whole
if (!user_mode(regs) && !search_exception_tables(pc))
goto no_context;
to _outside_ the mmap_sem check, and change the code to basically be
/*
* If we're in an interrupt or have no user
* context, we must not take the fault..
*/
if (in_atomic() || !mm)
goto no_context;
/*
* If we're in kernel mode, and don't have the access
* listed in the exception tables, we must not take
* the fault...
*/
if (!user_mode(regs) && !search_exception_tables(pc))
goto no_context;
/* Do the fault */
down_read(&mm->mmap_sem);
fault = __do_page_fault(mm, addr, flags, tsk);
up_read(&mm->mmap_sem);
which looks cleaner, I agree.
However, one issue that I don't know was ever part of the reason is that
the exception table search may be expensive, and maybe skipping it when
getting the mmap_sem is easy is the right thing for a _performance_
reason. I doubt it's a real issue (we should generally not be taking
a lot of faults from kernel mode!), but it's a thing to keep in mind.
Comments?
Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
