Re: [RFC/PATCH 1/2] fsnotify

2005-07-11 Thread Chris Wright
* 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC/PATCH 1/2] fsnotify

2005-07-11 Thread Robert Love
On Mon, 2005-07-11 at 13:52 +0100, David Woodhouse wrote:

> 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


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC/PATCH 1/2] fsnotify

2005-07-11 Thread David Woodhouse
On Fri, 2005-07-08 at 18:26 -0700, Chris Wright wrote:
> 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 '' 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.

-- 
dwmw2

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/