Re: [PATCH] shm: optimize exit_shm()

[Vasiliy Kulikov <segoon@openwall.com>]

On Wed, Aug 03, 2011 at 21:29 +0200, Oleg Nesterov wrote:
> On 08/03, Vasiliy Kulikov wrote:
> >
> > On Wed, Aug 03, 2011 at 21:08 +0200, Manuel Lauss wrote:
> > > > +
> > > >        /* Destroy all already created segments, but not mapped yet */
> > > >        down_write(&shm_ids(ns).rw_mutex);
> > > >        if (shm_ids(ns).in_use)
> > >
> > > This check here is now unnecessary, yes?
> >
> > No, as I said in the comment above, other task may be holding the mutex and
> > deleting the last shm segment.  So, current task will see in_use == 1
> > before down_write(), but == 0 after it.
> 
> And? Why we can not do idr_for_each() in this (unikely) case?

Because it's pointless.  idr_for_each() would not find any used segment.

> > > And this also fixes the oops.
> >
> > Yes, but it only hides the real problem - tasks' dependency on initialized
> > init_*_ns.
> 
> This is true, but your patch has the same dependency, but pretends
> it doesn't ;) and it complicates the code.

I didn't say that .in_use check fixes the oops.  The description says
nothing about it.  It does an optimization, which is said in the
description.

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments