Re: [PATCH 1/2] break out page allocation warning code

[john stultz]

On Wed, 2011-04-20 at 13:24 -0700, David Rientjes wrote:
> On Wed, 20 Apr 2011, KOSAKI Motohiro wrote:
> 
> > > That was true a while ago, but you now need to protect every thread's 
> > > ->comm with get_task_comm() or ensuring task_lock() is held to protect 
> > > against /proc/pid/comm which can change other thread's ->comm.  That was 
> > > different before when prctl(PR_SET_NAME) would only operate on current, so 
> > > no lock was needed when reading current->comm.
> > 
> > Right. /proc/pid/comm is evil. We have to fix it. otherwise we need change
> > all of current->comm user. It's very lots!
> > 
> 
> Fixing it in this case would be removing it and only allowing it for 
> current via the usual prctl() :)  The code was introduced in 4614a696bd1c 
> (procfs: allow threads to rename siblings via /proc/pid/tasks/tid/comm) in 
> December 2009 and seems to originally be meant for debugging.  We simply 
> can't continue to let it modify any thread's ->comm unless we change the 
> over 300 current->comm deferences in the kernel.
> 
> I'd prefer that we remove /proc/pid/comm entirely or at least prevent 
> writing to it unless CONFIG_EXPERT.

Eeeh. That's probably going to be a tough sell, as I think there is
wider interest in what it provides. Its useful for debugging
applications not kernels, so I doubt folks will want to rebuild their
kernel to try to analyze a java issue.

So I'm well aware that there is the chance that you catch the race and
read an incomplete/invalid comm (it was discussed at length when the
change went in), but somewhere I've missed how that's causing actual
problems. Other then just being "evil" and having the documented race,
could you clarify what the issue is that your hitting?

thanks
-john




--
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/