Re: [PATCH] BUG() in exec_mmap()

[Marcelo Tosatti]

On Thu, 9 Oct 2003, Ernie Petrides wrote:

> On Thursday, 9-Oct-2003 at 17:47 -0300, Marcelo Tosatti wrote:
> 
> > On Thu, 9 Oct 2003, Mika Penttilä wrote:
> >
> > > Hmm.. you still need to mmput(old_mm) etc, just remove the mm_users == 1
> > > optimization from the beginning of exec_mmap, so this patch is wrong!
> >
> > Right. Ill fix it up by hand.
> 
> Mika is correct that the exit_mmap(old_mm) still needs to happen on the
> last use of the "mm_struct".  But whether it's called directly from
> exec_mmap() or indirectly from mmput() still needs to depend on the
> value of "mm_users".
> 
> The original logic avoided the mmdrop(active_mm) call if there was an
> old_mm, so I'd infer that the mm_struct reference count is not bumped
> twice for both references from the task_struct (mm and active_mm).  So
> the patch would need to be reworked to avoid the double decrement, too.

I dont get you, sorry (I'm not a real expert on that piece of code, 
so...).

>From what I understand the "if (old_mm && mm_users == 1)" if case is just 
an optimization to avoid the allocation of a new mm structure.

The functionality will be the same without that piece of code, it will 
just be slower. 

> Sorry I missed the discussion on the original changes.  Was there a
> race condition with another cpu gaining a reference in proc_pid_status()
> or access_process_vm() or something like that?  

Exactly.

> Is it possible to just use down_read(&old_mm->mmap_sem) and
> up_read(&old_mm->mmap_sem) inside exec_mmap() around the optimized call
> to exit_mmap() instead?

Doing that locking inside exit_mmap() not feasible IMO... it might be too 
expensive. 

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/