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