mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/3] send notification events about exec
@ 2008-12-15 16:44 Eric Paris
  2008-12-15 16:44 ` [PATCH 1/3] filesystem notification: create fs/notify to contain all fs notification Eric Paris
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Eric Paris @ 2008-12-15 16:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: hch, akpm

The following series does two things.  First it pushes inotify and dnotify
down into subdirectores inside fs/notify (help clean up fs.)  It then adds a
new fsnotify hook for exec functions to call into and uses that new hookd to
send notifications to inotify and dnotify.

---

Eric Paris (3):
      fsnotify: use the new open-exec hook for inotify and dnotify
      fsnotify: sys_execve and sys_uselib do not call into fsnotify
      filesystem notification: create fs/notify to contain all fs notification


 fs/Kconfig                       |   39 --
 fs/Makefile                      |    5 
 fs/dnotify.c                     |  194 --------
 fs/exec.c                        |    5 
 fs/inotify.c                     |  913 --------------------------------------
 fs/inotify_user.c                |  778 --------------------------------
 fs/notify/Kconfig                |    2 
 fs/notify/Makefile               |    2 
 fs/notify/dnotify/Kconfig        |   10 
 fs/notify/dnotify/Makefile       |    1 
 fs/notify/dnotify/dnotify.c      |  194 ++++++++
 fs/notify/inotify/Kconfig        |   27 +
 fs/notify/inotify/Makefile       |    2 
 fs/notify/inotify/inotify.c      |  913 ++++++++++++++++++++++++++++++++++++++
 fs/notify/inotify/inotify_user.c |  778 ++++++++++++++++++++++++++++++++
 include/linux/fsnotify.h         |   13 +
 16 files changed, 1949 insertions(+), 1927 deletions(-)
 delete mode 100644 fs/dnotify.c
 delete mode 100644 fs/inotify.c
 delete mode 100644 fs/inotify_user.c
 create mode 100644 fs/notify/Kconfig
 create mode 100644 fs/notify/Makefile
 create mode 100644 fs/notify/dnotify/Kconfig
 create mode 100644 fs/notify/dnotify/Makefile
 create mode 100644 fs/notify/dnotify/dnotify.c
 create mode 100644 fs/notify/inotify/Kconfig
 create mode 100644 fs/notify/inotify/Makefile
 create mode 100644 fs/notify/inotify/inotify.c
 create mode 100644 fs/notify/inotify/inotify_user.c

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

* [PATCH 1/3] filesystem notification: create fs/notify to contain all fs notification
  2008-12-15 16:44 [PATCH 0/3] send notification events about exec Eric Paris
@ 2008-12-15 16:44 ` Eric Paris
  2008-12-16  1:02   ` KOSAKI Motohiro
  2008-12-15 16:44 ` [PATCH 2/3] fsnotify: sys_execve and sys_uselib do not call into fsnotify Eric Paris
  2008-12-15 16:44 ` [PATCH 3/3] fsnotify: use the new open-exec hook for inotify and dnotify Eric Paris
  2 siblings, 1 reply; 11+ messages in thread
From: Eric Paris @ 2008-12-15 16:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: hch, akpm

Creating a generic filesystem notification interface, fsnotify, which will be
used by inotify, dnotify, and eventually fanotify is really starting to
clutter the fs directory.  This patch simply moves inotify and dnotify into
fs/notify/inotify and fs/notify/dnotify respectively.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 fs/Kconfig                       |   39 --
 fs/Makefile                      |    5 
 fs/dnotify.c                     |  194 --------
 fs/inotify.c                     |  913 --------------------------------------
 fs/inotify_user.c                |  778 --------------------------------
 fs/notify/Kconfig                |    2 
 fs/notify/Makefile               |    2 
 fs/notify/dnotify/Kconfig        |   10 
 fs/notify/dnotify/Makefile       |    1 
 fs/notify/dnotify/dnotify.c      |  194 ++++++++
 fs/notify/inotify/Kconfig        |   27 +
 fs/notify/inotify/Makefile       |    2 
 fs/notify/inotify/inotify.c      |  913 ++++++++++++++++++++++++++++++++++++++
 fs/notify/inotify/inotify_user.c |  778 ++++++++++++++++++++++++++++++++
 14 files changed, 1931 insertions(+), 1927 deletions(-)
 delete mode 100644 fs/dnotify.c
 delete mode 100644 fs/inotify.c
 delete mode 100644 fs/inotify_user.c
 create mode 100644 fs/notify/Kconfig
 create mode 100644 fs/notify/Makefile
 create mode 100644 fs/notify/dnotify/Kconfig
 create mode 100644 fs/notify/dnotify/Makefile
 create mode 100644 fs/notify/dnotify/dnotify.c
 create mode 100644 fs/notify/inotify/Kconfig
 create mode 100644 fs/notify/inotify/Makefile
 create mode 100644 fs/notify/inotify/inotify.c
 create mode 100644 fs/notify/inotify/inotify_user.c

diff --git a/fs/Kconfig b/fs/Kconfig
index 522469a..ff0e819 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -270,44 +270,7 @@ config OCFS2_COMPAT_JBD
 
 endif # BLOCK
 
-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.
-
-	  If unsure, say Y.
-
-config INOTIFY
-	bool "Inotify file change notification support"
-	default y
-	---help---
-	  Say Y here to enable inotify support.  Inotify is a file change
-	  notification system and a replacement for dnotify.  Inotify fixes
-	  numerous shortcomings in dnotify and introduces several new features
-	  including multiple file events, one-shot support, and unmount
-	  notification.
-
-	  For more information, see <file:Documentation/filesystems/inotify.txt>
-
-	  If unsure, say Y.
-
-config INOTIFY_USER
-	bool "Inotify support for userspace"
-	depends on INOTIFY
-	default y
-	---help---
-	  Say Y here to enable inotify support for userspace, including the
-	  associated system calls.  Inotify allows monitoring of both files and
-	  directories via a single open fd.  Events are read from the file
-	  descriptor, which is also select()- and poll()-able.
-
-	  For more information, see <file:Documentation/filesystems/inotify.txt>
-
-	  If unsure, say Y.
+source "fs/notify/Kconfig"
 
 config QUOTA
 	bool "Quota support"
diff --git a/fs/Makefile b/fs/Makefile
index d9f8afe..e6f423d 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -20,8 +20,7 @@ obj-y +=	no-block.o
 endif
 
 obj-$(CONFIG_BLK_DEV_INTEGRITY) += bio-integrity.o
-obj-$(CONFIG_INOTIFY)		+= inotify.o
-obj-$(CONFIG_INOTIFY_USER)	+= inotify_user.o
+obj-y				+= notify/
 obj-$(CONFIG_EPOLL)		+= eventpoll.o
 obj-$(CONFIG_ANON_INODES)	+= anon_inodes.o
 obj-$(CONFIG_SIGNALFD)		+= signalfd.o
@@ -57,8 +56,6 @@ obj-$(CONFIG_QFMT_V1)		+= quota_v1.o
 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 --git a/fs/notify/Kconfig b/fs/notify/Kconfig
new file mode 100644
index 0000000..50914d7
--- /dev/null
+++ b/fs/notify/Kconfig
@@ -0,0 +1,2 @@
+source "fs/notify/dnotify/Kconfig"
+source "fs/notify/inotify/Kconfig"
diff --git a/fs/notify/Makefile b/fs/notify/Makefile
new file mode 100644
index 0000000..5a95b60
--- /dev/null
+++ b/fs/notify/Makefile
@@ -0,0 +1,2 @@
+obj-y			+= dnotify/
+obj-y			+= inotify/
diff --git a/fs/notify/dnotify/Kconfig b/fs/notify/dnotify/Kconfig
new file mode 100644
index 0000000..26adf5d
--- /dev/null
+++ b/fs/notify/dnotify/Kconfig
@@ -0,0 +1,10 @@
+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.
+
+	  If unsure, say Y.
diff --git a/fs/notify/dnotify/Makefile b/fs/notify/dnotify/Makefile
new file mode 100644
index 0000000..f145251
--- /dev/null
+++ b/fs/notify/dnotify/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_DNOTIFY)		+= dnotify.o
diff --git a/fs/dnotify.c b/fs/notify/dnotify/dnotify.c
similarity index 100%
rename from fs/dnotify.c
rename to fs/notify/dnotify/dnotify.c
diff --git a/fs/notify/inotify/Kconfig b/fs/notify/inotify/Kconfig
new file mode 100644
index 0000000..4467928
--- /dev/null
+++ b/fs/notify/inotify/Kconfig
@@ -0,0 +1,27 @@
+config INOTIFY
+	bool "Inotify file change notification support"
+	default y
+	---help---
+	  Say Y here to enable inotify support.  Inotify is a file change
+	  notification system and a replacement for dnotify.  Inotify fixes
+	  numerous shortcomings in dnotify and introduces several new features
+	  including multiple file events, one-shot support, and unmount
+	  notification.
+
+	  For more information, see <file:Documentation/filesystems/inotify.txt>
+
+	  If unsure, say Y.
+
+config INOTIFY_USER
+	bool "Inotify support for userspace"
+	depends on INOTIFY
+	default y
+	---help---
+	  Say Y here to enable inotify support for userspace, including the
+	  associated system calls.  Inotify allows monitoring of both files and
+	  directories via a single open fd.  Events are read from the file
+	  descriptor, which is also select()- and poll()-able.
+
+	  For more information, see <file:Documentation/filesystems/inotify.txt>
+
+	  If unsure, say Y.
diff --git a/fs/notify/inotify/Makefile b/fs/notify/inotify/Makefile
new file mode 100644
index 0000000..e290f3b
--- /dev/null
+++ b/fs/notify/inotify/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_INOTIFY)		+= inotify.o
+obj-$(CONFIG_INOTIFY_USER)	+= inotify_user.o
diff --git a/fs/inotify.c b/fs/notify/inotify/inotify.c
similarity index 100%
rename from fs/inotify.c
rename to fs/notify/inotify/inotify.c
diff --git a/fs/inotify_user.c b/fs/notify/inotify/inotify_user.c
similarity index 100%
rename from fs/inotify_user.c
rename to fs/notify/inotify/inotify_user.c


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

* [PATCH 2/3] fsnotify: sys_execve and sys_uselib do not call into fsnotify
  2008-12-15 16:44 [PATCH 0/3] send notification events about exec Eric Paris
  2008-12-15 16:44 ` [PATCH 1/3] filesystem notification: create fs/notify to contain all fs notification Eric Paris
@ 2008-12-15 16:44 ` Eric Paris
  2008-12-16  1:02   ` KOSAKI Motohiro
  2008-12-15 16:44 ` [PATCH 3/3] fsnotify: use the new open-exec hook for inotify and dnotify Eric Paris
  2 siblings, 1 reply; 11+ messages in thread
From: Eric Paris @ 2008-12-15 16:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: hch, akpm

sys_execve and sys_uselib do not call into fsnotify so inotify and dnotify do
not see opens on things which are going to be exectued.  This patch creates a
generic fsnotify hook for these operations

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 fs/exec.c                |    5 +++++
 include/linux/fsnotify.h |    7 +++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index ec5df9a..8a659a8 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -51,6 +51,7 @@
 #include <linux/audit.h>
 #include <linux/tracehook.h>
 #include <linux/kmod.h>
+#include <linux/fsnotify.h>
 
 #include <asm/uaccess.h>
 #include <asm/mmu_context.h>
@@ -135,6 +136,8 @@ asmlinkage long sys_uselib(const char __user * library)
 	if (IS_ERR(file))
 		goto out;
 
+	fsnotify_open_exec(file);
+
 	error = -ENOEXEC;
 	if(file->f_op) {
 		struct linux_binfmt * fmt;
@@ -687,6 +690,8 @@ struct file *open_exec(const char *name)
 	if (IS_ERR(file))
 		return file;
 
+	fsnotify_open_exec(file);
+
 	err = deny_write_access(file);
 	if (err) {
 		fput(file);
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 00fbd5b..88265dd 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -166,6 +166,13 @@ static inline void fsnotify_modify(struct dentry *dentry)
 }
 
 /*
+ * fsnotify_open_exec - file was opened by execve or uselib
+ */
+static inline void fsnotify_open_exec(struct file *file)
+{
+}
+
+/*
  * fsnotify_open - file was opened
  */
 static inline void fsnotify_open(struct dentry *dentry)


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

* [PATCH 3/3] fsnotify: use the new open-exec hook for inotify and dnotify
  2008-12-15 16:44 [PATCH 0/3] send notification events about exec Eric Paris
  2008-12-15 16:44 ` [PATCH 1/3] filesystem notification: create fs/notify to contain all fs notification Eric Paris
  2008-12-15 16:44 ` [PATCH 2/3] fsnotify: sys_execve and sys_uselib do not call into fsnotify Eric Paris
@ 2008-12-15 16:44 ` Eric Paris
  2008-12-16  1:12   ` KOSAKI Motohiro
  2 siblings, 1 reply; 11+ messages in thread
From: Eric Paris @ 2008-12-15 16:44 UTC (permalink / raw)
  To: linux-kernel; +Cc: hch, akpm

inotify and dnotify did not get access events when their children were
accessed for shlib or exec purposes.  Trigger on those events as well.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 include/linux/fsnotify.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 88265dd..a7122c6 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -170,6 +170,12 @@ static inline void fsnotify_modify(struct dentry *dentry)
  */
 static inline void fsnotify_open_exec(struct file *file)
 {
+	struct dentry *dentry = file->f_path.dentry;
+	struct inode *inode = dentry->d_inode;
+
+	dnotify_parent(dentry, DN_ACCESS);
+	inotify_dentry_parent_queue_event(dentry, IN_ACCESS, 0, dentry->d_name.name);
+	inotify_inode_queue_event(inode, IN_ACCESS, 0, NULL, NULL);
 }
 
 /*


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

* Re: [PATCH 1/3] filesystem notification: create fs/notify to contain all fs notification
  2008-12-15 16:44 ` [PATCH 1/3] filesystem notification: create fs/notify to contain all fs notification Eric Paris
@ 2008-12-16  1:02   ` KOSAKI Motohiro
  2008-12-16  1:37     ` Eric Paris
  0 siblings, 1 reply; 11+ messages in thread
From: KOSAKI Motohiro @ 2008-12-16  1:02 UTC (permalink / raw)
  To: Eric Paris; +Cc: kosaki.motohiro, linux-kernel, hch, akpm

Hi

> Creating a generic filesystem notification interface, fsnotify, which will be
> used by inotify, dnotify, and eventually fanotify is really starting to
> clutter the fs directory.  This patch simply moves inotify and dnotify into
> fs/notify/inotify and fs/notify/dnotify respectively.
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>
> ---
> 
>  fs/Kconfig                       |   39 --
>  fs/Makefile                      |    5 
>  fs/dnotify.c                     |  194 --------
>  fs/inotify.c                     |  913 --------------------------------------
>  fs/inotify_user.c                |  778 --------------------------------
>  fs/notify/Kconfig                |    2 
>  fs/notify/Makefile               |    2 
>  fs/notify/dnotify/Kconfig        |   10 
>  fs/notify/dnotify/Makefile       |    1 
>  fs/notify/dnotify/dnotify.c      |  194 ++++++++
>  fs/notify/inotify/Kconfig        |   27 +
>  fs/notify/inotify/Makefile       |    2 
>  fs/notify/inotify/inotify.c      |  913 ++++++++++++++++++++++++++++++++++++++
>  fs/notify/inotify/inotify_user.c |  778 ++++++++++++++++++++++++++++++++
>  14 files changed, 1931 insertions(+), 1927 deletions(-)
>  delete mode 100644 fs/dnotify.c
>  delete mode 100644 fs/inotify.c
>  delete mode 100644 fs/inotify_user.c
>  create mode 100644 fs/notify/Kconfig
>  create mode 100644 fs/notify/Makefile
>  create mode 100644 fs/notify/dnotify/Kconfig
>  create mode 100644 fs/notify/dnotify/Makefile
>  create mode 100644 fs/notify/dnotify/dnotify.c
>  create mode 100644 fs/notify/inotify/Kconfig
>  create mode 100644 fs/notify/inotify/Makefile
>  create mode 100644 fs/notify/inotify/inotify.c
>  create mode 100644 fs/notify/inotify/inotify_user.c

1) this mail only contain Kconfig and Makefile changing patch.
   iow, this patch doesn't contain file moving information.
   iow, patch statics and actual patch are different.
2) your latter patch doesn't depend on this patch.
   this patch doesn't have justification.
   I recommend to separate this patch.




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

* Re: [PATCH 2/3] fsnotify: sys_execve and sys_uselib do not call into fsnotify
  2008-12-15 16:44 ` [PATCH 2/3] fsnotify: sys_execve and sys_uselib do not call into fsnotify Eric Paris
@ 2008-12-16  1:02   ` KOSAKI Motohiro
  0 siblings, 0 replies; 11+ messages in thread
From: KOSAKI Motohiro @ 2008-12-16  1:02 UTC (permalink / raw)
  To: Eric Paris; +Cc: kosaki.motohiro, linux-kernel, hch, akpm

> sys_execve and sys_uselib do not call into fsnotify so inotify and dnotify do
> not see opens on things which are going to be exectued.  This patch creates a
> generic fsnotify hook for these operations
> 
> Signed-off-by: Eric Paris <eparis@redhat.com>

looks good to me.
	Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>




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

* Re: [PATCH 3/3] fsnotify: use the new open-exec hook for inotify and dnotify
  2008-12-15 16:44 ` [PATCH 3/3] fsnotify: use the new open-exec hook for inotify and dnotify Eric Paris
@ 2008-12-16  1:12   ` KOSAKI Motohiro
  2008-12-16  1:52     ` Eric Paris
  0 siblings, 1 reply; 11+ messages in thread
From: KOSAKI Motohiro @ 2008-12-16  1:12 UTC (permalink / raw)
  To: Eric Paris; +Cc: kosaki.motohiro, linux-kernel, hch, akpm

> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index 88265dd..a7122c6 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -170,6 +170,12 @@ static inline void fsnotify_modify(struct dentry *dentry)
>   */
>  static inline void fsnotify_open_exec(struct file *file)
>  {
> +	struct dentry *dentry = file->f_path.dentry;
> +	struct inode *inode = dentry->d_inode;
> +
> +	dnotify_parent(dentry, DN_ACCESS);
> +	inotify_dentry_parent_queue_event(dentry, IN_ACCESS, 0, dentry->d_name.name);
> +	inotify_inode_queue_event(inode, IN_ACCESS, 0, NULL, NULL);
>  }

Current fsnotify_open() has following code

	static inline void fsnotify_open(struct dentry *dentry)
	{
	        struct inode *inode = dentry->d_inode;
	        u32 mask = IN_OPEN;
	
	        if (S_ISDIR(inode->i_mode))
	                mask |= IN_ISDIR;
	
	        inotify_dentry_parent_queue_event(dentry, mask, 0, dentry->d_name.name);
	        inotify_inode_queue_event(inode, mask, 0, NULL, NULL);
	}

they are two different.

1) Call dnotify_parent() or not
2) Use IN_OPEN or IN_ACCESS

The patch description doesn't explain any reason.


IOW, IN_ACCESS is usually used by read(). but linux has demand paging
mechanism. then exec() only do open and mmap.
actual reading is processed by page fault.

I guess you have the reason of this design choice.
but it isn't described.





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

* Re: [PATCH 1/3] filesystem notification: create fs/notify to contain all fs notification
  2008-12-16  1:02   ` KOSAKI Motohiro
@ 2008-12-16  1:37     ` Eric Paris
  2008-12-16  1:46       ` KOSAKI Motohiro
  0 siblings, 1 reply; 11+ messages in thread
From: Eric Paris @ 2008-12-16  1:37 UTC (permalink / raw)
  To: KOSAKI Motohiro; +Cc: linux-kernel, hch, akpm

On Tue, 2008-12-16 at 10:02 +0900, KOSAKI Motohiro wrote:
> Hi
> 
> > Creating a generic filesystem notification interface, fsnotify, which will be
> > used by inotify, dnotify, and eventually fanotify is really starting to
> > clutter the fs directory.  This patch simply moves inotify and dnotify into
> > fs/notify/inotify and fs/notify/dnotify respectively.
> > 
> > Signed-off-by: Eric Paris <eparis@redhat.com>
> > ---
> > 
> >  fs/Kconfig                       |   39 --
> >  fs/Makefile                      |    5 
> >  fs/dnotify.c                     |  194 --------
> >  fs/inotify.c                     |  913 --------------------------------------
> >  fs/inotify_user.c                |  778 --------------------------------
> >  fs/notify/Kconfig                |    2 
> >  fs/notify/Makefile               |    2 
> >  fs/notify/dnotify/Kconfig        |   10 
> >  fs/notify/dnotify/Makefile       |    1 
> >  fs/notify/dnotify/dnotify.c      |  194 ++++++++
> >  fs/notify/inotify/Kconfig        |   27 +
> >  fs/notify/inotify/Makefile       |    2 
> >  fs/notify/inotify/inotify.c      |  913 ++++++++++++++++++++++++++++++++++++++
> >  fs/notify/inotify/inotify_user.c |  778 ++++++++++++++++++++++++++++++++
> >  14 files changed, 1931 insertions(+), 1927 deletions(-)
> >  delete mode 100644 fs/dnotify.c
> >  delete mode 100644 fs/inotify.c
> >  delete mode 100644 fs/inotify_user.c
> >  create mode 100644 fs/notify/Kconfig
> >  create mode 100644 fs/notify/Makefile
> >  create mode 100644 fs/notify/dnotify/Kconfig
> >  create mode 100644 fs/notify/dnotify/Makefile
> >  create mode 100644 fs/notify/dnotify/dnotify.c
> >  create mode 100644 fs/notify/inotify/Kconfig
> >  create mode 100644 fs/notify/inotify/Makefile
> >  create mode 100644 fs/notify/inotify/inotify.c
> >  create mode 100644 fs/notify/inotify/inotify_user.c
> 
> 1) this mail only contain Kconfig and Makefile changing patch.
>    iow, this patch doesn't contain file moving information.
>    iow, patch statics and actual patch are different.
> 2) your latter patch doesn't depend on this patch.
>    this patch doesn't have justification.
>    I recommend to separate this patch.

It is there, it is just in the git -M rename diff output rather than
"normal" diff.  See the sections like:

diff --git a/fs/dnotify.c b/fs/notify/dnotify/dnotify.c
similarity index 100%
rename from fs/dnotify.c
rename to fs/notify/dnotify/dnotify.c

This type of patch can be applied directly to your git tree and makes
review trivial.....

If people who will put this into a subsystem would prefer the long diff
I'll gladly resend.

I'll agree it isn't required but the other 2 aren't going to apply
without it, and it does clean up fs.  I'd much rather get this early
than in the middle somewhere since others are also looking at changing
this subsystem at the same time....     It can be dropped it others
agree....


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

* Re: [PATCH 1/3] filesystem notification: create fs/notify to contain all fs notification
  2008-12-16  1:37     ` Eric Paris
@ 2008-12-16  1:46       ` KOSAKI Motohiro
  0 siblings, 0 replies; 11+ messages in thread
From: KOSAKI Motohiro @ 2008-12-16  1:46 UTC (permalink / raw)
  To: Eric Paris; +Cc: kosaki.motohiro, linux-kernel, hch, akpm

> > 1) this mail only contain Kconfig and Makefile changing patch.
> >    iow, this patch doesn't contain file moving information.
> >    iow, patch statics and actual patch are different.
> > 2) your latter patch doesn't depend on this patch.
> >    this patch doesn't have justification.
> >    I recommend to separate this patch.
> 
> It is there, it is just in the git -M rename diff output rather than
> "normal" diff.  See the sections like:
> 
> diff --git a/fs/dnotify.c b/fs/notify/dnotify/dnotify.c
> similarity index 100%
> rename from fs/dnotify.c
> rename to fs/notify/dnotify/dnotify.c
> 
> This type of patch can be applied directly to your git tree and makes
> review trivial.....
> 
> If people who will put this into a subsystem would prefer the long diff
> I'll gladly resend.

Oh sorry ;)
To be honest, almost linux-mm develper are using quilt and I don't use
git almost case.


> I'll agree it isn't required but the other 2 aren't going to apply
> without it, and it does clean up fs.  I'd much rather get this early
> than in the middle somewhere since others are also looking at changing
> this subsystem at the same time....     It can be dropped it others
> agree....

I don't objection this cleanup. but the patch series should be made by
logical group.
I don't like to contain independent cleanup patch ;-)



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

* Re: [PATCH 3/3] fsnotify: use the new open-exec hook for inotify and dnotify
  2008-12-16  1:12   ` KOSAKI Motohiro
@ 2008-12-16  1:52     ` Eric Paris
  2008-12-16  2:01       ` KOSAKI Motohiro
  0 siblings, 1 reply; 11+ messages in thread
From: Eric Paris @ 2008-12-16  1:52 UTC (permalink / raw)
  To: KOSAKI Motohiro; +Cc: linux-kernel, hch, akpm

On Tue, 2008-12-16 at 10:12 +0900, KOSAKI Motohiro wrote:
> > diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> > index 88265dd..a7122c6 100644
> > --- a/include/linux/fsnotify.h
> > +++ b/include/linux/fsnotify.h
> > @@ -170,6 +170,12 @@ static inline void fsnotify_modify(struct dentry *dentry)
> >   */
> >  static inline void fsnotify_open_exec(struct file *file)
> >  {
> > +	struct dentry *dentry = file->f_path.dentry;
> > +	struct inode *inode = dentry->d_inode;
> > +
> > +	dnotify_parent(dentry, DN_ACCESS);
> > +	inotify_dentry_parent_queue_event(dentry, IN_ACCESS, 0, dentry->d_name.name);
> > +	inotify_inode_queue_event(inode, IN_ACCESS, 0, NULL, NULL);
> >  }
> 
> Current fsnotify_open() has following code
> 
> 	static inline void fsnotify_open(struct dentry *dentry)
> 	{
> 	        struct inode *inode = dentry->d_inode;
> 	        u32 mask = IN_OPEN;
> 	
> 	        if (S_ISDIR(inode->i_mode))
> 	                mask |= IN_ISDIR;
> 	
> 	        inotify_dentry_parent_queue_event(dentry, mask, 0, dentry->d_name.name);
> 	        inotify_inode_queue_event(inode, mask, 0, NULL, NULL);
> 	}
> 
> they are two different.
> 
> 1) Call dnotify_parent() or not
> 2) Use IN_OPEN or IN_ACCESS
> 
> The patch description doesn't explain any reason.
> 
> 
> IOW, IN_ACCESS is usually used by read(). but linux has demand paging
> mechanism. then exec() only do open and mmap.
> actual reading is processed by page fault.
> 
> I guess you have the reason of this design choice.
> but it isn't described.

The original logic was all predicated on my thoughts on how my new
fanotify would want these events and how I felt that open for exec was
worth the separate hook.  None of that is useful at this time and in any
case IN_OPEN makes a lot more sense than IN_ACCESS.  Since you've got me
looking at these as freestanding patchs I do tend to think that the
easiest thing for now would be to just drop patch 2 and make the call
sites from patch 2 call fsnotify_open directly.

I'll resend in the morning a single patch to call directly to
fsnotify_open.  (and another single patch to immediately do the rename
that I want done which I'll send as the full normal diff since it'll be
freestanding)


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

* Re: [PATCH 3/3] fsnotify: use the new open-exec hook for inotify and dnotify
  2008-12-16  1:52     ` Eric Paris
@ 2008-12-16  2:01       ` KOSAKI Motohiro
  0 siblings, 0 replies; 11+ messages in thread
From: KOSAKI Motohiro @ 2008-12-16  2:01 UTC (permalink / raw)
  To: Eric Paris; +Cc: kosaki.motohiro, linux-kernel, hch, akpm

> > they are two different.
> > 
> > 1) Call dnotify_parent() or not
> > 2) Use IN_OPEN or IN_ACCESS
> > 
> > The patch description doesn't explain any reason.
> > 
> > 
> > IOW, IN_ACCESS is usually used by read(). but linux has demand paging
> > mechanism. then exec() only do open and mmap.
> > actual reading is processed by page fault.
> > 
> > I guess you have the reason of this design choice.
> > but it isn't described.
> 
> The original logic was all predicated on my thoughts on how my new
> fanotify would want these events and how I felt that open for exec was
> worth the separate hook.  None of that is useful at this time and in any
> case IN_OPEN makes a lot more sense than IN_ACCESS.  Since you've got me
> looking at these as freestanding patchs I do tend to think that the
> easiest thing for now would be to just drop patch 2 and make the call
> sites from patch 2 call fsnotify_open directly.

it seems make sense. thanks.


> I'll resend in the morning a single patch to call directly to
> fsnotify_open.  (and another single patch to immediately do the rename
> that I want done which I'll send as the full normal diff since it'll be
> freestanding)

you don't need full normal diff. last comment was my mistake. sorry.
I only hope patch separation.





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

end of thread, other threads:[~2008-12-16  2:01 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-12-15 16:44 [PATCH 0/3] send notification events about exec Eric Paris
2008-12-15 16:44 ` [PATCH 1/3] filesystem notification: create fs/notify to contain all fs notification Eric Paris
2008-12-16  1:02   ` KOSAKI Motohiro
2008-12-16  1:37     ` Eric Paris
2008-12-16  1:46       ` KOSAKI Motohiro
2008-12-15 16:44 ` [PATCH 2/3] fsnotify: sys_execve and sys_uselib do not call into fsnotify Eric Paris
2008-12-16  1:02   ` KOSAKI Motohiro
2008-12-15 16:44 ` [PATCH 3/3] fsnotify: use the new open-exec hook for inotify and dnotify Eric Paris
2008-12-16  1:12   ` KOSAKI Motohiro
2008-12-16  1:52     ` Eric Paris
2008-12-16  2:01       ` KOSAKI Motohiro

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

all inboxes | Powered by JetHome®