Skip to contentSkip to conversation

make dnotify a configure-time option

PATCH In review Robert Love 2 participants 5 messages Oct 26, 2004

Subsystems (4)
Documentation VFS Proc sysctl Directory notification (dnotify)
lore
Share & tools
lore

Robert Love

author
#0 lore raw

PATCH 1/1 make dnotify a configure-time option

Following patch, against 2.6.9-mm1, makes dnotify a configure-time option via CONFIG_DNOTIFY.

The motivation is in anticipation of inotify, but making dnotify configurable makes sense regardless. A similar patch is also in 2.6-tiny.

Everything is straightforward, except the removal of the dir_notify_enable boot-time option. My contention is that there is no reason to make dnotify a boot-time option. But even if there is, then surely the user wants it always off, in which case the compile-time option fulfills the need.

By the way, straightforward is really long for a single word.

Tested with both dnotify on and off.

Best,

Robert Love

make dnotify a configure-time option, via CONFIG_DNOTIFY

 Documentation/dnotify.txt |    6 ++++++
 fs/Kconfig                |   11 +++++++++++
 fs/Makefile               |   13 +++++++------
 fs/dnotify.c              |    8 +-------
 include/linux/dnotify.h   |   46 +++++++++++++++++++++++++++++++++++++++++-----
 include/linux/fs.h        |    4 +++-
 kernel/sysctl.c           |    8 --------
 7 files changed, 69 insertions(+), 27 deletions(-)
Documentation/dnotify.txt +6 -0
diff -urN linux-2.6.9-mm1/Documentation/dnotify.txt linux/Documentation/dnotify.txt
@@ -54,6 +54,12 @@
 Also, files that are unlinked, will still cause notifications in the
 last directory that they were linked to.
 
+Configuration
+-------------
+
+Dnotify is controlled via the CONFIG_DNOTIFY configuration option.  When
+disabled, fcntl(fd, F_NOTIFY, ...) will return -EINVAL.
+
 Example
 -------
fs/dnotify.c +1 -7
diff -urN linux-2.6.9-mm1/fs/dnotify.c linux/fs/dnotify.c
@@ -13,6 +13,7 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * General Public License for more details.
  */
+
 #include <linux/fs.h>
 #include <linux/module.h>
 #include <linux/sched.h>

@@ -21,8 +22,6 @@
 #include <linux/spinlock.h>
 #include <linux/slab.h>
 
-int dir_notify_enable = 1;
-
 static kmem_cache_t *dn_cache;
 
 static void redo_inode_mask(struct inode *inode)

@@ -72,8 +71,6 @@
 		dnotify_flush(filp, id);
 		return 0;
 	}
-	if (!dir_notify_enable)
-		return -EINVAL;
 	inode = filp->f_dentry->d_inode;
 	if (!S_ISDIR(inode->i_mode))
 		return -ENOTDIR;

@@ -157,9 +154,6 @@
 {
 	struct dentry *parent;
 
-	if (!dir_notify_enable)
-		return;
-
 	spin_lock(&dentry->d_lock);
 	parent = dentry->d_parent;
 	if (parent->d_inode->i_dnotify_mask & event) {
fs/Kconfig +11 -0
diff -urN linux-2.6.9-mm1/fs/Kconfig linux/fs/Kconfig
@@ -440,6 +440,17 @@
 	depends on XFS_QUOTA || QUOTA
 	default y
 
+config DNOTIFY
+	bool "Dnotify support"
+	default y
+	help
+	  Dnotify is a directory-based per-fd file change notification system
+	  that uses signals to communicate events to user-space.  There exist
+	  superior alternatives, but some applications may still rely on
+	  dnotify.
+	  
+	  Because of this, if unsure, say Y.
+
 config AUTOFS_FS
 	tristate "Kernel automounter support"
 	help
fs/Makefile +7 -6
diff -urN linux-2.6.9-mm1/fs/Makefile linux/fs/Makefile
@@ -5,12 +5,11 @@
 # Rewritten to use lists instead of if-statements.
 # 
 
-obj-y :=	open.o read_write.o file_table.o buffer.o \
-		bio.o super.o block_dev.o char_dev.o stat.o exec.o pipe.o \
-		namei.o fcntl.o ioctl.o readdir.o select.o fifo.o locks.o \
-		dcache.o inode.o attr.o bad_inode.o file.o dnotify.o \
-		filesystems.o namespace.o seq_file.o xattr.o libfs.o \
-		fs-writeback.o mpage.o direct-io.o aio.o
+obj-y :=	open.o read_write.o file_table.o buffer.o  bio.o super.o \
+		block_dev.o char_dev.o stat.o exec.o pipe.o namei.o fcntl.o \
+		ioctl.o readdir.o select.o fifo.o locks.o dcache.o inode.o \
+		attr.o bad_inode.o file.o filesystems.o namespace.o aio.o \
+		seq_file.o xattr.o libfs.o fs-writeback.o mpage.o direct-io.o \
 
 obj-$(CONFIG_EPOLL)		+= eventpoll.o
 obj-$(CONFIG_COMPAT)		+= compat.o

@@ -37,6 +36,8 @@
 obj-$(CONFIG_QFMT_V2)		+= quota_v2.o
 obj-$(CONFIG_QUOTACTL)		+= quota.o
 
+obj-$(CONFIG_DNOTIFY)		+= dnotify.o
+
 obj-$(CONFIG_PROC_FS)		+= proc/
 obj-y				+= partitions/
 obj-$(CONFIG_SYSFS)		+= sysfs/
include/linux/dnotify.h +41 -5

64 lines not on this page · raw message

include/linux/fs.h +3 -1
diff -urN linux-2.6.9-mm1/include/linux/fs.h linux/include/linux/fs.h
@@ -61,7 +61,7 @@
 };
 extern struct inodes_stat_t inodes_stat;
 
-extern int leases_enable, dir_notify_enable, lease_break_time;
+extern int leases_enable, lease_break_time;
 
 #define NR_FILE  8192	/* this can well be larger on a larger system */
 #define NR_RESERVED_FILES 10 /* reserved for root */

@@ -464,8 +464,10 @@
 
 	__u32			i_generation;
 
+#ifdef CONFIG_DNOTIFY
 	unsigned long		i_dnotify_mask; /* Directory notify events */
 	struct dnotify_struct	*i_dnotify; /* for directory notifications */
+#endif
 
 	unsigned long		i_state;
 	unsigned long		dirtied_when;	/* jiffies of first dirtying */
kernel/sysctl.c +0 -8
diff -urN linux-2.6.9-mm1/kernel/sysctl.c linux/kernel/sysctl.c
@@ -903,14 +903,6 @@
 		.proc_handler	= &proc_dointvec,
 	},
 	{
-		.ctl_name	= FS_DIR_NOTIFY,
-		.procname	= "dir-notify-enable",
-		.data		= &dir_notify_enable,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= &proc_dointvec,
-	},
-	{
 		.ctl_name	= FS_LEASE_TIME,
 		.procname	= "lease-break-time",
 		.data		= &lease_break_time,
#1 lore raw
Quoted 6 lines · Robert Love

Everything is straightforward, except the removal of the dir_notify_enable boot-time option. My contention is that there is no reason to make dnotify a boot-time option. But even if there is, then surely the user wants it always off, in which case the compile-time option fulfills the need.

It's actually a sysctl and is also present in 2.4. I doubt if it's very useful but it seems a bit random to be deleting it now.

author
#2 lore raw
Quoted 2 lines · Andrew Morton

It's actually a sysctl and is also present in 2.4. I doubt if it's very useful but it seems a bit random to be deleting it now.

Well, let's not pretend that we care about interfaces outside of syscalls. Removing the sysctl simply results in a return of ENOTDIR from sysctl(2). Nothing should break.

But, yah, it is a bit random. OK. How about keeping the sysctl, but only when CONFIG_DNOTIFY=y?

Here is a version of the patch that leaves the sysctl in when dnotify is enabled, and removes it otherwise. Is this better?

Thanks,

Robert Love

make dnotify configurable, via CONFIG_DNOTIFY.

 Documentation/dnotify.txt |    6 ++++++
 fs/Kconfig                |   11 +++++++++++
 fs/Makefile               |   13 +++++++------
 include/linux/dnotify.h   |   46 +++++++++++++++++++++++++++++++++++++++++-----
 include/linux/fs.h        |    8 +++++++-
 kernel/sysctl.c           |    2 ++
 6 files changed, 74 insertions(+), 12 deletions(-)
Documentation/dnotify.txt +6 -0
diff -urN linux-2.6.9-mm1/Documentation/dnotify.txt linux/Documentation/dnotify.txt
@@ -54,6 +54,12 @@
 Also, files that are unlinked, will still cause notifications in the
 last directory that they were linked to.
 
+Configuration
+-------------
+
+Dnotify is controlled via the CONFIG_DNOTIFY configuration option.  When
+disabled, fcntl(fd, F_NOTIFY, ...) will return -EINVAL.
+
 Example
 -------
fs/Kconfig +11 -0
diff -urN linux-2.6.9-mm1/fs/Kconfig linux/fs/Kconfig
@@ -440,6 +440,17 @@
 	depends on XFS_QUOTA || QUOTA
 	default y
 
+config DNOTIFY
+	bool "Dnotify support"
+	default y
+	help
+	  Dnotify is a directory-based per-fd file change notification system
+	  that uses signals to communicate events to user-space.  There exist
+	  superior alternatives, but some applications may still rely on
+	  dnotify.
+	  
+	  Because of this, if unsure, say Y.
+
 config AUTOFS_FS
 	tristate "Kernel automounter support"
 	help
fs/Makefile +7 -6
diff -urN linux-2.6.9-mm1/fs/Makefile linux/fs/Makefile
@@ -5,12 +5,11 @@
 # Rewritten to use lists instead of if-statements.
 # 
 
-obj-y :=	open.o read_write.o file_table.o buffer.o \
-		bio.o super.o block_dev.o char_dev.o stat.o exec.o pipe.o \
-		namei.o fcntl.o ioctl.o readdir.o select.o fifo.o locks.o \
-		dcache.o inode.o attr.o bad_inode.o file.o dnotify.o \
-		filesystems.o namespace.o seq_file.o xattr.o libfs.o \
-		fs-writeback.o mpage.o direct-io.o aio.o
+obj-y :=	open.o read_write.o file_table.o buffer.o  bio.o super.o \
+		block_dev.o char_dev.o stat.o exec.o pipe.o namei.o fcntl.o \
+		ioctl.o readdir.o select.o fifo.o locks.o dcache.o inode.o \
+		attr.o bad_inode.o file.o filesystems.o namespace.o aio.o \
+		seq_file.o xattr.o libfs.o fs-writeback.o mpage.o direct-io.o \
 
 obj-$(CONFIG_EPOLL)		+= eventpoll.o
 obj-$(CONFIG_COMPAT)		+= compat.o

@@ -37,6 +36,8 @@
 obj-$(CONFIG_QFMT_V2)		+= quota_v2.o
 obj-$(CONFIG_QUOTACTL)		+= quota.o
 
+obj-$(CONFIG_DNOTIFY)		+= dnotify.o
+
 obj-$(CONFIG_PROC_FS)		+= proc/
 obj-y				+= partitions/
 obj-$(CONFIG_SYSFS)		+= sysfs/
include/linux/dnotify.h +41 -5

64 lines not on this page · raw message

include/linux/fs.h +7 -1
diff -urN linux-2.6.9-mm1/include/linux/fs.h linux/include/linux/fs.h
@@ -61,7 +61,11 @@
 };
 extern struct inodes_stat_t inodes_stat;
 
-extern int leases_enable, dir_notify_enable, lease_break_time;
+extern int leases_enable, lease_break_time;
+
+#ifdef CONFIG_DNOTIFY
+extern int dir_notify_enable;
+#endif
 
 #define NR_FILE  8192	/* this can well be larger on a larger system */
 #define NR_RESERVED_FILES 10 /* reserved for root */

@@ -464,8 +468,10 @@
 
 	__u32			i_generation;
 
+#ifdef CONFIG_DNOTIFY
 	unsigned long		i_dnotify_mask; /* Directory notify events */
 	struct dnotify_struct	*i_dnotify; /* for directory notifications */
+#endif
 
 	unsigned long		i_state;
 	unsigned long		dirtied_when;	/* jiffies of first dirtying */
kernel/sysctl.c +2 -0
diff -urN linux-2.6.9-mm1/kernel/sysctl.c linux/kernel/sysctl.c
@@ -902,6 +902,7 @@
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec,
 	},
+#ifdef CONFIG_DNOTIFY
 	{
 		.ctl_name	= FS_DIR_NOTIFY,
 		.procname	= "dir-notify-enable",

@@ -910,6 +911,7 @@
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec,
 	},
+#endif
 	{
 		.ctl_name	= FS_LEASE_TIME,
 		.procname	= "lease-break-time",
#3 lore raw
Quoted 2 lines · Robert Love

make dnotify configurable, via CONFIG_DNOTIFY.

I don't think we want people accidentally disabling dnotify and making kernels which don't implement the standard kernel API. So...

fs/Kconfig renamed from fs/Kconfig~make-dnotify-a-configure-time-option-embedded +1 -1
diff -puN fs/Kconfig~make-dnotify-a-configure-time-option-embedded fs/Kconfig
@@ -441,7 +441,7 @@ config QUOTACTL
 	default y
 
 config DNOTIFY
-	bool "Dnotify support"
+	bool "Dnotify support" if EMBEDDED
 	default y
 	help
 	  Dnotify is a directory-based per-fd file change notification system

_

author
#4 lore raw
Quoted 2 lines · Andrew Morton

I don't think we want people accidentally disabling dnotify and making kernels which don't implement the standard kernel API. So...

Fine with me.

Thanks!

Robert Love