* [PATCH v2] fs: Improve eventpoll logging to stop indicting timerfd
@ 2021-04-01 2:16 Manish Varma
2021-04-01 2:28 ` Al Viro
2021-04-01 6:44 ` kernel test robot
0 siblings, 2 replies; 4+ messages in thread
From: Manish Varma @ 2021-04-01 2:16 UTC (permalink / raw)
To: Alexander Viro, Thomas Gleixner
Cc: linux-fsdevel, linux-kernel, kernel-team, Manish Varma,
Kelly Rossmoyer
timerfd doesn't create any wakelocks, but eventpoll can. When it does,
it names them after the underlying file descriptor, and since all
timerfd file descriptors are named "[timerfd]" (which saves memory on
systems like desktops with potentially many timerfd instances), all
wakesources created as a result of using the eventpoll-on-timerfd idiom
are called... "[timerfd]".
However, it becomes impossible to tell which "[timerfd]" wakesource is
affliated with which process and hence troubleshooting is difficult.
This change addresses this problem by changing the way eventpoll
wakesources are named:
1) the top-level per-process eventpoll wakesource is now named "epoll:P"
(instead of just "eventpoll"), where P, is the PID of the creating
process.
2) individual per-underlying-filedescriptor eventpoll wakesources are
now named "epollitemN:P.F", where N is a unique ID token and P is PID
of the creating process and F is the name of the underlying file
descriptor.
All together that should be splitted up into a change to eventpoll and
timerfd (or other file descriptors).
Co-developed-by: Kelly Rossmoyer <krossmo@google.com>
Signed-off-by: Kelly Rossmoyer <krossmo@google.com>
Signed-off-by: Manish Varma <varmam@google.com>
---
fs/eventpoll.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 7df8c0fa462b..8d3369a02633 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -297,6 +297,7 @@ static LIST_HEAD(tfile_check_list);
static long long_zero;
static long long_max = LONG_MAX;
+static atomic_t wakesource_create_id = ATOMIC_INIT(0);
struct ctl_table epoll_table[] = {
{
@@ -1451,15 +1452,23 @@ static int ep_create_wakeup_source(struct epitem *epi)
{
struct name_snapshot n;
struct wakeup_source *ws;
+ pid_t task_pid;
+ char buf[64];
+ int id;
+
+ task_pid = task_pid_nr(current);
if (!epi->ep->ws) {
- epi->ep->ws = wakeup_source_register(NULL, "eventpoll");
+ snprintf(buf, sizeof(buf), "epoll:%d", task_pid);
+ epi->ep->ws = wakeup_source_register(NULL, buf);
if (!epi->ep->ws)
return -ENOMEM;
}
+ id = atomic_inc_return(&wakesource_create_id);
take_dentry_name_snapshot(&n, epi->ffd.file->f_path.dentry);
- ws = wakeup_source_register(NULL, n.name.name);
+ snprintf(buf, sizeof(buf), "epollitem%d:%d.%s", id, task_pid, n.name.name);
+ ws = wakeup_source_register(NULL, buf);
release_dentry_name_snapshot(&n);
if (!ws)
--
2.31.0.291.g576ba9dcdaf-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] fs: Improve eventpoll logging to stop indicting timerfd
2021-04-01 2:16 [PATCH v2] fs: Improve eventpoll logging to stop indicting timerfd Manish Varma
@ 2021-04-01 2:28 ` Al Viro
2021-04-01 23:57 ` Manish Varma
2021-04-01 6:44 ` kernel test robot
1 sibling, 1 reply; 4+ messages in thread
From: Al Viro @ 2021-04-01 2:28 UTC (permalink / raw)
To: Manish Varma
Cc: Thomas Gleixner, linux-fsdevel, linux-kernel, kernel-team,
Kelly Rossmoyer
On Wed, Mar 31, 2021 at 07:16:45PM -0700, Manish Varma wrote:
> timerfd doesn't create any wakelocks, but eventpoll can. When it does,
> it names them after the underlying file descriptor, and since all
> timerfd file descriptors are named "[timerfd]" (which saves memory on
> systems like desktops with potentially many timerfd instances), all
> wakesources created as a result of using the eventpoll-on-timerfd idiom
> are called... "[timerfd]".
>
> However, it becomes impossible to tell which "[timerfd]" wakesource is
> affliated with which process and hence troubleshooting is difficult.
>
> This change addresses this problem by changing the way eventpoll
> wakesources are named:
>
> 1) the top-level per-process eventpoll wakesource is now named "epoll:P"
> (instead of just "eventpoll"), where P, is the PID of the creating
> process.
> 2) individual per-underlying-filedescriptor eventpoll wakesources are
> now named "epollitemN:P.F", where N is a unique ID token and P is PID
> of the creating process and F is the name of the underlying file
> descriptor.
>
> All together that should be splitted up into a change to eventpoll and
> timerfd (or other file descriptors).
FWIW, it smells like a variant of wakeup_source_register() that would
take printf format + arguments would be a good idea. I.e. something
like
> + snprintf(buf, sizeof(buf), "epoll:%d", task_pid);
> + epi->ep->ws = wakeup_source_register(NULL, buf);
... = wakeup_source_register(NULL, "epoll:%d", task_pid);
etc.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] fs: Improve eventpoll logging to stop indicting timerfd
2021-04-01 2:16 [PATCH v2] fs: Improve eventpoll logging to stop indicting timerfd Manish Varma
2021-04-01 2:28 ` Al Viro
@ 2021-04-01 6:44 ` kernel test robot
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-04-01 6:44 UTC (permalink / raw)
To: Manish Varma, Alexander Viro, Thomas Gleixner
Cc: kbuild-all, linux-fsdevel, linux-kernel, kernel-team,
Manish Varma, Kelly Rossmoyer
[-- Attachment #1: Type: text/plain, Size: 2925 bytes --]
Hi Manish,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc5 next-20210331]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Manish-Varma/fs-Improve-eventpoll-logging-to-stop-indicting-timerfd/20210401-101954
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git d19cc4bfbff1ae72c3505a00fb8ce0d3fa519e6c
config: um-randconfig-r006-20210401 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/49789288695d81f196d99fadd89b5101b26bca8b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Manish-Varma/fs-Improve-eventpoll-logging-to-stop-indicting-timerfd/20210401-101954
git checkout 49789288695d81f196d99fadd89b5101b26bca8b
# save the attached .config to linux build tree
make W=1 ARCH=um
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
cc1: warning: arch/um/include/uapi: No such file or directory [-Wmissing-include-dirs]
fs/eventpoll.c: In function 'ep_create_wakeup_source':
>> fs/eventpoll.c:1375:26: error: 'wakesource_create_id' undeclared (first use in this function); did you mean 'wakeup_source_create'?
1375 | id = atomic_inc_return(&wakesource_create_id);
| ^~~~~~~~~~~~~~~~~~~~
| wakeup_source_create
fs/eventpoll.c:1375:26: note: each undeclared identifier is reported only once for each function it appears in
vim +1375 fs/eventpoll.c
1357
1358 static int ep_create_wakeup_source(struct epitem *epi)
1359 {
1360 struct name_snapshot n;
1361 struct wakeup_source *ws;
1362 pid_t task_pid;
1363 char buf[64];
1364 int id;
1365
1366 task_pid = task_pid_nr(current);
1367
1368 if (!epi->ep->ws) {
1369 snprintf(buf, sizeof(buf), "epoll:%d", task_pid);
1370 epi->ep->ws = wakeup_source_register(NULL, buf);
1371 if (!epi->ep->ws)
1372 return -ENOMEM;
1373 }
1374
> 1375 id = atomic_inc_return(&wakesource_create_id);
1376 take_dentry_name_snapshot(&n, epi->ffd.file->f_path.dentry);
1377 snprintf(buf, sizeof(buf), "epollitem%d:%d.%s", id, task_pid, n.name.name);
1378 ws = wakeup_source_register(NULL, buf);
1379 release_dentry_name_snapshot(&n);
1380
1381 if (!ws)
1382 return -ENOMEM;
1383 rcu_assign_pointer(epi->ws, ws);
1384
1385 return 0;
1386 }
1387
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 14435 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] fs: Improve eventpoll logging to stop indicting timerfd
2021-04-01 2:28 ` Al Viro
@ 2021-04-01 23:57 ` Manish Varma
0 siblings, 0 replies; 4+ messages in thread
From: Manish Varma @ 2021-04-01 23:57 UTC (permalink / raw)
To: Al Viro
Cc: Thomas Gleixner, linux-fsdevel, linux-kernel, kernel-team,
Kelly Rossmoyer
Hi Al,
On Wed, Mar 31, 2021 at 7:29 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Wed, Mar 31, 2021 at 07:16:45PM -0700, Manish Varma wrote:
> > timerfd doesn't create any wakelocks, but eventpoll can. When it does,
> > it names them after the underlying file descriptor, and since all
> > timerfd file descriptors are named "[timerfd]" (which saves memory on
> > systems like desktops with potentially many timerfd instances), all
> > wakesources created as a result of using the eventpoll-on-timerfd idiom
> > are called... "[timerfd]".
> >
> > However, it becomes impossible to tell which "[timerfd]" wakesource is
> > affliated with which process and hence troubleshooting is difficult.
> >
> > This change addresses this problem by changing the way eventpoll
> > wakesources are named:
> >
> > 1) the top-level per-process eventpoll wakesource is now named "epoll:P"
> > (instead of just "eventpoll"), where P, is the PID of the creating
> > process.
> > 2) individual per-underlying-filedescriptor eventpoll wakesources are
> > now named "epollitemN:P.F", where N is a unique ID token and P is PID
> > of the creating process and F is the name of the underlying file
> > descriptor.
> >
> > All together that should be splitted up into a change to eventpoll and
> > timerfd (or other file descriptors).
>
> FWIW, it smells like a variant of wakeup_source_register() that would
> take printf format + arguments would be a good idea. I.e. something
> like
>
> > + snprintf(buf, sizeof(buf), "epoll:%d", task_pid);
> > + epi->ep->ws = wakeup_source_register(NULL, buf);
>
> ... = wakeup_source_register(NULL, "epoll:%d", task_pid);
>
> etc.
Noted. I will fix this in the v3 patch.
Thanks,
Manish
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-04-01 23:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-01 2:16 [PATCH v2] fs: Improve eventpoll logging to stop indicting timerfd Manish Varma
2021-04-01 2:28 ` Al Viro
2021-04-01 23:57 ` Manish Varma
2021-04-01 6:44 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).