Re: [PATCH 1/2] (repost) New System call, unshare (fwd)

[Janak Desai <janak@us.ibm.com>]

Nick Piggin wrote:

> Janak Desai wrote:
>
>
> > -    tsk->min_flt = tsk->maj_flt = 0;
> > -    tsk->nvcsw = tsk->nivcsw = 0;
> > +    /*
> > +     * If the process memory is being duplicated as part of the
> > +     * unshare system call, we are working with the current process
> > +     * and not a newly allocated task strucutre, and should not
> > +     * zero out fault info, context switch counts, mm and active_mm
> > +     * fields.
> > +     */
> > +    if (copy_share_action == MAY_SHARE) {
> > +        tsk->min_flt = tsk->maj_flt = 0;
> > +        tsk->nvcsw = tsk->nivcsw = 0;
> >  
>
>
> Why don't you just do this in copy_process?

I was trying to avoid changing interface of copy_process since it
is also used by do_fork() and fork_idle().

> > -    tsk->mm = NULL;
> > -    tsk->active_mm = NULL;
> > +        tsk->mm = NULL;
> > +        tsk->active_mm = NULL;
> > +    }
> >  
> >      /*
> >       * Are we cloning a kernel thread?
> > @@ -1002,7 +1023,7 @@ static task_t *copy_process(unsigned lon
> >          goto bad_fork_cleanup_fs;
> >      if ((retval = copy_signal(clone_flags, p)))
> >          goto bad_fork_cleanup_sighand;
> > -    if ((retval = copy_mm(clone_flags, p)))
> > +    if ((retval = copy_mm(clone_flags, p, MAY_SHARE)))
> >          goto bad_fork_cleanup_signal;
> >      if ((retval = copy_keys(clone_flags, p)))
> >          goto bad_fork_cleanup_mm;
> > @@ -1317,3 +1338,172 @@ void __init proc_caches_init(void)
> >              sizeof(struct mm_struct), 0,
> >              SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL, NULL);
> >  }
> > +
> > +/*
> > + * unshare_mm is called from the unshare system call handler function to
> > + * make a private copy of the mm_struct structure. It calls copy_mm with
> > + * CLONE_VM flag cleard, to ensure that a private copy of mm_struct 
> > is made,
> > + * and with mm_copy_share enum set to UNSHARE, to ensure that copy_mm
> > + * does not clear fault info, context switch counts, mm and active_mm
> > + * fields of the mm_struct.
> > + */
> > +static int unshare_mm(unsigned long unshare_flags, struct task_struct 
> > *tsk)
> > +{
> > +    int retval = 0;
> > +    struct mm_struct *mm = tsk->mm;
> > +
> > +    /*
> > +     * If the virtual memory is being shared, make a private
> > +     * copy and disassociate the process from the shared virtual
> > +     * memory.
> > +     */
> > +    if (atomic_read(&mm->mm_users) > 1) {
> > +        retval = copy_mm((unshare_flags & ~CLONE_VM), tsk, UNSHARE);
> > +
> > +        /*
> > +         * If copy_mm was successful, decrement the number of users
> > +         * on the original, shared, mm_struct.
> > +         */
> > +        if (!retval)
> > +            atomic_dec(&mm->mm_users);
> > +    }
> > +    return retval;
> > +}
> > +
>
>
> What prevents thread 1 from decrementing mm_users after thread 2 has
> found it to be 2?

Yes, Chris pointed out that as well. I will be reviewing and reworking
this logic.

Thanks.

-Janak


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