> On Mon, 2011-04-18 at 13:25 -0700, David Rientjes wrote: > > It shouldn't be a follow-on patch since you're introducing a new feature > > here (vmalloc allocation failure warnings) and what I'm identifying is a > > race in the access to current->comm. A bug fix for a race should always > > preceed a feature that touches the same code. > > So, what's the race here? kmemleak.c says? > > /* > * There is a small chance of a race with set_task_comm(), > * however using get_task_comm() here may cause locking > * dependency issues with current->alloc_lock. In the worst > * case, the command line is not correct. > */ > strncpy(object->comm, current->comm, sizeof(object->comm)); > > We're trying to make sure we don't print out a partially updated > tsk->comm? Or, is there a bigger issue here like potential oopses or > kernel information leaks. > > 1. We require that no memory allocator ever holds the task lock for the > current task, and we audit all the existing GFP_ATOMIC users in the > kernel to ensure they're not doing it now. In the case of a problem, > we end up with a hung kernel while trying to get a message out to the > console. > 2. We remove current->comm from the printk(), and deal with the > information loss. > 3. We live with corrupted output, like the other ~400 in-kernel users of > ->comm do. (I'm assuming that very few of them hold the task lock). > In the case of a race, we get junk on the console, but an otherwise > fine bug report (the way it is now). > 4. We come up with some way to print out current->comm, without holding > any task locks. We could do this by copying it somewhere safe on > each context switch. Could probably also do it with RCU. > > There's also a very, very odd message in fs/exec.c: > > /* > * Threads may access current->comm without holding > * the task lock, so write the string carefully. > * Readers without a lock may see incomplete new > * names but are safe from non-terminating string reads. > */ The rule is, 1) writing comm need task_lock 2) read _another_ thread's comm need task_lock 3) read own comm no need task_lock That's the reason why oom-kill.c need task_lock and other a lot of place don't need task_lock. I agree this is very strange. it's only historical reason. The comment of set_task_comm() explained a race against (3). Thanks. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxxx For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>