2009-09-09 00:21:13

by Giuseppe Scrivano

[permalink] [raw]
Subject: extend inotify to support file descriptors in addition to paths

Hello,

at the moment inotify permits to add new files to be watched using their
path. There are situations where the file path is not know but a
descriptor is available. It would be desiderable to have the
possibility to use the inotify system even in these (rare) cases.

A concrete example of application that can take advantage of this
possibility is GNU tail. GNU tail, since version 7.5, is using inotify
to watch changes on files. This command will make `tail' sleeps until
some events are catched:

tail -qf foo bar baz

Inotify can't be used when stdin is watched too:

tail -qf boo bar - < baz

While the user gets the same result, internally it is a completely
different thing because polling is done on stdin instead of graciously
sleep until something is ready.
I don't know if it is possible to obtain the former result when the fd
is know but the path isn't.


My proposal is to extend inotify adding a new syscall:

long sys_inotify_add_watch_fd(int wd, int fd, u32 mask);

in this way the same mechanism can be used indifferently either a
descriptor or on a path is provided.

What do you think about this idea? Is there another way to get the same
result?

Regards,
Giuseppe


P.S.
In the attached patch I changed the syscalls table only for x86.

>From 6f3340347fb5d26000bec3a0a9cd8ef59c710922 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <[email protected]>
Date: Tue, 8 Sep 2009 23:40:03 +0200
Subject: [PATCH] Add new syscall \`inotify_add_watch_fd'.

It makes possible to register a file to be watched by inotify using
its file descriptor instead of its path.
---
arch/x86/kernel/syscall_table_32.S | 1 +
fs/notify/inotify/inotify_user.c | 54 ++++++++++++++++++++++++++++++++++--
include/linux/syscalls.h | 3 ++
3 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/syscall_table_32.S b/arch/x86/kernel/syscall_table_32.S
index d51321d..46eb4ac 100644
--- a/arch/x86/kernel/syscall_table_32.S
+++ b/arch/x86/kernel/syscall_table_32.S
@@ -336,3 +336,4 @@ ENTRY(sys_call_table)
.long sys_pwritev
.long sys_rt_tgsigqueueinfo /* 335 */
.long sys_perf_counter_open
+ .long sys_inotify_add_watch_fd
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index dcd2040..53f3c41 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -747,16 +747,64 @@ SYSCALL_DEFINE3(inotify_add_watch, int, fd, const char __user *, pathname,

/* create/update an inode mark */
ret = inotify_update_watch(group, inode, mask);
- if (unlikely(ret))
- goto path_put_and_out;

-path_put_and_out:
path_put(&path);
fput_and_out:
fput_light(filp, fput_needed);
return ret;
}

+SYSCALL_DEFINE3(inotify_add_watch_fd, int, wd, int, fd, u32, mask)
+{
+ struct fsnotify_group *group;
+ struct inode *inode;
+ struct path path;
+ struct file *filp;
+ struct file *filp2;
+ int ret, fput_needed, fput2_needed;
+ unsigned flags = 0;
+
+ filp = fget_light(wd, &fput_needed);
+ if (unlikely(!filp))
+ return -EBADF;
+
+ /* verify that this is indeed an inotify instance */
+ if (unlikely(filp->f_op != &inotify_fops)) {
+ ret = -EINVAL;
+ goto fput_and_out;
+ }
+
+ if (!(mask & IN_DONT_FOLLOW))
+ flags |= LOOKUP_FOLLOW;
+ if (mask & IN_ONLYDIR)
+ flags |= LOOKUP_DIRECTORY;
+
+ filp2 = fget_light(fd, &fput2_needed);
+
+ if (unlikely(!filp2)){
+ ret = -EBADF;
+ goto fput_and_out;
+ }
+
+ /* you can only watch an inode if you have read permissions on it */
+ ret = inode_permission(filp2->f_path.dentry->d_inode, MAY_READ);
+ if (ret)
+ goto fput2_and_out;
+
+ /* inode held in place by reference to path; group by fget on fd */
+ inode = filp2->f_path.dentry->d_inode;
+ group = filp->private_data;
+
+ /* create/update an inode mark */
+ ret = inotify_update_watch(group, inode, mask);
+
+fput2_and_out:
+ fput_light(filp2, fput2_needed);
+fput_and_out:
+ fput_light(filp, fput_needed);
+ return ret;
+}
+
SYSCALL_DEFINE2(inotify_rm_watch, int, fd, __s32, wd)
{
struct fsnotify_group *group;
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 80de700..0833b1d 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -761,4 +761,7 @@ int kernel_execve(const char *filename, char *const argv[], char *const envp[]);
asmlinkage long sys_perf_counter_open(
struct perf_counter_attr __user *attr_uptr,
pid_t pid, int cpu, int group_fd, unsigned long flags);
+
+
+asmlinkage long sys_inotify_add_watch_fd(int wd, int fd, u32 mask);
#endif
--
1.6.3.3


2009-09-10 14:27:24

by Eric Paris

[permalink] [raw]
Subject: Re: extend inotify to support file descriptors in addition to paths

On Tue, Sep 8, 2009 at 8:21 PM, Giuseppe Scrivano <[email protected]> wrote:

> at the moment inotify permits to add new files to be watched using their
> path. ?There are situations where the file path is not know but a
> descriptor is available. ?It would be desiderable to have the
> possibility to use the inotify system even in these (rare) cases.

I don't think specifying the inode in question by fd is fundamentally
a bad idea. It is the reason I decided to use fd's when registering
event's in the upcoming fanotify rather than pathnames. I do however
question if we really want to add yet another syscall for inotify.
We've already seen that inotify is very hard to expand. The fixed
message length, lack of information a number of users want, and
difficultly in extending those things make me reticent to support more
extentions.

Personally I'd rather see us/you move to fanotify which is (I hope)
extensible forever. If only I could get networking people to review
it. Have you looked at fanotify? I'm going to repost the series in a
couple minutes, maybe you could tell me if fanotify might work for
you?

2009-09-11 11:47:48

by Giuseppe Scrivano

[permalink] [raw]
Subject: Re: extend inotify to support file descriptors in addition to paths

Eric Paris <[email protected]> writes:

> On Tue, Sep 8, 2009 at 8:21 PM, Giuseppe Scrivano <[email protected]> wrote:
>
>> at the moment inotify permits to add new files to be watched using their
>> path.  There are situations where the file path is not know but a
>> descriptor is available.  It would be desiderable to have the
>> possibility to use the inotify system even in these (rare) cases.
>
> I don't think specifying the inode in question by fd is fundamentally
> a bad idea. It is the reason I decided to use fd's when registering
> event's in the upcoming fanotify rather than pathnames. I do however
> question if we really want to add yet another syscall for inotify.
> We've already seen that inotify is very hard to expand. The fixed
> message length, lack of information a number of users want, and
> difficultly in extending those things make me reticent to support more
> extentions.

> Personally I'd rather see us/you move to fanotify which is (I hope)
> extensible forever. If only I could get networking people to review
> it. Have you looked at fanotify? I'm going to repost the series in a
> couple minutes, maybe you could tell me if fanotify might work for
> you?

Sure, I'll take a look at it. I have few questions before look at
details: is it an attempt to replace inotify? Does fanotify use only
fd? Have the possibility to watch a file by its path, like inotify
does, is not a bad idea when the file is not already opened.

Thanks,
Giuseppe