From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755307Ab1D1Vfv (ORCPT ); Thu, 28 Apr 2011 17:35:51 -0400 Received: from smtp-out.google.com ([74.125.121.67]:40560 "EHLO smtp-out.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754290Ab1D1Vft (ORCPT ); Thu, 28 Apr 2011 17:35:49 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=google.com; s=beta; h=date:from:x-x-sender:to:cc:subject:in-reply-to:message-id :references:user-agent:mime-version:content-type; b=kUjufzLe8HzQj3Q7PrilzYg8O5Rgsji9KfJgqw6TxDn5f6Ef0oxiIyLk6wKJ06I01u XvC6yZ3Fb61M8OnzA+nw== Date: Thu, 28 Apr 2011 14:35:32 -0700 (PDT) From: David Rientjes X-X-Sender: rientjes@chino.kir.corp.google.com To: John Stultz cc: LKML , KOSAKI Motohiro , Dave Hansen , Andrew Morton , linux-mm@kvack.org Subject: Re: [PATCH 3/3] comm: ext4: Protect task->comm access by using get_task_comm() In-Reply-To: <1303963411-2064-4-git-send-email-john.stultz@linaro.org> Message-ID: References: <1303963411-2064-1-git-send-email-john.stultz@linaro.org> <1303963411-2064-4-git-send-email-john.stultz@linaro.org> User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-System-Of-Record: true Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 27 Apr 2011, John Stultz wrote: > diff --git a/fs/ext4/file.c b/fs/ext4/file.c > index 7b80d54..d37414e 100644 > --- a/fs/ext4/file.c > +++ b/fs/ext4/file.c > @@ -124,11 +124,15 @@ ext4_file_write(struct kiocb *iocb, const struct iovec *iov, > static unsigned long unaligned_warn_time; > > /* Warn about this once per day */ > - if (printk_timed_ratelimit(&unaligned_warn_time, 60*60*24*HZ)) > + if (printk_timed_ratelimit(&unaligned_warn_time, 60*60*24*HZ)) { > + char comm[TASK_COMM_LEN]; > + > + get_task_comm(comm, current); > ext4_msg(inode->i_sb, KERN_WARNING, > "Unaligned AIO/DIO on inode %ld by %s; " > "performance will be poor.", > - inode->i_ino, current->comm); > + inode->i_ino, comm); > + } > mutex_lock(ext4_aio_mutex(inode)); > ext4_aiodio_wait(inode); > } Thanks very much for looking into concurrent readers of current->comm, John! This patch in the series demonstrates one of the problems with using get_task_comm(), however: we must allocate a 16-byte buffer on the stack and that could become risky if we don't know its current depth. We may be particularly deep in the stack and then cause an overflow because of the 16 bytes. I'm wondering if it would be better for ->comm to be protected by a spinlock (or rwlock) other than ->alloc_lock and then just require readers to take the lock prior to dereferencing it? That's what is done in the oom killer with task_lock(). Perhaps you could introduce new task_comm_lock() and task_comm_unlock() to prevent the extra stack usage in over 300 locations within the kernel? From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail191.messagelabs.com (mail191.messagelabs.com [216.82.242.19]) by kanga.kvack.org (Postfix) with ESMTP id 630456B0011 for ; Thu, 28 Apr 2011 17:35:54 -0400 (EDT) Received: from hpaq11.eem.corp.google.com (hpaq11.eem.corp.google.com [172.25.149.11]) by smtp-out.google.com with ESMTP id p3SLZZMK029417 for ; Thu, 28 Apr 2011 14:35:35 -0700 Received: from pzk30 (pzk30.prod.google.com [10.243.19.158]) by hpaq11.eem.corp.google.com with ESMTP id p3SLZJG3002390 (version=TLSv1/SSLv3 cipher=RC4-MD5 bits=128 verify=NOT) for ; Thu, 28 Apr 2011 14:35:34 -0700 Received: by pzk30 with SMTP id 30so2396121pzk.18 for ; Thu, 28 Apr 2011 14:35:33 -0700 (PDT) Date: Thu, 28 Apr 2011 14:35:32 -0700 (PDT) From: David Rientjes Subject: Re: [PATCH 3/3] comm: ext4: Protect task->comm access by using get_task_comm() In-Reply-To: <1303963411-2064-4-git-send-email-john.stultz@linaro.org> Message-ID: References: <1303963411-2064-1-git-send-email-john.stultz@linaro.org> <1303963411-2064-4-git-send-email-john.stultz@linaro.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-linux-mm@kvack.org List-ID: To: John Stultz Cc: LKML , KOSAKI Motohiro , Dave Hansen , Andrew Morton , linux-mm@kvack.org On Wed, 27 Apr 2011, John Stultz wrote: > diff --git a/fs/ext4/file.c b/fs/ext4/file.c > index 7b80d54..d37414e 100644 > --- a/fs/ext4/file.c > +++ b/fs/ext4/file.c > @@ -124,11 +124,15 @@ ext4_file_write(struct kiocb *iocb, const struct iovec *iov, > static unsigned long unaligned_warn_time; > > /* Warn about this once per day */ > - if (printk_timed_ratelimit(&unaligned_warn_time, 60*60*24*HZ)) > + if (printk_timed_ratelimit(&unaligned_warn_time, 60*60*24*HZ)) { > + char comm[TASK_COMM_LEN]; > + > + get_task_comm(comm, current); > ext4_msg(inode->i_sb, KERN_WARNING, > "Unaligned AIO/DIO on inode %ld by %s; " > "performance will be poor.", > - inode->i_ino, current->comm); > + inode->i_ino, comm); > + } > mutex_lock(ext4_aio_mutex(inode)); > ext4_aiodio_wait(inode); > } Thanks very much for looking into concurrent readers of current->comm, John! This patch in the series demonstrates one of the problems with using get_task_comm(), however: we must allocate a 16-byte buffer on the stack and that could become risky if we don't know its current depth. We may be particularly deep in the stack and then cause an overflow because of the 16 bytes. I'm wondering if it would be better for ->comm to be protected by a spinlock (or rwlock) other than ->alloc_lock and then just require readers to take the lock prior to dereferencing it? That's what is done in the oom killer with task_lock(). Perhaps you could introduce new task_comm_lock() and task_comm_unlock() to prevent the extra stack usage in over 300 locations within the kernel? -- 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: email@kvack.org