public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [patch] make dnotify compile-time configurable
@ 2004-10-01  6:24 Robert Love
  2004-10-01 15:11 ` Matt Mackall
  0 siblings, 1 reply; 12+ messages in thread
From: Robert Love @ 2004-10-01  6:24 UTC (permalink / raw)
  To: ttb, akpm, mpm; +Cc: linux-kernel, gamin-list

[-- Attachment #1: Type: text/plain, Size: 916 bytes --]

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


[-- Attachment #2: dnotify-configurable-rml-1.patch --]
[-- Type: text/x-patch, Size: 6946 bytes --]

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

Signed-Off-By: Robert Love <rml@novell.com>

 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
--- linux-2.6.9-rc2/Documentation/dnotify.txt	2004-08-14 06:55:33.000000000 -0400
+++ linux/Documentation/dnotify.txt	2004-10-01 02:20:50.615450192 -0400
@@ -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
--- linux-2.6.9-rc2/fs/dnotify.c	2004-08-14 06:55:10.000000000 -0400
+++ linux/fs/dnotify.c	2004-10-01 02:04:01.264894800 -0400
@@ -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
--- linux-2.6.9-rc2/fs/Kconfig	2004-09-21 00:55:17.000000000 -0400
+++ linux/fs/Kconfig	2004-10-01 02:01:02.218114048 -0400
@@ -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
--- linux-2.6.9-rc2/fs/Makefile	2004-09-21 00:55:17.000000000 -0400
+++ linux/fs/Makefile	2004-10-01 02:00:57.627811880 -0400
@@ -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/dnotify.h linux/include/linux/dnotify.h
--- linux-2.6.9-rc2/include/linux/dnotify.h	2004-08-14 06:54:47.000000000 -0400
+++ linux/include/linux/dnotify.h	2004-10-01 01:56:48.868629024 -0400
@@ -1,3 +1,5 @@
+#ifndef _LINUX_DNOTIFY_H
+#define _LINUX_DNOTIFY_H
 /*
  * Directory notification for Linux
  *
@@ -8,20 +10,54 @@
 
 struct dnotify_struct {
 	struct dnotify_struct *	dn_next;
-	unsigned long		dn_mask;	/* Events to be notified
-						   see linux/fcntl.h */
+	unsigned long		dn_mask;
 	int			dn_fd;
 	struct file *		dn_filp;
 	fl_owner_t		dn_owner;
 };
 
+#ifdef __KERNEL__
+
+#include <linux/config.h>
+
+#ifdef CONFIG_DNOTIFY
+
 extern void __inode_dir_notify(struct inode *, unsigned long);
-extern void dnotify_flush(struct file *filp, fl_owner_t id);
+extern void dnotify_flush(struct file *, fl_owner_t);
 extern int fcntl_dirnotify(int, struct file *, unsigned long);
-void dnotify_parent(struct dentry *dentry, unsigned long event);
+extern void dnotify_parent(struct dentry *, unsigned long);
 
 static inline void inode_dir_notify(struct inode *inode, unsigned long event)
 {
-	if ((inode)->i_dnotify_mask & (event))
+	if (inode->i_dnotify_mask & (event))
 		__inode_dir_notify(inode, event);
 }
+
+#else
+
+static inline void __inode_dir_notify(struct inode *inode, unsigned long event)
+{
+}
+
+static inline void dnotify_flush(struct file *filp, fl_owner_t id)
+{
+}
+
+static inline int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg)
+{
+	return -EINVAL;
+}
+
+static inline void dnotify_parent(struct dentry *dentry, unsigned long event)
+{
+}
+
+static inline void inode_dir_notify(struct inode *inode, unsigned long event)
+{
+}
+
+#endif /* CONFIG_DNOTIFY */
+
+#endif /* __KERNEL __ */
+
+#endif /* _LINUX_DNOTIFY_H */
diff -urN linux-2.6.9-rc2/include/linux/fs.h linux/include/linux/fs.h
--- linux-2.6.9-rc2/include/linux/fs.h	2004-09-21 00:55:18.000000000 -0400
+++ linux/include/linux/fs.h	2004-10-01 02:04:15.245769384 -0400
@@ -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
--- linux-2.6.9-rc2/kernel/sysctl.c	2004-09-21 00:55:18.000000000 -0400
+++ linux/kernel/sysctl.c	2004-10-01 02:03:44.610426664 -0400
@@ -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,

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] make dnotify compile-time configurable
  2004-10-01  6:24 [patch] make dnotify compile-time configurable Robert Love
@ 2004-10-01 15:11 ` Matt Mackall
  2004-10-01 15:21   ` Robert Love
  0 siblings, 1 reply; 12+ messages in thread
From: Matt Mackall @ 2004-10-01 15:11 UTC (permalink / raw)
  To: Robert Love; +Cc: ttb, akpm, linux-kernel, gamin-list

On Fri, Oct 01, 2004 at 02:24:34AM -0400, Robert Love wrote:
> 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.

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

> Disabling CONFIG_DNOTIFY saves a couple hundred bytes off of vmlinux.

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

-- 
Mathematics is the supreme nostalgia of our time.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] make dnotify compile-time configurable
  2004-10-01 15:11 ` Matt Mackall
@ 2004-10-01 15:21   ` Robert Love
  2004-10-01 15:31     ` Randy.Dunlap
  0 siblings, 1 reply; 12+ messages in thread
From: Robert Love @ 2004-10-01 15:21 UTC (permalink / raw)
  To: Matt Mackall; +Cc: ttb, akpm, linux-kernel, gamin-list

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

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?

Thanks, Matt.

	Robert Love


[-- Attachment #2: dnotify-configurable-rml-2.patch --]
[-- Type: text/x-patch, Size: 7145 bytes --]

make dnotify configurable via CONFIG_DNOTIFY

Signed-Off-By: Robert Love <rml@novell.com>

 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
--- linux-inotify/Documentation/dnotify.txt	2004-09-30 12:09:42.919493656 -0400
+++ linux/Documentation/dnotify.txt	2004-10-01 11:15:38.727889256 -0400
@@ -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
--- linux-inotify/fs/dnotify.c	2004-09-30 12:09:42.488559168 -0400
+++ linux/fs/dnotify.c	2004-10-01 11:15:38.728889104 -0400
@@ -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
--- linux-inotify/fs/Kconfig	2004-09-30 12:09:42.754518736 -0400
+++ linux/fs/Kconfig	2004-10-01 11:15:38.739887432 -0400
@@ -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
--- linux-inotify/fs/Makefile	2004-09-30 12:09:42.755518584 -0400
+++ linux/fs/Makefile	2004-10-01 11:15:38.739887432 -0400
@@ -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/dnotify.h linux/include/linux/dnotify.h
--- linux-inotify/include/linux/dnotify.h	2004-09-30 12:09:40.019934456 -0400
+++ linux/include/linux/dnotify.h	2004-10-01 11:15:38.739887432 -0400
@@ -1,3 +1,5 @@
+#ifndef _LINUX_DNOTIFY_H
+#define _LINUX_DNOTIFY_H
 /*
  * Directory notification for Linux
  *
@@ -8,20 +10,54 @@
 
 struct dnotify_struct {
 	struct dnotify_struct *	dn_next;
-	unsigned long		dn_mask;	/* Events to be notified
-						   see linux/fcntl.h */
+	unsigned long		dn_mask;
 	int			dn_fd;
 	struct file *		dn_filp;
 	fl_owner_t		dn_owner;
 };
 
+#ifdef __KERNEL__
+
+#include <linux/config.h>
+
+#ifdef CONFIG_DNOTIFY
+
 extern void __inode_dir_notify(struct inode *, unsigned long);
-extern void dnotify_flush(struct file *filp, fl_owner_t id);
+extern void dnotify_flush(struct file *, fl_owner_t);
 extern int fcntl_dirnotify(int, struct file *, unsigned long);
-void dnotify_parent(struct dentry *dentry, unsigned long event);
+extern void dnotify_parent(struct dentry *, unsigned long);
 
 static inline void inode_dir_notify(struct inode *inode, unsigned long event)
 {
-	if ((inode)->i_dnotify_mask & (event))
+	if (inode->i_dnotify_mask & (event))
 		__inode_dir_notify(inode, event);
 }
+
+#else
+
+static inline void __inode_dir_notify(struct inode *inode, unsigned long event)
+{
+}
+
+static inline void dnotify_flush(struct file *filp, fl_owner_t id)
+{
+}
+
+static inline int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg)
+{
+	return -EINVAL;
+}
+
+static inline void dnotify_parent(struct dentry *dentry, unsigned long event)
+{
+}
+
+static inline void inode_dir_notify(struct inode *inode, unsigned long event)
+{
+}
+
+#endif /* CONFIG_DNOTIFY */
+
+#endif /* __KERNEL __ */
+
+#endif /* _LINUX_DNOTIFY_H */
diff -urN linux-inotify/include/linux/fs.h linux/include/linux/fs.h
--- linux-inotify/include/linux/fs.h	2004-09-30 12:09:40.036931872 -0400
+++ linux/include/linux/fs.h	2004-10-01 11:16:19.980617888 -0400
@@ -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
--- linux-inotify/kernel/sysctl.c	2004-09-30 12:09:42.926492592 -0400
+++ linux/kernel/sysctl.c	2004-10-01 11:15:38.750885760 -0400
@@ -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,

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] make dnotify compile-time configurable
  2004-10-01 15:21   ` Robert Love
@ 2004-10-01 15:31     ` Randy.Dunlap
  2004-10-01 15:44       ` Robert Love
  0 siblings, 1 reply; 12+ messages in thread
From: Randy.Dunlap @ 2004-10-01 15:31 UTC (permalink / raw)
  To: Robert Love; +Cc: mpm, ttb, akpm, linux-kernel, gamin-list

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] make dnotify compile-time configurable
  2004-10-01 15:31     ` Randy.Dunlap
@ 2004-10-01 15:44       ` Robert Love
  2004-10-01 15:58         ` Randy.Dunlap
  0 siblings, 1 reply; 12+ messages in thread
From: Robert Love @ 2004-10-01 15:44 UTC (permalink / raw)
  To: Randy.Dunlap; +Cc: mpm, ttb, akpm, linux-kernel, gamin-list

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.

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] make dnotify compile-time configurable
  2004-10-01 15:44       ` Robert Love
@ 2004-10-01 15:58         ` Randy.Dunlap
  2004-10-01 17:01           ` Robert Love
  0 siblings, 1 reply; 12+ messages in thread
From: Randy.Dunlap @ 2004-10-01 15:58 UTC (permalink / raw)
  To: Robert Love; +Cc: mpm, ttb, akpm, linux-kernel, gamin-list

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)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] make dnotify compile-time configurable
  2004-10-01 15:58         ` Randy.Dunlap
@ 2004-10-01 17:01           ` Robert Love
  2004-10-01 17:27             ` Matt Mackall
  0 siblings, 1 reply; 12+ messages in thread
From: Robert Love @ 2004-10-01 17:01 UTC (permalink / raw)
  To: Randy.Dunlap; +Cc: mpm, ttb, akpm, linux-kernel, gamin-list

On Fri, 2004-10-01 at 08:58 -0700, Randy.Dunlap wrote:

> 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.

In this case I offer A or A&B.

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

No patch I posted removes dnotify unconditionally.

	Robert Love



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] make dnotify compile-time configurable
  2004-10-01 17:01           ` Robert Love
@ 2004-10-01 17:27             ` Matt Mackall
  2004-10-01 17:30               ` Robert Love
  0 siblings, 1 reply; 12+ messages in thread
From: Matt Mackall @ 2004-10-01 17:27 UTC (permalink / raw)
  To: Robert Love; +Cc: Randy.Dunlap, ttb, akpm, linux-kernel, gamin-list

On Fri, Oct 01, 2004 at 01:01:55PM -0400, Robert Love wrote:
> On Fri, 2004-10-01 at 08:58 -0700, Randy.Dunlap wrote:
> 
> > 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.
> 
> 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..

-- 
Mathematics is the supreme nostalgia of our time.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] make dnotify compile-time configurable
  2004-10-01 17:27             ` Matt Mackall
@ 2004-10-01 17:30               ` Robert Love
  2004-10-01 17:30                 ` Randy.Dunlap
  0 siblings, 1 reply; 12+ messages in thread
From: Robert Love @ 2004-10-01 17:30 UTC (permalink / raw)
  To: Matt Mackall; +Cc: Randy.Dunlap, ttb, akpm, linux-kernel, gamin-list

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.

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] make dnotify compile-time configurable
  2004-10-01 17:30               ` Robert Love
@ 2004-10-01 17:30                 ` Randy.Dunlap
  2004-10-01 17:37                   ` Robert Love
  0 siblings, 1 reply; 12+ messages in thread
From: Randy.Dunlap @ 2004-10-01 17:30 UTC (permalink / raw)
  To: Robert Love; +Cc: mpm, ttb, akpm, linux-kernel, gamin-list

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] make dnotify compile-time configurable
  2004-10-01 17:30                 ` Randy.Dunlap
@ 2004-10-01 17:37                   ` Robert Love
  2004-10-01 17:38                     ` Randy.Dunlap
  0 siblings, 1 reply; 12+ messages in thread
From: Robert Love @ 2004-10-01 17:37 UTC (permalink / raw)
  To: Randy.Dunlap; +Cc: mpm, ttb, akpm, linux-kernel, gamin-list

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?

	Robert Love




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch] make dnotify compile-time configurable
  2004-10-01 17:37                   ` Robert Love
@ 2004-10-01 17:38                     ` Randy.Dunlap
  0 siblings, 0 replies; 12+ messages in thread
From: Randy.Dunlap @ 2004-10-01 17:38 UTC (permalink / raw)
  To: Robert Love; +Cc: mpm, ttb, akpm, linux-kernel, gamin-list

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2004-10-01 17:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-10-01  6:24 [patch] make dnotify compile-time configurable Robert Love
2004-10-01 15:11 ` Matt Mackall
2004-10-01 15:21   ` Robert Love
2004-10-01 15:31     ` Randy.Dunlap
2004-10-01 15:44       ` Robert Love
2004-10-01 15:58         ` Randy.Dunlap
2004-10-01 17:01           ` Robert Love
2004-10-01 17:27             ` Matt Mackall
2004-10-01 17:30               ` Robert Love
2004-10-01 17:30                 ` Randy.Dunlap
2004-10-01 17:37                   ` Robert Love
2004-10-01 17:38                     ` Randy.Dunlap

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox