make dnotify compile-time configurable

Attached patch makes dnotify compile-time configurable via CONFIG_DNOTIFY. The patch stands alone from the inotify patch, although it really makes most sense in that context. Maybe the tiny kernel will find it useful as well.

On a desktop system with inotify, there is no reason to enable dnotify support. The primary user of dnotify (FAM) already supports inotify (via Gamin). Many non-desktop systems do not use dnotify.

There was a sysctl, dir_notify_enable, for run-time configuration of dnotify. I removed it. It makes no sense to me, even without compile-time configuration, to disable this feature. With the ability to compile dnotify away, a sysctl makes even less sense (which, as it made zero sense before, makes no sense in and of itself now).

Disabling CONFIG_DNOTIFY saves a couple hundred bytes off of vmlinux.

When disabled, fcntl(fd, F_NOTIFY, ...) returns -EINVAL.

Best,

Robert Love

make dnotify compile-time configurable via CONFIG_DNOTIFY because, well, dnotify sucks

 Documentation/dnotify.txt |    8 ++++++++
 fs/Kconfig                |   12 ++++++++++++
 fs/Makefile               |   13 +++++++------
 fs/dnotify.c              |    8 +-------
 include/linux/dnotify.h   |   46 +++++++++++++++++++++++++++++++++++++++++-----
 include/linux/fs.h        |    2 +-
 kernel/sysctl.c           |    8 --------
 7 files changed, 70 insertions(+), 27 deletions(-)

diff -urN linux-2.6.9-rc2/Documentation/dnotify.txt linux/Documentation/dnotify.txt
@@ -54,6 +54,14 @@
 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.
+
+Dnotify is deprecated in favor of inotify (CONFIG_INOTIFY).
+
 Example
 -------
 

diff -urN linux-2.6.9-rc2/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) {

diff -urN linux-2.6.9-rc2/fs/Kconfig linux/fs/Kconfig
@@ -438,6 +438,18 @@
 	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.  It has
+	  been replaced by inotify (see CONFIG_INOTIFY), which solves many of
+	  the shortcomings of dnotify and adds new features, but some
+	  applications may still rely on dnotify.
+	  
+	  Because of this, if unsure, say Y.
+
 config AUTOFS_FS
 	tristate "Kernel automounter support"
 	help

diff -urN linux-2.6.9-rc2/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/

diff -urN linux-2.6.9-rc2/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 */

diff -urN linux-2.6.9-rc2/kernel/sysctl.c linux/kernel/sysctl.c
@@ -879,14 +879,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,

Indeed, it's been useful for months. Unfortunately my development boxes are still mothballed so progress upstream is stalled.

Hmm, thought I saved at least 1 or 2k.

Mathematics is the supreme nostalgia of our time.

Oh, great...

/me finds your broken out patches ...

Ah, you ifdef'ed out the dnotify variables in the inode structure. That was the original reason I did this--doh.

In mine, I remove the sysctl entirely, which you don't. I also add some documentation to dnotify.txt. Not a big difference.

I thought my number sounded suspect. Dunno.

Here is a respun patch ifdef'ing out the dnotify member variables in struct inode. This one is diff'ed against my inode tree since the struct inode changes break one path or the other and, well, I like to mix things up.

John, want to merge this into the inotify patch?

Thanks, Matt.

Robert Love

make dnotify configurable via CONFIG_DNOTIFY

 Documentation/dnotify.txt |    8 ++++++++
 fs/Kconfig                |   12 ++++++++++++
 fs/Makefile               |   13 +++++++------
 fs/dnotify.c              |    8 +-------
 include/linux/dnotify.h   |   46 +++++++++++++++++++++++++++++++++++++++++-----
 include/linux/fs.h        |    4 +++-
 kernel/sysctl.c           |    8 --------
 7 files changed, 72 insertions(+), 27 deletions(-)

diff -urN linux-inotify/Documentation/dnotify.txt linux/Documentation/dnotify.txt
@@ -54,6 +54,14 @@
 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.
+
+Dnotify is deprecated in favor of inotify (CONFIG_INOTIFY).
+
 Example
 -------
 

diff -urN linux-inotify/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) {

diff -urN linux-inotify/fs/Kconfig linux/fs/Kconfig
@@ -438,6 +438,18 @@
 	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.  It has
+	  been replaced by inotify (see CONFIG_INOTIFY), which solves many of
+	  the shortcomings of dnotify and adds new features, but some
+	  applications may still rely on dnotify.
+	  
+	  Because of this, if unsure, say Y.
+
 config AUTOFS_FS
 	tristate "Kernel automounter support"
 	help

diff -urN linux-inotify/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/

diff -urN linux-inotify/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 */

@@ -459,8 +459,10 @@
 
 	__u32			i_generation;
 
+#ifdef CONFIG_DNOTIFY
 	unsigned long		i_dnotify_mask; /* Directory notify events */
 	struct dnotify_struct	*i_dnotify; /* for directory notifications */
+#endif
 
 #ifdef CONFIG_INOTIFY
 	struct list_head	watchers;

diff -urN linux-inotify/kernel/sysctl.c linux/kernel/sysctl.c
@@ -879,14 +879,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,

On Fri, 01 Oct 2004 11:21:16 -0400 Robert Love wrote:

| On Fri, 2004-10-01 at 10:11 -0500, Matt Mackall wrote:
| 
| > Indeed, it's been useful for months. Unfortunately my development
| > boxes are still mothballed so progress upstream is stalled.
| 
| Oh, great...
| 
| /me finds your broken out patches ...
| 
| Ah, you ifdef'ed out the dnotify variables in the inode structure.  That
| was the original reason I did this--doh.
| 
| In mine, I remove the sysctl entirely, which you don't.  I also add some
| documentation to dnotify.txt.  Not a big difference.
| 
| > > Disabling CONFIG_DNOTIFY saves a couple hundred bytes off of vmlinux.
| > 
| > Hmm, thought I saved at least 1 or 2k.
| 
| I thought my number sounded suspect.  Dunno.
| 
| Here is a respun patch ifdef'ing out the dnotify member variables in
| struct inode.  This one is diff'ed against my inode tree since the
| struct inode changes break one path or the other and, well, I like to
| mix things up.
| 
| John, want to merge this into the inotify patch?

I'd rather see inotify additions and dnotify config options kept separate. They may serve a similar purpose, but inotify doesn't replace the dnotify API. If the latter were true, combining them would make sense IMO.

| Thanks, Matt.

~Randy

I'm not really following.

Whether or not dnotify is a configuration option is separate, and could go into the kernel either way.

But what matters if our inotify patch also carries the change? People with inotify definitely DO want this patch, because they don't need dnotify. Not much uses dnotify--it is a pain to use--and inotify replaces its functionality.

It is also a practical move: the diffs conflict.

Robert Love

On Fri, 01 Oct 2004 11:44:39 -0400 Robert Love wrote:

| On Fri, 2004-10-01 at 08:31 -0700, Randy.Dunlap wrote:
| 
| > I'd rather see inotify additions and dnotify config options kept
| > separate.  They may serve a similar purpose, but inotify doesn't
| > replace the dnotify API.  If the latter were true, combining
| > them would make sense IMO.
| 
| I'm not really following.
| 
| Whether or not dnotify is a configuration option is separate, and could
| go into the kernel either way.

Sorry, that's about all that I was trying to say. If patches A & B are logically separate, don't combine them. Nothing new there.

| But what matters if our inotify patch also carries the change?  People
| with inotify definitely DO want this patch, because they don't need
| dnotify.  Not much uses dnotify--it is a pain to use--and inotify
| replaces its functionality.

Well, the patch shouldn't remove dnotify unconditionally, or not until we have that elusive stable kernel series that people keep mentioning elsewhere.

| It is also a practical move: the diffs conflict.

I see.

~Randy
MOTD:  Always include version info.
(Again.  Sometimes I think ln -s /usr/src/linux/.config .signature)

In this case I offer A or A&B.

No patch I posted removes dnotify unconditionally.

Robert Love

The configurable dnotify patch makes sense on its own and is perhaps less contentious. Push it first and resolve the conflicts with inotify later..

Mathematics is the supreme nostalgia of our time.

..the A above is dnotify by itself. All I said--and I don't know why anyone questioned it--is that I want to put dnotify's configurability in the inotify patch. They make sense together, and the patches conflict anyways. I can do this.

CONFIG_DNOTIFY makes sense either way, on its own or (more so) with inotify, and I already posted the patch separately.

Why are we even talking about this?!?

Robert Love

On Fri, 01 Oct 2004 13:30:08 -0400 Robert Love wrote:

| On Fri, 2004-10-01 at 12:27 -0500, Matt Mackall wrote:
| 
| > > In this case I offer A or A&B.
| > 
| > The configurable dnotify patch makes sense on its own and is perhaps
| > less contentious. Push it first and resolve the conflicts with inotify
| > later..
| 
| ..the A above is dnotify by itself.  All I said--and I don't know why
| anyone questioned it--is that I want to put dnotify's configurability in
| the inotify patch.  They make sense together, and the patches conflict
| anyways.  I can do this.

You can do that. Go ahead.
Even if it isn't clear that they make sense together.
Lots of patches conflict, but that's no grand reason to combine them.

| CONFIG_DNOTIFY makes sense either way, on its own or (more so) with
| inotify, and I already posted the patch separately.
| 
| Why are we even talking about this?!?

I'm going quiet on it...

~Randy

It is not clear that users of inotify probably don't need dnotify?

Robert Love

On Fri, 01 Oct 2004 13:37:38 -0400 Robert Love wrote:

| On Fri, 2004-10-01 at 10:30 -0700, Randy.Dunlap wrote:
| 
| > You can do that.  Go ahead.
| > Even if it isn't clear that they make sense together.
| 
| It is not clear that users of inotify probably don't need dnotify?

I expect that much is clear.

It is not clear that there are only applications on the system which use inotify and that there are none that use dnotify.

I wanted this to end, too.

~Randy