fsnotify/inotify split
Subsystems (8)
cover fsnotify/inotify split
The following two patches simply split fsnotify from inotify. There should be no functional change to inotify at all. Perhaps the split will help identify the interface bits that can easily converge for inotify and audit. They're completely untested... I started with inotify-45 in 2.6.13-rc2-mm1, and worked against base 2.6.13-rc2.
My first pass was trivial split, this is my second pass which moves dnotify and inotify functionality out of fsnotify.h into their respective corners. I'm sure audit can at least use the hooks, and perhaps more base inode watch functionality could be shared (at no cost to you! ;-) moving forward.
thanks,
-chris
PATCH 1/2 fsnotify
Add fsnotify as infrastructure for various fs notifcation schemes. Move dnotify to fsnotify.
fs/attr.c | 33 +------------ fs/compat.c | 12 +++- fs/file_table.c | 3 + fs/namei.c | 30 ++++++----- fs/nfsd/vfs.c | 6 +- fs/open.c | 3 - fs/read_write.c | 15 +++-- fs/sysfs/file.c | 7 -- fs/xattr.c | 5 + include/linux/dnotify.h | 101 ++++++++++++++++++++++++++++++++++++++++ include/linux/fs.h | 1 include/linux/fsnotify.h | 118 +++++++++++++++++++++++++++++++++++++++++++++++ 12 files changed, 270 insertions(+), 64 deletions(-)
fs/attr.c +4 -29
@@ -10,7 +10,7 @@#include <linux/mm.h> #include <linux/string.h> #include <linux/smp_lock.h> -#include <linux/dnotify.h> +#include <linux/fsnotify.h> #include <linux/fcntl.h> #include <linux/quotaops.h> #include <linux/security.h>
@@ -107,31 +107,8 @@ int inode_setattr(struct inode * inode, out:return error; } -EXPORT_SYMBOL (inode_setattr); -int setattr_mask (unsigned int ia_valid) -{ -unsigned long dn_mask =0 ; - -if (ia_valid &ATTR_UID ) - dn_mask |=DN_ATTRIB ; -if (ia_valid &ATTR_GID ) - dn_mask |=DN_ATTRIB ; -if (ia_valid &ATTR_SIZE ) - dn_mask |=DN_MODIFY ; -/* both times implies a utime(s) call */ -if ((ia_valid & (ATTR_ATIME |ATTR_MTIME )) == (ATTR_ATIME |ATTR_MTIME )) - dn_mask |=DN_ATTRIB ; -else if (ia_valid &ATTR_ATIME ) - dn_mask |=DN_ACCESS ; -else if (ia_valid &ATTR_MTIME ) - dn_mask |=DN_MODIFY ; -if (ia_valid &ATTR_MODE ) - dn_mask |=DN_ATTRIB ; -return dn_mask; -} -int notify_change (struct dentry * dentry,struct iattr * attr) {struct inode *inode = dentry->d_inode;
@@ -197,11 +174,9 @@ int notify_change(struct dentry * dentryif (ia_valid &ATTR_SIZE )up_write (&dentry->d_inode->i_alloc_sem); -if (!error) { -unsigned long dn_mask =setattr_mask (ia_valid); -if (dn_mask) -dnotify_parent (dentry, dn_mask); - } +if (!error) +fsnotify_change (dentry, ia_valid); +return error; }
fs/compat.c +8 -4
@@ -37,7 +37,7 @@#include <linux/ctype.h> #include <linux/module.h> #include <linux/dirent.h> -#include <linux/dnotify.h> +#include <linux/fsnotify.h> #include <linux/highuid.h> #include <linux/sunrpc/svc.h> #include <linux/nfsd/nfsd.h>
@@ -1307,9 +1307,13 @@ static ssize_t compat_do_readv_writev(in out:if (iov != iovstack)kfree (iov); -if ((ret + (type ==READ )) >0 ) -dnotify_parent (file->f_dentry, - (type ==READ ) ?DN_ACCESS :DN_MODIFY ); +if ((ret + (type ==READ )) >0 ) { +struct dentry *dentry = file->f_dentry; +if (type ==READ ) +fsnotify_access (dentry); +else +fsnotify_modify (dentry); + }return ret; }
fs/file_table.c +3 -0
@@ -16,6 +16,7 @@#include <linux/eventpoll.h> #include <linux/mount.h> #include <linux/cdev.h> +#include <linux/fsnotify.h> /* sysctl tunables... */ struct files_stat_struct files_stat = {
@@ -126,6 +127,8 @@ void fastcall __fput(struct file *file)struct inode *inode = dentry->d_inode;might_sleep (); + +fsnotify_close (file);/* * The function eventpoll_release() should be the first called * in the file cleanup chain.
fs/namei.c +16 -14
@@ -21,7 +21,7 @@#include <linux/namei.h> #include <linux/quotaops.h> #include <linux/pagemap.h> -#include <linux/dnotify.h> +#include <linux/fsnotify.h> #include <linux/smp_lock.h> #include <linux/personality.h> #include <linux/security.h>
@@ -1312,7 +1312,7 @@ int vfs_create(struct inode *dir, structDQUOT_INIT (dir); error = dir->i_op->create (dir, dentry, mode, nd);if (!error) { -inode_dir_notify (dir,DN_CREATE ); +fsnotify_create (dir, dentry->d_name.name);security_inode_post_create (dir, dentry, mode); }return error;
@@ -1637,7 +1637,7 @@ int vfs_mknod(struct inode *dir, structDQUOT_INIT (dir); error = dir->i_op->mknod (dir, dentry, mode, dev);if (!error) { -inode_dir_notify (dir,DN_CREATE ); +fsnotify_create (dir, dentry->d_name.name);security_inode_post_mknod (dir, dentry, mode, dev); }return error;
@@ -1710,7 +1710,7 @@ int vfs_mkdir(struct inode *dir, structDQUOT_INIT (dir); error = dir->i_op->mkdir (dir, dentry, mode);if (!error) { -inode_dir_notify (dir,DN_CREATE ); +fsnotify_mkdir (dir, dentry->d_name.name);security_inode_post_mkdir (dir,dentry, mode); }return error;
@@ -1801,7 +1801,7 @@ int vfs_rmdir(struct inode *dir, struct }up (&dentry->d_inode->i_sem);if (!error) { -inode_dir_notify (dir,DN_DELETE ); +fsnotify_rmdir (dentry, dentry->d_inode, dir);d_delete (dentry); }dput (dentry);
@@ -1874,9 +1874,10 @@ int vfs_unlink(struct inode *dir, struct/* We don't d_delete() NFS sillyrenamed files--they still exist. */ if (!error && !(dentry->d_flags &DCACHE_NFSFS_RENAMED )) { +fsnotify_unlink (dentry, dir);d_delete (dentry); -inode_dir_notify (dir,DN_DELETE ); } +return error; }
@@ -1950,7 +1951,7 @@ int vfs_symlink(struct inode *dir, strucDQUOT_INIT (dir); error = dir->i_op->symlink (dir, dentry, oldname);if (!error) { -inode_dir_notify (dir,DN_CREATE ); +fsnotify_create (dir, dentry->d_name.name);security_inode_post_symlink (dir, dentry, oldname); }return error;
@@ -2023,7 +2024,7 @@ int vfs_link(struct dentry *old_dentry, error = dir->i_op->link (old_dentry, dir, new_dentry);up (&old_dentry->d_inode->i_sem);if (!error) { -inode_dir_notify (dir,DN_CREATE ); +fsnotify_create (dir, new_dentry->d_name.name);security_inode_post_link (old_dentry, dir, new_dentry); }return error;
@@ -2187,6 +2188,7 @@ int vfs_rename(struct inode *old_dir, st {int error;int is_dir =S_ISDIR (old_dentry->d_inode->i_mode); +const char *old_name;if (old_dentry->d_inode == new_dentry->d_inode)return 0 ;
@@ -2208,18 +2210,18 @@ int vfs_rename(struct inode *old_dir, stDQUOT_INIT (old_dir);DQUOT_INIT (new_dir); + old_name =fsnotify_oldname_init (old_dentry->d_name.name); +if (is_dir) error =vfs_rename_dir (old_dir,old_dentry,new_dir,new_dentry);else error =vfs_rename_other (old_dir,old_dentry,new_dir,new_dentry);if (!error) { -if (old_dir == new_dir) -inode_dir_notify (old_dir,DN_RENAME ); -else { -inode_dir_notify (old_dir,DN_DELETE ); -inode_dir_notify (new_dir,DN_CREATE ); - } +const char *new_name = old_dentry->d_name.name; +fsnotify_move (old_dir, new_dir, old_name, new_name, is_dir); } +fsnotify_oldname_free (old_name); +return error; }
fs/nfsd/vfs.c +3 -3
@@ -45,7 +45,7 @@#endif /* CONFIG_NFSD_V3 */ #include <linux/nfsd/nfsfh.h> #include <linux/quotaops.h> -#include <linux/dnotify.h> +#include <linux/fsnotify.h> #include <linux/posix_acl.h> #include <linux/posix_acl_xattr.h> #ifdef CONFIG_NFSD_V4
@@ -860,7 +860,7 @@ nfsd_vfs_read(struct svc_rqst *rqstp, st nfsdstats.io_read += err; *count = err; err =0 ; -dnotify_parent (file->f_dentry,DN_ACCESS ); +fsnotify_access (file->f_dentry); }else err =nfserrno (err); out:
@@ -916,7 +916,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, sset_fs (oldfs);if (err >=0 ) { nfsdstats.io_write += cnt; -dnotify_parent (file->f_dentry,DN_MODIFY ); +fsnotify_modify (file->f_dentry); }/* clear setuid/setgid flag after write */
fs/open.c +2 -1
@@ -10,7 +10,7 @@#include <linux/file.h> #include <linux/smp_lock.h> #include <linux/quotaops.h> -#include <linux/dnotify.h> +#include <linux/fsnotify.h> #include <linux/module.h> #include <linux/slab.h> #include <linux/tty.h>
@@ -951,6 +951,7 @@ asmlinkage long sys_open(const char __usput_unused_fd (fd); fd =PTR_ERR (f); }else { +fsnotify_open (f->f_dentry);fd_install (fd, f); } }
fs/read_write.c +9 -6
@@ -10,7 +10,7 @@#include <linux/file.h> #include <linux/uio.h> #include <linux/smp_lock.h> -#include <linux/dnotify.h> +#include <linux/fsnotify.h> #include <linux/security.h> #include <linux/module.h> #include <linux/syscalls.h>
@@ -252,7 +252,7 @@ ssize_t vfs_read(struct file *file, charelse ret =do_sync_read (file, buf, count, pos);if (ret >0 ) { -dnotify_parent (file->f_dentry,DN_ACCESS ); +fsnotify_access (file->f_dentry); current->rchar += ret; } current->syscr++;
@@ -303,7 +303,7 @@ ssize_t vfs_write(struct file *file, conelse ret =do_sync_write (file, buf, count, pos);if (ret >0 ) { -dnotify_parent (file->f_dentry,DN_MODIFY ); +fsnotify_modify (file->f_dentry); current->wchar += ret; } current->syscw++;
@@ -539,9 +539,12 @@ static ssize_t do_readv_writev(int type, out:if (iov != iovstack)kfree (iov); -if ((ret + (type ==READ )) >0 ) -dnotify_parent (file->f_dentry, - (type ==READ ) ?DN_ACCESS :DN_MODIFY ); +if ((ret + (type ==READ )) >0 ) { +if (type ==READ ) +fsnotify_access (file->f_dentry); +else +fsnotify_modify (file->f_dentry); + }return ret; Efault: ret = -EFAULT ;
fs/sysfs/file.c +2 -5
@@ -3,7 +3,7 @@*/ #include <linux/module.h> -#include <linux/dnotify.h> +#include <linux/fsnotify.h> #include <linux/kobject.h> #include <linux/namei.h> #include <asm/uaccess.h>
@@ -391,9 +391,6 @@ int sysfs_create_file(struct kobject * k* sysfs_update_file - update the modified timestamp on an object attribute. * @kobj: object we're acting for. * @attr: attribute descriptor. -* -* Also call dnotify for the dentry, which lots of userspace programs -* use. */ int sysfs_update_file (struct kobject * kobj,const struct attribute * attr) {
@@ -408,7 +405,7 @@ int sysfs_update_file(struct kobject * kif (victim->d_inode && (victim->d_parent->d_inode == dir->d_inode)) { victim->d_inode->i_mtime =CURRENT_TIME ; -dnotify_parent (victim,DN_MODIFY ); +fsnotify_modify (victim);/** * Drop reference from initial sysfs_get_dentry().
fs/xattr.c +4 -1
@@ -16,6 +16,7 @@#include <linux/security.h> #include <linux/syscalls.h> #include <linux/module.h> +#include <linux/fsnotify.h> #include <asm/uaccess.h> /*
@@ -57,8 +58,10 @@ setxattr(struct dentry *d, char __user *if (error) goto out; error = d->d_inode->i_op->setxattr (d, kname, kvalue, size, flags); -if (!error) +if (!error) { +fsnotify_xattr (d);security_inode_post_setxattr (d, kname, kvalue, size, flags); + } out:up (&d->d_inode->i_sem); }
include/linux/fs.h +0 -1
@@ -1393,7 +1393,6 @@ extern void emergency_remount(void);extern int do_remount_sb (struct super_block *sb,int flags,void *data,int force);extern sector_t bmap (struct inode *,sector_t ); -extern int setattr_mask (unsigned int );extern int notify_change (struct dentry *,struct iattr *);extern int permission (struct inode *,int ,struct nameidata *); extern int generic_permission(struct inode *, int,
include/linux/fsnotify.h new file +118 -0
118 lines not on this page · raw message
include/linux/dnotify.h +101 -0
109 lines not on this page · raw message
PATCH 2/2 inotify
inotify
Documentation/filesystems/inotify.txt | 155 +++++ fs/Kconfig | 13 fs/Makefile | 1 fs/inode.c | 6 fs/inotify.c | 1011 ++++++++++++++++++++++++++++++++++ include/linux/fs.h | 5 include/linux/fsnotify.h | 34 + include/linux/inotify.h | 301 ++++++++++ include/linux/sched.h | 4 kernel/user.c | 4 10 files changed, 1534 insertions(+)
Documentation/filesystems/inotify.txt new file +155 -0
@@ -0,0 +1,155 @@ + inotify + a powerful yet simple file change notification system + + + +Document started 15 Mar 2005 by Robert Love <[email protected]> + +(i) User Interface + +Inotify is controlled by a device node, /dev/inotify. If you do not use udev, +this device may need to be created manually. + +First step in using inotify is to open the device file: + + int dev_fd = open ("/dev/inotify", O_RDONLY); + +Change events are managed by "watches". A watch is an (object,mask) pair where +the object is a file or directory and the mask is a bitmask of one or more +inotify events that the application wishes to receive. See <linux/inotify.h> +for valid events. A watch is referenced by a watch descriptor, or wd. + +Watches are added via a file descriptor to the file. + +Watches on a directory will return events on any files inside of the directory. + +Adding a watch is simple, + + /* 'wd' represents the watch on fd with mask */ + struct inotify_request req = { fd, mask }; + int wd = ioctl (dev_fd, INOTIFY_WATCH, &req); + +You can add a large number of files via something like + + for each file to watch { + struct inotify_request req; + int file_fd; + + file_fd = open (file, O_RDONLY); + if (fd < 0) { + perror ("open"); + break; + } + + req.fd = file_fd; + req.mask = mask; + + wd = ioctl (dev_fd, INOTIFY_WATCH, &req); + + close (fd); + } + +You can update an existing watch in the same manner, by passing in a new mask. + +An existing watch is removed via the INOTIFY_IGNORE ioctl, for example + + ioctl (dev_fd, INOTIFY_IGNORE, wd); + +Events are provided in the form of an inotify_event structure that is read(2) +from /dev/inotify. The filename is of dynamic length and follows the struct. +It is of size len. The filename is padded with null bytes to ensure proper +alignment. This padding is reflected in len. + +You can slurp multiple events by passing a large buffer, for example + + size_t len = read (fd, buf, BUF_LEN); + +Will return as many events as are available and fit in BUF_LEN. + +/dev/inotify is also select()- and poll()-able. + +You can find the size of the current event queue via the FIONREAD ioctl. + +All watches are destroyed and cleaned up on close. + + +(ii) Internal Kernel Implementation + +Each open inotify device is associated with an inotify_device structure. + +Each watch is associated with an inotify_watch structure. Watches are chained +off of each associated device and each associated inode. + +See fs/inotify.c for the locking and lifetime rules. + + +(iii) Rationale + +Q: What is the design decision behind not tying the watch to the open fd of + the watched object? + +A: Watches are associated with an open inotify device, not an open file. + This solves the primary problem with dnotify: keeping the file open pins + the file and thus, worse, pins the mount. Dnotify is therefore infeasible + for use on a desktop system with removable media as the media cannot be + unmounted. + +Q: What is the design decision behind using an-fd-per-device as opposed to + an fd-per-watch? + +A: An fd-per-watch quickly consumes more file descriptors than are allowed, + more fd's than are feasible to manage, and more fd's than are optimally + select()-able. Yes, root can bump the per-process fd limit and yes, users + can use epoll, but requiring both is a silly and extraneous requirement. + A watch consumes less memory than an open file, separating the number + spaces is thus sensible. The current design is what user-space developers + want: Users open the device, once, and add n watches, requiring but one fd + and no twiddling with fd limits. Opening /dev/inotify two thousand times + is silly. If we can implement user-space's preferences cleanly--and we + can, the idr layer makes stuff like this trivial--then we should. + + There are other good arguments. With a single fd, there is a single + item to block on, which is mapped to a single queue of events. The single + fd returns all watch events and also any potential out-of-band data. If + every fd was a separate watch, + + - There would be no way to get event ordering. Events on file foo and + file bar would pop poll() on both fd's, but there would be no way to tell + which happened first. A single queue trivially gives you ordering. Such + ordering is crucial to existing applications such as Beagle. Imagine + "mv a b ; mv b a" events without ordering. + + - We'd have to maintain n fd's and n internal queues with state, + versus just one. It is a lot messier in the kernel. A single, linear + queue is the data structure that makes sense. + + - User-space developers prefer the current API. The Beagle guys, for + example, love it. Trust me, I asked. It is not a surprise: Who'd want + to manage and block on 1000 fd's via select? + + - You'd have to manage the fd's, as an example: Call close() when you + received a delete event. + + - No way to get out of band data. + + - 1024 is still too low. ;-) + + When you talk about designing a file change notification system that + scales to 1000s of directories, juggling 1000s of fd's just does not seem + the right interface. It is too heavy. + +Q: Why a device node? + +A: The poor user-space interface is the second biggest problem with dnotify. + Signals are a terrible, terrible interface for file notification. Or for + anything, for that matter. The idea solution, from all perspectives, is a + file descriptor-based one that allows basic file I/O and poll/select. + Obtaining the fd and managing the watches could have been done either via a + device file or a family of new system calls. We decided to implement a + device file because adding three or four new system calls that mirrored + open, close, and ioctl seemed silly. A character device makes sense from + user-space and was easy to implement inside of the kernel. + + Additionally, it _is_ possible to open /dev/inotify more than once and + juggle more than one queue and thus more than one associated fd. +
fs/inode.c +6 -0
@@ -21,6 +21,7 @@#include <linux/pagemap.h> #include <linux/cdev.h> #include <linux/bootmem.h> +#include <linux/inotify.h> /* * This is needed for the following functions:
@@ -202,6 +203,10 @@ void inode_init_once(struct inode *inodeINIT_LIST_HEAD (&inode->i_data.i_mmap_nonlinear);spin_lock_init (&inode->i_lock);i_size_ordered_init (inode); +#ifdef CONFIG_INOTIFY +INIT_LIST_HEAD (&inode->inotify_watches); +sema_init (&inode->inotify_sem,1 ); +#endif }EXPORT_SYMBOL (inode_init_once);
@@ -346,6 +351,7 @@ int invalidate_inodes(struct super_blockdown (&iprune_sem);spin_lock (&inode_lock); +inotify_unmount_inodes (&sb->s_inodes); busy =invalidate_list (&sb->s_inodes, &throw_away);spin_unlock (&inode_lock);
fs/inotify.c new file +1011 -0
1,011 lines not on this page · raw message
fs/Kconfig +13 -0
@@ -356,6 +356,19 @@ config ROMFS_FS If you don't know whether you need it, then you don't need it: answer N. +config INOTIFY + bool "Inotify file change notification support" + default y + ---help--- + Say Y here to enable inotify support and the /dev/inotify character + device. Inotify is a file change notification system and a + replacement for dnotify. Inotify fixes numerous shortcomings in + dnotify and introduces several new features. It allows monitoring + of both files and directories via a single open fd. Multiple file + events are supported. + + If unsure, say Y. + config QUOTA bool "Quota support" help
fs/Makefile +1 -0
@@ -12,6 +12,7 @@ obj-y := open.o read_write.o file_table. seq_file.o xattr.o libfs.o fs-writeback.o mpage.o direct-io.o \ ioprio.o +obj-$(CONFIG_INOTIFY) += inotify.o obj-$(CONFIG_EPOLL) += eventpoll.o obj-$(CONFIG_COMPAT) += compat.o
include/linux/fs.h +5 -0
@@ -474,6 +474,11 @@ struct inode {struct dnotify_struct *i_dnotify;/* for directory notifications */ #endif +#ifdef CONFIG_INOTIFY +struct list_head inotify_watches;/* watches on this inode */ +struct semaphore inotify_sem;/* protects the watches list */ +#endif +unsigned long i_state;unsigned long dirtied_when;/* jiffies of first dirtying */
include/linux/inotify.h new file +301 -0
301 lines not on this page · raw message
include/linux/sched.h +4 -0
@@ -410,6 +410,10 @@ struct user_struct {atomic_t processes;/* How many processes does this user have? */ atomic_t files;/* How many open files does this user have? */ atomic_t sigpending;/* How many pending signals does this user have? */ +#ifdef CONFIG_INOTIFY +atomic_t inotify_watches;/* How many inotify watches does this user have? */ +atomic_t inotify_devs;/* How many inotify devs does this user have opened? */ +#endif /* protected by mq_lock */ unsigned long mq_bytes;/* How many bytes can be allocated to mqueue? */ unsigned long locked_shm;/* How many pages of mlocked shm ? */
kernel/user.c +4 -0
@@ -120,6 +120,10 @@ struct user_struct * alloc_uid(uid_t uidatomic_set (&new->processes,0 );atomic_set (&new->files,0 );atomic_set (&new->sigpending,0 ); +#ifdef CONFIG_INOTIFY +atomic_set (&new->inotify_watches,0 ); +atomic_set (&new->inotify_devs,0 ); +#endif new->mq_bytes =0 ; new->locked_shm =0 ;
include/linux/fsnotify.h +34 -0
@@ -14,6 +14,7 @@#ifdef __KERNEL__#include <linux/dnotify.h> +#include <linux/inotify.h> /* * fsnotify_move - file old_name at old_dir was moved to new_name at new_dir
@@ -23,6 +24,7 @@ static inline void fsnotify_move(struct int isdir) {dnotify_move (old_dir, new_dir, old_name, new_name, isdir); +inotify_move (old_dir, new_dir, old_name, new_name, isdir); }/*
@@ -31,6 +33,7 @@ static inline void fsnotify_move(structstatic inline void fsnotify_unlink (struct dentry *dentry,struct inode *dir) {dnotify_unlink (dentry, dir); +inotify_unlink (dentry, dir); }/*
@@ -40,6 +43,7 @@ static inline void fsnotify_rmdir(struct struct inode *dir) {dnotify_rmdir (dentry, inode, dir); +inotify_rmdir (dentry, inode, dir); }/*
@@ -48,6 +52,7 @@ static inline void fsnotify_rmdir(structstatic inline void fsnotify_create (struct inode *inode,const char *name) {dnotify_create (inode, name); +inotify_create (inode, name); }/*
@@ -56,6 +61,7 @@ static inline void fsnotify_create(strucstatic inline void fsnotify_mkdir (struct inode *inode,const char *name) {dnotify_mkdir (inode, name); +inotify_mkdir (inode, name); }/*
@@ -64,6 +70,7 @@ static inline void fsnotify_mkdir(structstatic inline void fsnotify_access (struct dentry *dentry) {dnotify_access (dentry); +inotify_access (dentry); }/*
@@ -72,6 +79,7 @@ static inline void fsnotify_access(strucstatic inline void fsnotify_modify (struct dentry *dentry) {dnotify_modify (dentry); +inotify_modify (dentry); }/*
@@ -79,6 +87,7 @@ static inline void fsnotify_modify(struc*/ static inline void fsnotify_open (struct dentry *dentry) { +inotify_open (dentry); }/*
@@ -86,6 +95,7 @@ static inline void fsnotify_open(struct*/ static inline void fsnotify_close (struct file *file) { +inotify_close (file); }/*
@@ -93,6 +103,7 @@ static inline void fsnotify_close(struct*/ static inline void fsnotify_xattr (struct dentry *dentry) { +inotify_xattr (dentry); }/*
@@ -102,8 +113,29 @@ static inline void fsnotify_xattr(structstatic inline void fsnotify_change (struct dentry *dentry,unsigned int ia_valid) {dnotify_change (dentry, ia_valid); +inotify_change (dentry, ia_valid); } +#ifdef CONFIG_INOTIFY /* inotify helpers */ + +/* +* fsnotify_oldname_init - save off the old filename before we change it +*/ +static inline const char *fsnotify_oldname_init (const char *name) +{ +return kstrdup (name,GFP_KERNEL ); +} + +/* +* snotify_oldname_free - free the name we got from fsnotify_oldname_init +*/ +static inline void fsnotify_oldname_free (const char *old_name) +{ +kfree (old_name); +} + +#else /* CONFIG_INOTIFY */ +static inline const char *fsnotify_oldname_init (const char *name) {return NULL ;
@@ -113,6 +145,8 @@ static inline void fsnotify_oldname_free { } +#endif /* ! CONFIG_INOTIFY */ +#endif /* __KERNEL__ */ #endif /* _LINUX_FS_NOTIFY_H */
Re: [RFC/PATCH 1/2] fsnotify
In reply to Chris WrightQuoted 2 lines · Chris Wright
Add fsnotify as infrastructure for various fs notifcation schemes. Move dnotify to fsnotify.
- inode_dir_notify(dir, DN_CREATE);
+ fsnotify_create(dir, new_dentry->d_name.name);
....
+ * We don't compile any of this away in some complicated menagerie of ifdefs.
+ * Instead, we rely on the code inside to optimize away as needed.
....
+static inline void fsnotify_create(struct inode *inode, const char *name) +{
+ dnotify_create(inode, name);
+}
....
+static inline void dnotify_create(struct inode *inode, const char *name) +{
+ inode_dir_notify(inode, DN_CREATE);
+}
To be honest, I don't really see that this is in any way better than what we had before. Yes, two different pieces of code actually use hooks in similar places in the VFS code. But this 'infrastructure' just to share those hooks is overkill as far as I can tell. It really isn't any better than having both inotify and audit hooks side by side where we can actually see what's going on at a glance. In fact, it's worse.
What would make sense, perhaps, would be to actually merge those hooks; not just a cosmetic amalgamation of the calling sites. Currently, each of inotify and the audit code does its own filtering when its hooks are triggered, and then acts upon the event only if it affects a watched inode.
To actually merge that filtering code would make sense -- and then both inotify and auditfs would just register watches on certain inodes and receive notification as appropriate.
There are features of each which would probably be desirable in the merged result. The inotify version grows the inode structure by quite a lot, while the auditfs version recognises that watches will be relatively infrequent, so uses only a bit in i_flags as a quickly-accessible indication that an inode is 'interesting' and maintains its actual data for that inode elsewhere. The auditfs version is also capable of handling the "Child named 'XXXX' of this inode" kind of watch, where watches are placed on objects which don't yet exist. Also, the auditfs code already calls back to a separate function auditfs_attach_wdata() in the _real_ audit code, which actually reports when the watch is triggered; it shouldn't be hard to make it call into an inotify function instead, depending on the type of watch which is hit.
On the other hand, the inotify triggers require a little more information from the original hook, which audit_notify_watch() would need to pass through if the audit code were used as the basis for a merged 'core watch' functionality.
Signature · 1 line
dwmw2
Re: [RFC/PATCH 1/2] fsnotify
Quoted 6 lines · David Woodhouse
To be honest, I don't really see that this is in any way better than what we had before. Yes, two different pieces of code actually use hooks in similar places in the VFS code. But this 'infrastructure' just to share those hooks is overkill as far as I can tell. It really isn't any better than having both inotify and audit hooks side by side where we can actually see what's going on at a glance. In fact, it's worse.
I think what makes this patch look superfluous is that Chris added a set of wrappers for dnotify, too.
In the inotify patch, the fsnotify wrappers call directly into the inotify and dnotify interfaces and they do consolidate code and clean things up. I added fsnotify at hch's request.
Now that audit is coming along, fsnotify makes even more sense.
I would like to share some more code at a lower level, though, as you pointed out.
I planned to look at redoing dnotify entirely on top of inotify, once inotify is in the kernel proper, for example.
Robert Love
Re: [RFC/PATCH 1/2] fsnotify
In reply to David WoodhouseQuoted 5 lines · * David Woodhouse ([email protected]) wrote:
What would make sense, perhaps, would be to actually merge those hooks; not just a cosmetic amalgamation of the calling sites. Currently, each of inotify and the audit code does its own filtering when its hooks are triggered, and then acts upon the event only if it affects a watched inode.
That's exactly what my intention was by posting this. The hook site is just enough to get the conversation going. The really useful bits to merge are at inode watch level.
thanks,
-chris