diff mbox series

[resend] eventfd: make eventfd files distinguishable in /proc/$PID/fd

Message ID 20181209183546.5550-1-yamato@redhat.com (mailing list archive)
State New, archived
Headers show
Series [resend] eventfd: make eventfd files distinguishable in /proc/$PID/fd | expand

Commit Message

Masatake YAMATO Dec. 9, 2018, 6:35 p.m. UTC
Finding endpoints of an IPC channel is one of essential task to
understand how a user program works. Procfs and netlink socket provide
enough hints to find endpoints for IPC channels like pipes, unix
sockets, and pseudo terminals. However, there is no simple way to find
endpoints for an eventfd file from userland. An inode number doesn't
hint. Unlike pipe, all eventfd files shares one inode object.

To provide the way to find endpoints of an eventfd file, this patch
adds eventfd identifiers to the output of 'ls -l /proc/$pid/fd' like:

  ...
  lrwx------. 1 qemu qemu 64 May 20 04:49 93 -> 'anon_inode:[eventfd:130]'
  lrwx------. 1 qemu qemu 64 May 20 04:49 94 -> 'anon_inode:[eventfd:131]'
  ...

Here "130" and "131" are added as identifiers newly added.
In the case that ida_simple_get returns an error, this change doesn't add
an identifier; just use "[eventfd]" as before.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
---
 fs/eventfd.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Serge E. Hallyn Dec. 11, 2018, 11:09 p.m. UTC | #1
On Mon, Dec 10, 2018 at 03:35:46AM +0900, Masatake YAMATO wrote:
> Finding endpoints of an IPC channel is one of essential task to
> understand how a user program works. Procfs and netlink socket provide
> enough hints to find endpoints for IPC channels like pipes, unix
> sockets, and pseudo terminals. However, there is no simple way to find
> endpoints for an eventfd file from userland. An inode number doesn't
> hint. Unlike pipe, all eventfd files shares one inode object.
> 
> To provide the way to find endpoints of an eventfd file, this patch
> adds eventfd identifiers to the output of 'ls -l /proc/$pid/fd' like:
> 
>   ...
>   lrwx------. 1 qemu qemu 64 May 20 04:49 93 -> 'anon_inode:[eventfd:130]'
>   lrwx------. 1 qemu qemu 64 May 20 04:49 94 -> 'anon_inode:[eventfd:131]'
>   ...
> 
> Here "130" and "131" are added as identifiers newly added.
> In the case that ida_simple_get returns an error, this change doesn't add
> an identifier; just use "[eventfd]" as before.
> 
> Signed-off-by: Masatake YAMATO <yamato@redhat.com>

I'm going to love this when I need it :)  Thanks.

Acked-by: Serge Hallyn <serge@hallyn.com>

> ---
>  fs/eventfd.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/eventfd.c b/fs/eventfd.c
> index 08d3bd602f73..c18952948110 100644
> --- a/fs/eventfd.c
> +++ b/fs/eventfd.c
> @@ -21,6 +21,11 @@
>  #include <linux/eventfd.h>
>  #include <linux/proc_fs.h>
>  #include <linux/seq_file.h>
> +#include <linux/idr.h>
> +
> +/* Worst case buffer size needed for holding an integer. */
> +#define ITOA_MAX_LEN 12
> +DEFINE_IDA(eventfd_ida);
>  
>  struct eventfd_ctx {
>  	struct kref kref;
> @@ -35,6 +40,7 @@ struct eventfd_ctx {
>  	 */
>  	__u64 count;
>  	unsigned int flags;
> +	int id;
>  };
>  
>  /**
> @@ -69,6 +75,8 @@ EXPORT_SYMBOL_GPL(eventfd_signal);
>  
>  static void eventfd_free_ctx(struct eventfd_ctx *ctx)
>  {
> +	if (ctx->id >= 0)
> +		ida_simple_remove(&eventfd_ida, ctx->id);
>  	kfree(ctx);
>  }
>  
> @@ -384,6 +392,7 @@ static int do_eventfd(unsigned int count, int flags)
>  {
>  	struct eventfd_ctx *ctx;
>  	int fd;
> +	char name[1 + 8 + ITOA_MAX_LEN + 1 + 1] = "[eventfd]";
>  
>  	/* Check the EFD_* constants for consistency.  */
>  	BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
> @@ -400,8 +409,11 @@ static int do_eventfd(unsigned int count, int flags)
>  	init_waitqueue_head(&ctx->wqh);
>  	ctx->count = count;
>  	ctx->flags = flags;
> +	ctx->id = ida_simple_get(&eventfd_ida, 0, 0, GFP_KERNEL);
>  
> -	fd = anon_inode_getfd("[eventfd]", &eventfd_fops, ctx,
> +	if (ctx->id >= 0)
> +		snprintf(name, sizeof(name), "[eventfd:%d]", ctx->id);
> +	fd = anon_inode_getfd(name, &eventfd_fops, ctx,
>  			      O_RDWR | (flags & EFD_SHARED_FCNTL_FLAGS));
>  	if (fd < 0)
>  		eventfd_free_ctx(ctx);
> -- 
> 2.17.0
Masatake YAMATO Dec. 12, 2018, 7:57 a.m. UTC | #2
On Tue, 11 Dec 2018 17:09:14 -0600, "Serge E. Hallyn" <serge@hallyn.com> wrote:
> On Mon, Dec 10, 2018 at 03:35:46AM +0900, Masatake YAMATO wrote:
>> Finding endpoints of an IPC channel is one of essential task to
>> understand how a user program works. Procfs and netlink socket provide
>> enough hints to find endpoints for IPC channels like pipes, unix
>> sockets, and pseudo terminals. However, there is no simple way to find
>> endpoints for an eventfd file from userland. An inode number doesn't
>> hint. Unlike pipe, all eventfd files shares one inode object.
>> 
>> To provide the way to find endpoints of an eventfd file, this patch
>> adds eventfd identifiers to the output of 'ls -l /proc/$pid/fd' like:
>> 
>>   ...
>>   lrwx------. 1 qemu qemu 64 May 20 04:49 93 -> 'anon_inode:[eventfd:130]'
>>   lrwx------. 1 qemu qemu 64 May 20 04:49 94 -> 'anon_inode:[eventfd:131]'
>>   ...
>> 
>> Here "130" and "131" are added as identifiers newly added.
>> In the case that ida_simple_get returns an error, this change doesn't add
>> an identifier; just use "[eventfd]" as before.
>> 
>> Signed-off-by: Masatake YAMATO <yamato@redhat.com>
> 
> I'm going to love this when I need it :)  Thanks.
> 
> Acked-by: Serge Hallyn <serge@hallyn.com>
>

Thank you for replying.
This is the first time getting replying since my first post,
Mon, 21 May 2018 05:18:23 +0900.

Can I expect my patch is merged to the mainline?
Or should I do something more?

Masatake YAMATO

>> ---
>>  fs/eventfd.c | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>> 
>> diff --git a/fs/eventfd.c b/fs/eventfd.c
>> index 08d3bd602f73..c18952948110 100644
>> --- a/fs/eventfd.c
>> +++ b/fs/eventfd.c
>> @@ -21,6 +21,11 @@
>>  #include <linux/eventfd.h>
>>  #include <linux/proc_fs.h>
>>  #include <linux/seq_file.h>
>> +#include <linux/idr.h>
>> +
>> +/* Worst case buffer size needed for holding an integer. */
>> +#define ITOA_MAX_LEN 12
>> +DEFINE_IDA(eventfd_ida);
>>  
>>  struct eventfd_ctx {
>>  	struct kref kref;
>> @@ -35,6 +40,7 @@ struct eventfd_ctx {
>>  	 */
>>  	__u64 count;
>>  	unsigned int flags;
>> +	int id;
>>  };
>>  
>>  /**
>> @@ -69,6 +75,8 @@ EXPORT_SYMBOL_GPL(eventfd_signal);
>>  
>>  static void eventfd_free_ctx(struct eventfd_ctx *ctx)
>>  {
>> +	if (ctx->id >= 0)
>> +		ida_simple_remove(&eventfd_ida, ctx->id);
>>  	kfree(ctx);
>>  }
>>  
>> @@ -384,6 +392,7 @@ static int do_eventfd(unsigned int count, int flags)
>>  {
>>  	struct eventfd_ctx *ctx;
>>  	int fd;
>> +	char name[1 + 8 + ITOA_MAX_LEN + 1 + 1] = "[eventfd]";
>>  
>>  	/* Check the EFD_* constants for consistency.  */
>>  	BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
>> @@ -400,8 +409,11 @@ static int do_eventfd(unsigned int count, int flags)
>>  	init_waitqueue_head(&ctx->wqh);
>>  	ctx->count = count;
>>  	ctx->flags = flags;
>> +	ctx->id = ida_simple_get(&eventfd_ida, 0, 0, GFP_KERNEL);
>>  
>> -	fd = anon_inode_getfd("[eventfd]", &eventfd_fops, ctx,
>> +	if (ctx->id >= 0)
>> +		snprintf(name, sizeof(name), "[eventfd:%d]", ctx->id);
>> +	fd = anon_inode_getfd(name, &eventfd_fops, ctx,
>>  			      O_RDWR | (flags & EFD_SHARED_FCNTL_FLAGS));
>>  	if (fd < 0)
>>  		eventfd_free_ctx(ctx);
>> -- 
>> 2.17.0
diff mbox series

Patch

diff --git a/fs/eventfd.c b/fs/eventfd.c
index 08d3bd602f73..c18952948110 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -21,6 +21,11 @@ 
 #include <linux/eventfd.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
+#include <linux/idr.h>
+
+/* Worst case buffer size needed for holding an integer. */
+#define ITOA_MAX_LEN 12
+DEFINE_IDA(eventfd_ida);
 
 struct eventfd_ctx {
 	struct kref kref;
@@ -35,6 +40,7 @@  struct eventfd_ctx {
 	 */
 	__u64 count;
 	unsigned int flags;
+	int id;
 };
 
 /**
@@ -69,6 +75,8 @@  EXPORT_SYMBOL_GPL(eventfd_signal);
 
 static void eventfd_free_ctx(struct eventfd_ctx *ctx)
 {
+	if (ctx->id >= 0)
+		ida_simple_remove(&eventfd_ida, ctx->id);
 	kfree(ctx);
 }
 
@@ -384,6 +392,7 @@  static int do_eventfd(unsigned int count, int flags)
 {
 	struct eventfd_ctx *ctx;
 	int fd;
+	char name[1 + 8 + ITOA_MAX_LEN + 1 + 1] = "[eventfd]";
 
 	/* Check the EFD_* constants for consistency.  */
 	BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
@@ -400,8 +409,11 @@  static int do_eventfd(unsigned int count, int flags)
 	init_waitqueue_head(&ctx->wqh);
 	ctx->count = count;
 	ctx->flags = flags;
+	ctx->id = ida_simple_get(&eventfd_ida, 0, 0, GFP_KERNEL);
 
-	fd = anon_inode_getfd("[eventfd]", &eventfd_fops, ctx,
+	if (ctx->id >= 0)
+		snprintf(name, sizeof(name), "[eventfd:%d]", ctx->id);
+	fd = anon_inode_getfd(name, &eventfd_fops, ctx,
 			      O_RDWR | (flags & EFD_SHARED_FCNTL_FLAGS));
 	if (fd < 0)
 		eventfd_free_ctx(ctx);