Thread (18 messages) 18 messages, 5 authors, 2011-05-20

Re: [PATCH 1/4] comm: Introduce comm_lock spinlock to protect task->comm access

From: KOSAKI Motohiro <hidden>
Date: 2011-05-18 02:01:47
Also in: lkml

quoted hunk ↗ jump to hunk
diff --git a/fs/exec.c b/fs/exec.c
index 5e62d26..34fa611 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -998,17 +998,28 @@ static void flush_old_files(struct files_struct * files)

  char *get_task_comm(char *buf, struct task_struct *tsk)
  {
-	/* buf must be at least sizeof(tsk->comm) in size */
-	task_lock(tsk);
+	unsigned long flags;
+
+	spin_lock_irqsave(&tsk->comm_lock, flags);
  	strncpy(buf, tsk->comm, sizeof(tsk->comm));
-	task_unlock(tsk);
+	spin_unlock_irqrestore(&tsk->comm_lock, flags);
  	return buf;
  }

  void set_task_comm(struct task_struct *tsk, char *buf)
  {
+	unsigned long flags;
+
+	/*
+	 * XXX - Even though comm is protected by comm_lock,
+	 * we take the task_lock here to serialize against
+	 * current users that directly access comm.
+	 * Once those users are removed, we can drop the
+	 * task locking&  memsetting.
+	 */
If we provide __get_task_comm(), we can't remove memset() forever.

  	task_lock(tsk);
+	spin_lock_irqsave(&tsk->comm_lock, flags);
This is strange order. task_lock() doesn't disable interrupt.
And, can you please document why we need interrupt disabling?

  	/*
  	 * Threads may access current->comm without holding
  	 * the task lock, so write the string carefully.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  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@kvack.org"> email@kvack.org </a>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help