Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fs: Improve eventpoll logging to stop indicting timerfd
timerfd doesn't create any wakelocks; eventpoll can, and is creating the
wakelocks we see called "[timerfd]".  eventpoll creates two kinds of
wakelocks: a single top-level lock associated with the eventpoll fd
itself, and one additional lock for each fd it is polling that needs
such a lock (e.g. those using EPOLLWAKEUP).  Current code names the
per-fd locks using the undecorated names of the fds' associated files
(hence "[timerfd]"), and is naming the top-level lock after the PID of
the caller and the name of the file behind the first fd for which a
per-fd lock is created.  To make things clearer, the top-level lock is
now named using the caller command name and an "epollfd" designation,
while the per-fd locks are also named with the caller's command name (to
associate them with the top-level lock) and their respective fds' file
names.

Revert "Add kernel logging for when timerfd_read blocks"
This reverts commit 47c748b.

Adding logging to timerfd_read turns out to have been superfluous, since
the blocking that occurs in timerfd_read properly relinquishes the CPU
without holding any wakelocks.  The logging also proved to be overly
"chatty" due to frequency.

Bug: 63866355
Bug: 38042165
Test: Ran on device and observed new wakelock naming in bugreport,
dumpsys batterystats, /d/tracing/trace, and d/wakeup_reasons.
Signed-off-by: Kelly Rossmoyer <krossmo@google.com>

Change-Id: I4772c41a407afd17f87a6c15e01986c53ad0f9c0
  • Loading branch information
krossmo authored and MoetaYuko committed Aug 16, 2019
1 parent 70af78d commit 81cf6ad
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 11 deletions.
14 changes: 10 additions & 4 deletions fs/eventpoll.c
Expand Up @@ -1240,17 +1240,23 @@ static int ep_create_wakeup_source(struct epitem *epi)
{
const char *name;
struct wakeup_source *ws;
char task_comm_buf[TASK_COMM_LEN];
char buf[64];

get_task_comm(task_comm_buf, current);

name = epi->ffd.file->f_path.dentry->d_name.name;
if (!epi->ep->ws) {
char buf[64];
snprintf(buf, sizeof(buf), "eventpoll pid:%d file:%s", current->pid, name);
snprintf(buf, sizeof(buf), "epoll_%.*s_epollfd",
(int)sizeof(task_comm_buf), task_comm_buf);
epi->ep->ws = wakeup_source_register(buf);
if (!epi->ep->ws)
return -ENOMEM;
}

ws = wakeup_source_register(name);
name = epi->ffd.file->f_path.dentry->d_name.name;
snprintf(buf, sizeof(buf), "epoll_%.*s_file:%s",
(int)sizeof(task_comm_buf), task_comm_buf, name);
ws = wakeup_source_register(buf);

if (!ws)
return -ENOMEM;
Expand Down
19 changes: 12 additions & 7 deletions fs/timerfd.c
Expand Up @@ -44,6 +44,8 @@ struct timerfd_ctx {
bool might_cancel;
};

static atomic_t instance_count = ATOMIC_INIT(0);

static LIST_HEAD(cancel_list);
static DEFINE_SPINLOCK(cancel_lock);

Expand Down Expand Up @@ -254,13 +256,8 @@ static ssize_t timerfd_read(struct file *file, char __user *buf, size_t count,
spin_lock_irq(&ctx->wqh.lock);
if (file->f_flags & O_NONBLOCK)
res = -EAGAIN;
else {
printk("timerfd blocking read by tid %d\n",
pid_nr(get_task_pid(current, PIDTYPE_PID)));
else
res = wait_event_interruptible_locked_irq(ctx->wqh, ctx->ticks);
printk("timerfd blocking read released by tid %d\n",
pid_nr(get_task_pid(current, PIDTYPE_PID)));
}

/*
* If clock has changed, we do not care about the
Expand Down Expand Up @@ -391,6 +388,9 @@ SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)
{
int ufd;
struct timerfd_ctx *ctx;
char task_comm_buf[TASK_COMM_LEN];
char file_name_buf[32];
int instance;

/* Check the TFD_* constants for consistency. */
BUILD_BUG_ON(TFD_CLOEXEC != O_CLOEXEC);
Expand Down Expand Up @@ -422,7 +422,12 @@ SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)

ctx->moffs = ktime_mono_to_real((ktime_t){ .tv64 = 0 });

ufd = anon_inode_getfd("[timerfd]", &timerfd_fops, ctx,
instance = atomic_inc_return(&instance_count);
get_task_comm(task_comm_buf, current);
snprintf(file_name_buf, sizeof(file_name_buf), "[timerfd%d_%.*s]",
instance, (int)sizeof(task_comm_buf), task_comm_buf);

ufd = anon_inode_getfd(file_name_buf, &timerfd_fops, ctx,
O_RDWR | (flags & TFD_SHARED_FCNTL_FLAGS));
if (ufd < 0)
kfree(ctx);
Expand Down

0 comments on commit 81cf6ad

Please sign in to comment.