On 06/24, Louis Rilling wrote:
>
> [ Resending because of buggy quotes in Eric's address. Sorry for the noise. ]
>
> On 06/19, Oleg Nesterov wrote:
> > And the last one on top of this series, before I go away from this
> > thread ;)
> >
> > Since my further fixes were not approved, I think at least it makes
> > sense to cleanup pid_ns_release_proc() logic a bit.
>
> It's completely untested and could be split into three patches. But I think that
> it solves the issues found so far, and that it will work with Eric's
> unshare(CLONE_NEWPID) too.
>
> What do you think about this approach?

Oh. I shouldn't continue to participate in this discussion... I don't have
the time and my previous patch proves that my patches should be ignored ;)

But, looking at this patch,

> - defer pid_ns_release_proc()->mntput() to a worqueue context, so that
>   pid_ns_release_proc() can be called in atomic context;

OK, not good but this is what I did too,

> - introduce pid_ns->nr_pids, so that we can count the number of pids
>   allocated by alloc_pidmap();

and this adds the extra code to alloc/free pidmap.

> - move the call to pid_ns_prepare_proc() to alloc_pid(), where we know
>   when the first pid of a namespace is allocated;

This is what I personally dislike. I do not think pid_ns_prepare_proc()
should depend on the fact that the first pid was already allocated.

And, this subjective, yes, but it looks a bit strange that upid->nr
has a reference to proc_mnt.

And of course, imho it would nice to not create the circular reference
we currently have.


Louis, Eric.

I am attaching my 2 patches (on top of cleanups) again. Could you take
a look?

Changes:

	- pid_ns_release_proc() nullifies sb->s_fs_info

	- proc_pid_lookup() and proc_self_readlink() check ns != NULL
	  (this is sb->s_fs_info)

I even tried to test this finally, seems to work.

I am not going to argue if you prefer Louis's approach. But I will appreciate
if you explain why my fix is wrong. I am curious because I spent 3 hours doing
grep fs/proc ;)

Oleg.