procfs, fdinfo updated

fs/proc/fd.c line 22

Quoted 2 lines · Cyrill Gorcunov

fs/proc/fd.c +1 -1

-static int fdinfo_open_helper(struct inode *inode, int *f_flags, struct path *path)
+static int fdinfo_open_helper(struct inode *inode, int *f_flags, struct file **f_file, struct path *path)

Bloody bad taste, that... This kind of optional arguments is almost always a bad sign - tends to happen when you have two barely related functions crammed into one. And yes, proc_fd_info() suffers the same braindamage. Trying to avoid code duplication is generally a good thing, but it's not always worth doing - less obfuscated code wins.

Quoted 8 lines

static int seq_show(struct seq_file *m, void *v)
{

fs/proc/fd.c +1 -4

 	struct proc_fdinfo *fdinfo = m->private;
-	seq_printf(m, "pos:\t%lli\nflags:\t0%o\n",
-		   (long long)fdinfo->f_pos,
-		   fdinfo->f_flags);
-	return 0;
+	int ret;

fs/proc/fd.c static int fdinfo_open_helper(struct ino · line 72

Quoted 4 lines

fs/proc/fd.c +4 -0

+
+	ret = seq_printf(m, "pos:\t%lli\nflags:\t0%o\n",
+			 (long long)fdinfo->f_file->f_pos,
+			 fdinfo->f_flags);

Realistically, that one is not going to overflow; you are almost certainly wasting more cycles on that check of !ret just below than you'll save on not going into ->show_fdinfo() in case of full buffer.

fs/proc/fd.c static int fdinfo_open_helper(struct ino · line 96

Quoted 10 lines

fs/proc/fd.c +7 -0

+	if (!ret && fdinfo->f_file->f_op->show_fdinfo)
+		ret = fdinfo->f_file->f_op->show_fdinfo(m, fdinfo->f_file);
+
+	return ret;
 }
 
+	ret = single_open(file, seq_show, fdinfo);
+	if (ret) {
+		put_filp(fdinfo->f_file);

Excuse me? We should *never* do put_filp() on anything that has already been opened. Think what happens if you race with close(); close() would rip the reference from descriptor table and do fput(), leaving you with the last reference to that struct file. You really don't want to just go and free it. IOW, that one should be fput().

Review

Quoted 1 line

(quoted diff) +1 -0

+	put_filp(fdinfo->f_file);

Ditto.

struct proc_fdinfo {

(quoted diff) +5 -2

-	loff_t	f_pos;
-	int	f_flags;
+	struct file	*f_file;
+	int		f_flags;
 };
 
+	struct proc_fdinfo *fdinfo;
+	struct seq_file *m;
+	int ret;
 
 	fdinfo = kzalloc(sizeof(*fdinfo), GFP_KERNEL);
 	if (!fdinfo)

This, BTW, is too convoluted for its own good. What you need is something like

struct whatever {
	struct seq_file *m;
	struct file *f;
	int flags;
};

with single allocation of that sucker in your ->open(). Set file->private_data to address of seq_file field in your object *before* calling seq_open() and don't bother with m->private at all - just use container_of(m, struct whatever, m) in your ->show() to get to that structure...

This, BTW, is too convoluted for its own good. What you need is something like

struct whatever {
	struct seq_file *m;
	struct file *f;
	int flags;
};

with single allocation of that sucker in your ->open(). Set file->private_data to address of seq_file field in your object *before* calling seq_open() and don't bother with m->private at all - just use container_of(m, struct whatever, m) in your ->show() to get to that structure...

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

Signed-off-by Cyrill Gorcunov <[email protected]>

Acked-by Pavel Emelyanov <[email protected]>

CC Al Viro <[email protected]>

CC Alexey Dobriyan <[email protected]>

CC Andrew Morton <[email protected]>

CC James Bottomley <[email protected]>

---
 fs/exportfs/expfs.c      |   19 +++++++++++++++++++
 include/linux/exportfs.h |    2 ++
 2 files changed, 21 insertions(+)

Index: linux-2.6.git/fs/exportfs/expfs.c
===================================================================

fs/exportfs/expfs.c +19 -0

@@ -302,6 +302,25 @@ out:
 	return error;
 }
 
+int export_encode_inode_fh(struct inode *inode, struct fid *fid, int *max_len)
+{
+	int len = *max_len;
+	int type = FILEID_INO32_GEN;
+
+	if (len < 2) {
+		*max_len = 2;
+		return 255;
+	}
+
+	len = 2;
+	fid->i32.ino = inode->i_ino;
+	fid->i32.gen = inode->i_generation;
+	*max_len = len;
+
+	return type;
+}
+EXPORT_SYMBOL_GPL(export_encode_inode_fh);
+
 /**
  * export_encode_fh - default export_operations->encode_fh function
  * @inode:   the object to encode

Index: linux-2.6.git/include/linux/exportfs.h
===================================================================

include/linux/exportfs.h +2 -1

@@ -177,6 +177,8 @@ struct export_operations {
 	int (*commit_metadata)(struct inode *inode);
 };
 
+extern int export_encode_inode_fh(struct inode *inode, struct fid *fid, int *max_len);
+
 extern int exportfs_encode_fh(struct dentry *dentry, struct fid *fid,
 	int *max_len, int connectable);
 extern struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
 
--

To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 4 lines · Cyrill Gorcunov

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

I don't get it--this seems like a really roundabout way to get inode and generation number, if that's all you want.

On the other hand, if you want a real filehandle then wouldn't you want to e.g. call the filesystem's ->encode_fh() if necessary, as exportfs_encode_fh() does?

Quoted 6 lines · J. Bruce Fields

Quoted 4 lines · Cyrill Gorcunov

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

Quoted 2 lines

I don't get it--this seems like a really roundabout way to get inode and generation number, if that's all you want.

We can re-open the targets via filehandle on restore, this was the idea. All this series aimed to achieve the way to restore objects after checkpoit, thus we need to provide additional information which would be enough.

Quoted 3 lines

On the other hand, if you want a real filehandle then wouldn't you want to e.g. call the filesystem's ->encode_fh() if necessary, as exportfs_encode_fh() does?

Well, one of the problem I hit when I've been trying to use encode_fh is that every new implementation of encode_fh will require some size (even unknown) in buffer where encoded data pushed. Correct me please if I'm wrong. But with export_encode_inode_fh there is a small buffer with pretty known size needed on stack needed for printing data in fdinfo.

Quoted 10 lines · Cyrill Gorcunov

Quoted 8 lines · J. Bruce Fields

Quoted 4 lines · Cyrill Gorcunov

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

I don't get it--this seems like a really roundabout way to get inode and generation number, if that's all you want.

Quoted 3 lines

We can re-open the targets via filehandle on restore, this was the idea. All this series aimed to achieve the way to restore objects after checkpoit, thus we need to provide additional information which would be enough.

For this to work it'll need to be something you can pass to open_by_handle_at, won't it?

Quoted 11 lines

Quoted 4 lines

On the other hand, if you want a real filehandle then wouldn't you want to e.g. call the filesystem's ->encode_fh() if necessary, as exportfs_encode_fh() does?

Well, one of the problem I hit when I've been trying to use encode_fh is that every new implementation of encode_fh will require some size (even unknown) in buffer where encoded data pushed. Correct me please if I'm wrong. But with export_encode_inode_fh there is a small buffer with pretty known size needed on stack needed for printing data in fdinfo.

You can just give encode_fh a too-small data and let it fail if it's not big enough.

(In practice I think everyone supports NFSv3 filehandles which have a maximum size of 64 bytes.)

Quoted 11 lines

Quoted 10 lines

Quoted 3 lines

On the other hand, if you want a real filehandle then wouldn't you want to e.g. call the filesystem's ->encode_fh() if necessary, as exportfs_encode_fh() does?

Well, one of the problem I hit when I've been trying to use encode_fh is that every new implementation of encode_fh will require some size (even unknown) in buffer where encoded data pushed. Correct me please if I'm wrong. But with export_encode_inode_fh there is a small buffer with pretty known size needed on stack needed for printing data in fdinfo.

Quoted 5 lines

You can just give encode_fh a too-small data and let it fail if it's not big enough.

(In practice I think everyone supports NFSv3 filehandles which have a maximum size of 64 bytes.)

I'll think about it, thanks!

Quoted 17 lines · Cyrill Gorcunov

Quoted 16 lines

Quoted 10 lines

Quoted 3 lines

On the other hand, if you want a real filehandle then wouldn't you want to e.g. call the filesystem's ->encode_fh() if necessary, as exportfs_encode_fh() does?

Well, one of the problem I hit when I've been trying to use encode_fh is that every new implementation of encode_fh will require some size (even unknown) in buffer where encoded data pushed. Correct me please if I'm wrong. But with export_encode_inode_fh there is a small buffer with pretty known size needed on stack needed for printing data in fdinfo.

You can just give encode_fh a too-small data and let it fail if it's not big enough.

(In practice I think everyone supports NFSv3 filehandles which have a maximum size of 64 bytes.)

Quoted 1 line

I'll think about it, thanks!

Hi Bruce, thinking a bit more I guess using general encode_fh is not that convenient since it operates with dentries while our fdinfo output deals with inodes. Thus I should either provide some new encode_fh variant which would deal with inodes directly without "parents".

Which doesn't
look for me anyhow better than the new export_encode_inode_fh helper.

After all, if the use of encode_fh become a mandatory rule we can easily extend fsnotify fdinfo output to support new scheme without breaking user space, because output looks like

| fhandle-type: 1 f_handle: 49b1060023552153

(ie if something is changed than these fields will be simply updated).

Or maybe I miss something?

Cyrill

Quoted 4 lines

Hi Bruce, thinking a bit more I guess using general encode_fh is not that convenient since it operates with dentries while our fdinfo output deals with inodes. Thus I should either provide some new encode_fh variant which would deal with inodes directly without "parents".

I can't see why that wouldn't work.

Quoted 2 lines

Which doesn't
look for me anyhow better than the new export_encode_inode_fh helper.

That isn't going to work for filesystems that define their own encode_fh:

	$ git grep '\.encode_fh'
	fs/btrfs/export.c:      .encode_fh      = btrfs_encode_fh,
	fs/ceph/export.c:       .encode_fh = ceph_encode_fh,

Quoted 18 lines · Cyrill Gorcunov

Quoted 17 lines · Cyrill Gorcunov

Quoted 16 lines

Quoted 10 lines

Quoted 3 lines

On the other hand, if you want a real filehandle then wouldn't you want to e.g. call the filesystem's ->encode_fh() if necessary, as exportfs_encode_fh() does?

Well, one of the problem I hit when I've been trying to use encode_fh is that every new implementation of encode_fh will require some size (even unknown) in buffer where encoded data pushed. Correct me please if I'm wrong. But with export_encode_inode_fh there is a small buffer with pretty known size needed on stack needed for printing data in fdinfo.

You can just give encode_fh a too-small data and let it fail if it's not big enough.

(In practice I think everyone supports NFSv3 filehandles which have a maximum size of 64 bytes.)

Quoted 6 lines

Quoted 1 line

I'll think about it, thanks!

Hi Bruce, thinking a bit more I guess using general encode_fh is not that convenient since it operates with dentries while our fdinfo output deals with inodes. Thus I should either provide some new encode_fh variant which would deal with inodes directly without "parents".

I can't see why that wouldn't work.

Guys, would the patch below be more-less acceptible?
In inotify I think we could pass "parent" as NULL and use general encode engine then (ie it will look like someone called for name_to_handle_at on inotify target).

Quoted 4 lines · Cyrill Gorcunov

Guys, would the patch below be more-less acceptible?
In inotify I think we could pass "parent" as NULL and use general encode engine then (ie it will look like someone called for name_to_handle_at on inotify target).

Wait. What the hell are you going to do with those afterwards? Again, there's a shitload of filesystems that cannot be exported over NFS, exactly because there's no way to implement sanely working fhandles. And idiotify is allowed for all of them.

You *can't* decode anything fhandle-like on e.g. sysfs. Or procfs. Or configfs. Or any network filesystem (I'd argue that we should simply ban idiotify on those, but good luck doing that). You *can* do that for FAT derivatives, but only if you have parent directory when creating that sucker.

Hi Bruce, thinking a bit more I guess using general encode_fh is not that convenient since it operates with dentries while our fdinfo output deals with inodes. Thus I should either provide some new encode_fh variant which would deal with inodes directly without "parents". Which doesn't look for me anyhow better than the new export_encode_inode_fh helper.

Quoted 5 lines · Cyrill Gorcunov

Hi Bruce, thinking a bit more I guess using general encode_fh is not that convenient since it operates with dentries while our fdinfo output deals with inodes. Thus I should either provide some new encode_fh variant which would deal with inodes directly without "parents". Which doesn't look for me anyhow better than the new export_encode_inode_fh helper.

Huh? You do have dentries, for crying out loud...

.

Quoted 8 lines · Al Viro

Quoted 5 lines · Cyrill Gorcunov

Hi Bruce, thinking a bit more I guess using general encode_fh is not that convenient since it operates with dentries while our fdinfo output deals with inodes. Thus I should either provide some new encode_fh variant which would deal with inodes directly without "parents". Which doesn't look for me anyhow better than the new export_encode_inode_fh helper.

Quoted 1 line

Huh? You do have dentries, for crying out loud...

Sometimes we don't -- the inotify thing gets an inode only. Unlike other notifies that have dentries at hands...

Quoted 9 lines · Pavel Emelyanov

Quoted 8 lines · Al Viro

Quoted 5 lines · Cyrill Gorcunov

Hi Bruce, thinking a bit more I guess using general encode_fh is not that convenient since it operates with dentries while our fdinfo output deals with inodes. Thus I should either provide some new encode_fh variant which would deal with inodes directly without "parents". Which doesn't look for me anyhow better than the new export_encode_inode_fh helper.

Quoted 4 lines

Quoted 1 line

Huh? You do have dentries, for crying out loud...

Sometimes we don't -- the inotify thing gets an inode only. Unlike other notifies that have dentries at hands...

What's wrong with saying "we don't support idiotify"?

.

Quoted 9 lines · Pavel Emelyanov

Quoted 8 lines · Al Viro

Quoted 5 lines · Cyrill Gorcunov

Hi Bruce, thinking a bit more I guess using general encode_fh is not that convenient since it operates with dentries while our fdinfo output deals with inodes. Thus I should either provide some new encode_fh variant which would deal with inodes directly without "parents". Which doesn't look for me anyhow better than the new export_encode_inode_fh helper.

Quoted 4 lines

Quoted 1 line

Huh? You do have dentries, for crying out loud...

Sometimes we don't -- the inotify thing gets an inode only. Unlike other notifies that have dentries at hands...

What's wrong with saying "we don't support idiotify"?

Quoted 15 lines · Al Viro

Quoted 13 lines · Pavel Emelyanov

Quoted 9 lines · Al Viro

Quoted 5 lines · Cyrill Gorcunov

Hi Bruce, thinking a bit more I guess using general encode_fh is not that convenient since it operates with dentries while our fdinfo output deals with inodes. Thus I should either provide some new encode_fh variant which would deal with inodes directly without "parents". Which doesn't look for me anyhow better than the new export_encode_inode_fh helper.

Huh? You do have dentries, for crying out loud...

Sometimes we don't -- the inotify thing gets an inode only. Unlike other notifies that have dentries at hands...

Quoted 1 line

What's wrong with saying "we don't support idiotify"?

Al, we need some way to restore inotifies after checkpoint. At the very early versions of these patches I simply added dentry to the inotify mark thus once inotify created we always have a dentry to refer on in encode_fh, but I'm not sure if this will be good design.

Quoted 16 lines · Cyrill Gorcunov

Quoted 15 lines · Al Viro

Quoted 13 lines · Pavel Emelyanov

Quoted 9 lines · Al Viro

Quoted 5 lines · Cyrill Gorcunov

Hi Bruce, thinking a bit more I guess using general encode_fh is not that convenient since it operates with dentries while our fdinfo output deals with inodes. Thus I should either provide some new encode_fh variant which would deal with inodes directly without "parents". Which doesn't look for me anyhow better than the new export_encode_inode_fh helper.

Huh? You do have dentries, for crying out loud...

Sometimes we don't -- the inotify thing gets an inode only. Unlike other notifies that have dentries at hands...

Quoted 7 lines

Quoted 1 line

What's wrong with saying "we don't support idiotify"?

Al, we need some way to restore inotifies after checkpoint. At the very early versions of these patches I simply added dentry to the inotify mark thus once inotify created we always have a dentry to refer on in encode_fh, but I'm not sure if this will be good design.

Actually, I was about to suggest this. This can be done internally within fs/notify without actually modifying the syscall interface, can't it, since they take a path which is used to obtain the inode? It looks like the whole of the inotify interface could be internally recast to use dentries instead of inodes. Unless I've missed something obvious?

James

Quoted 7 lines

Quoted 1 line

What's wrong with saying "we don't support idiotify"?

Al, we need some way to restore inotifies after checkpoint. At the very early versions of these patches I simply added dentry to the inotify mark thus once inotify created we always have a dentry to refer on in encode_fh, but I'm not sure if this will be good design.

Actually, I was about to suggest this. This can be done internally within fs/notify without actually modifying the syscall interface, can't it, since they take a path which is used to obtain the inode? It looks like the whole of the inotify interface could be internally recast to use dentries instead of inodes. Unless I've missed something obvious?

Quoted 8 lines · James Bottomley

Quoted 7 lines

Quoted 1 line

What's wrong with saying "we don't support idiotify"?

Al, we need some way to restore inotifies after checkpoint. At the very early versions of these patches I simply added dentry to the inotify mark thus once inotify created we always have a dentry to refer on in encode_fh, but I'm not sure if this will be good design.

Quoted 5 lines

Actually, I was about to suggest this. This can be done internally within fs/notify without actually modifying the syscall interface, can't it, since they take a path which is used to obtain the inode? It looks like the whole of the inotify interface could be internally recast to use dentries instead of inodes. Unless I've missed something obvious?

Well, after looking into do_sys_name_to_handle->exportfs_encode_fh sequence more precisely it seems it will be easier to extend exportfs_encode_fh to support inodes directly instead of playing with notify code (again, if i'm not missing something too). i'm cooking a patch to show (once it's tested i'll send it out).

Quoted 15 lines · Cyrill Gorcunov

Quoted 13 lines · James Bottomley

Quoted 7 lines

Quoted 1 line

What's wrong with saying "we don't support idiotify"?

Al, we need some way to restore inotifies after checkpoint. At the very early versions of these patches I simply added dentry to the inotify mark thus once inotify created we always have a dentry to refer on in encode_fh, but I'm not sure if this will be good design.

Actually, I was about to suggest this. This can be done internally within fs/notify without actually modifying the syscall interface, can't it, since they take a path which is used to obtain the inode? It looks like the whole of the inotify interface could be internally recast to use dentries instead of inodes. Unless I've missed something obvious?

Quoted 5 lines

Well, after looking into do_sys_name_to_handle->exportfs_encode_fh sequence more precisely it seems it will be easier to extend exportfs_encode_fh to support inodes directly instead of playing with notify code (again, if i'm not missing something too). i'm cooking a patch to show (once it's tested i'll send it out).

Good luck doing that with e.g. VFAT... And then there's such thing as filesystems that don't have ->encode_fh() for a lot of very good

reasons; just try to do that on sysfs, for example. Or on ramfs, for that matter... And while saying "you can't export that over NFS" seems to work fine, idiotify-lovers will screech if you try to ban their perversion of choice on those filesystems.

Quoted 22 lines · Al Viro

Quoted 20 lines · Cyrill Gorcunov

Quoted 8 lines · James Bottomley

Quoted 7 lines

Quoted 1 line

What's wrong with saying "we don't support idiotify"?

Al, we need some way to restore inotifies after checkpoint. At the very early versions of these patches I simply added dentry to the inotify mark thus once inotify created we always have a dentry to refer on in encode_fh, but I'm not sure if this will be good design.

Quoted 5 lines

Actually, I was about to suggest this. This can be done internally within fs/notify without actually modifying the syscall interface, can't it, since they take a path which is used to obtain the inode? It looks like the whole of the inotify interface could be internally recast to use dentries instead of inodes. Unless I've missed something obvious?

Well, after looking into do_sys_name_to_handle->exportfs_encode_fh sequence more precisely it seems it will be easier to extend exportfs_encode_fh to support inodes directly instead of playing with notify code (again, if i'm not missing something too). i'm cooking a patch to show (once it's tested i'll send it out).

Quoted 2 lines

Good luck doing that with e.g. VFAT... And then there's such thing as filesystems that don't have ->encode_fh() for a lot of very good

Wait, Al, it seems I messed up. If some fs has no encode_fh() implemented the default encoding with FILEID_INO32_GEN_PARENT will be used for that.

Quoted 5 lines

reasons; just try to do that on sysfs, for example. Or on ramfs, for that matter... And while saying "you can't export that over NFS" seems to work fine, idiotify-lovers will screech if you try to ban their perversion of choice on those filesystems.

Cyrill

Quoted 2 lines

Good luck doing that with e.g. VFAT... And then there's such thing as filesystems that don't have ->encode_fh() for a lot of very good

Wait, Al, it seems I messed up. If some fs has no encode_fh() implemented the default encoding with FILEID_INO32_GEN_PARENT will be used for that.

Quoted 5 lines · Cyrill Gorcunov

Quoted 2 lines

Good luck doing that with e.g. VFAT... And then there's such thing as filesystems that don't have ->encode_fh() for a lot of very good

Wait, Al, it seems I messed up. If some fs has no encode_fh() implemented the default encoding with FILEID_INO32_GEN_PARENT will be used for that.

... which doesn't work for a lot of filesystems. Not if you want to be able to decode the result afterwards and get something useful out of that. Trying to implement ->fh_to_dentry(), especially with fhandle generated by inode alone is going to be really interesting for a bunch of stuff...
.

Quoted 8 lines · Al Viro

Quoted 5 lines · Cyrill Gorcunov

Quoted 2 lines

Good luck doing that with e.g. VFAT... And then there's such thing as filesystems that don't have ->encode_fh() for a lot of very good

Wait, Al, it seems I messed up. If some fs has no encode_fh() implemented the default encoding with FILEID_INO32_GEN_PARENT will be used for that.

Quoted 7 lines

... which doesn't work for a lot of filesystems. Not if you want to be able to decode the result afterwards and get something useful out of that. Trying to implement ->fh_to_dentry(), especially with fhandle generated by inode alone is going to be really interesting for a bunch of stuff...
.

Hm... Then I suppose the best we can do is -- show in a fdinfo file the inode number, device where it is and a filehandle _iff_ provided by a filesystem. For fanotify/dnotify -- only a path.

Quoted 3 lines · Cyrill Gorcunov

Quoted 2 lines

Good luck doing that with e.g. VFAT... And then there's such thing as filesystems that don't have ->encode_fh() for a lot of very good

Quoted 2 lines

Wait, Al, it seems I messed up. If some fs has no encode_fh() implemented the default encoding with FILEID_INO32_GEN_PARENT will be used for that.

... which doesn't work for a lot of filesystems. Not if you want to be able to decode the result afterwards and get something useful out of that. Trying to implement ->fh_to_dentry(), especially with fhandle generated by inode alone is going to be really interesting for a bunch of stuff...

Quoted 15 lines · Cyrill Gorcunov

Quoted 8 lines · James Bottomley

Quoted 7 lines

Quoted 1 line

What's wrong with saying "we don't support idiotify"?

Al, we need some way to restore inotifies after checkpoint. At the very early versions of these patches I simply added dentry to the inotify mark thus once inotify created we always have a dentry to refer on in encode_fh, but I'm not sure if this will be good design.

Quoted 5 lines

Actually, I was about to suggest this. This can be done internally within fs/notify without actually modifying the syscall interface, can't it, since they take a path which is used to obtain the inode? It looks like the whole of the inotify interface could be internally recast to use dentries instead of inodes. Unless I've missed something obvious?

Quoted 5 lines

Well, after looking into do_sys_name_to_handle->exportfs_encode_fh sequence more precisely it seems it will be easier to extend exportfs_encode_fh to support inodes directly instead of playing with notify code (again, if i'm not missing something too). i'm cooking a patch to show (once it's tested i'll send it out).

Good luck doing that with e.g. VFAT... And then there's such thing as filesystems that don't have ->encode_fh() for a lot of very good reasons; just try to do that on sysfs, for example. Or on ramfs, for that matter... And while saying "you can't export that over NFS" seems to work fine, idiotify-lovers will screech if you try to ban their perversion of choice on those filesystems.

Quoted 5 lines · Cyrill Gorcunov

Hi Bruce, thinking a bit more I guess using general encode_fh is not that convenient since it operates with dentries while our fdinfo output deals with inodes. Thus I should either provide some new encode_fh variant which would deal with inodes directly without "parents". Which doesn't look for me anyhow better than the new export_encode_inode_fh helper.

Huh? You do have dentries, for crying out loud...

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

Signed-off-by Cyrill Gorcunov <[email protected]>

Acked-by Pavel Emelyanov <[email protected]>

CC Al Viro <[email protected]>

CC Alexey Dobriyan <[email protected]>

CC Andrew Morton <[email protected]>

CC James Bottomley <[email protected]>

---
 fs/exportfs/expfs.c      |   19 +++++++++++++++++++
 include/linux/exportfs.h |    2 ++
 2 files changed, 21 insertions(+)

Index: linux-2.6.git/fs/exportfs/expfs.c
===================================================================

fs/exportfs/expfs.c +9 -0

@@ -302,6 +302,25 @@ out:
 	return error;
 }
 
+int export_encode_inode_fh(struct inode *inode, struct fid *fid, int *max_len)
+{
+	int len = *max_len;
+	int type = FILEID_INO32_GEN;
+
+	if (len < 2) {
+		*max_len = 2;
+		return 255;
+	}

fs/exportfs/expfs.c +0 -0

 /**
  * export_encode_fh - default export_operations->encode_fh function
  * @inode:   the object to encode

Index: linux-2.6.git/include/linux/exportfs.h
===================================================================

Quoted 4 lines · Cyrill Gorcunov

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

doesn't name_to_handle_at() work for you ? It also allows to get a file handle using file descriptor.

Quoted 7 lines · Aneesh Kumar K.V

Quoted 4 lines · Cyrill Gorcunov

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

Quoted 2 lines

doesn't name_to_handle_at() work for you ? It also allows to get a file handle using file descriptor.

Hi, sorry for dealy. Well, the last idea is to get rid of this helper, I've sent out an updated version where ino+dev is only printed.

Quoted 11 lines · Cyrill Gorcunov

Quoted 9 lines · Aneesh Kumar K.V

Quoted 4 lines · Cyrill Gorcunov

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

doesn't name_to_handle_at() work for you ? It also allows to get a file handle using file descriptor.

Quoted 2 lines

Hi, sorry for dealy. Well, the last idea is to get rid of this helper, I've sent out an updated version where ino+dev is only printed.

I don't understand how ino and dev are useful to you, though, if you're still hoping to be able to look up inodes using this information later on.

Quoted 15 lines · J. Bruce Fields

Quoted 13 lines · Cyrill Gorcunov

Quoted 9 lines · Aneesh Kumar K.V

Quoted 4 lines · Cyrill Gorcunov

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

doesn't name_to_handle_at() work for you ? It also allows to get a file handle using file descriptor.

Hi, sorry for dealy. Well, the last idea is to get rid of this helper, I've sent out an updated version where ino+dev is only printed.

Quoted 3 lines

I don't understand how ino and dev are useful to you, though, if you're still hoping to be able to look up inodes using this information later on.

Hi Bruce, I believe having ino+dev is better than nothing. Otherwise we simply have no clue which targets are bound to inotify mark. Sometime (!) we can try to generate fhandle in userspace from this ino+dev bundle and then open the target file.

Quoted 7 lines

Quoted 6 lines

Quoted 2 lines

Hi, sorry for dealy. Well, the last idea is to get rid of this helper, I've sent out an updated version where ino+dev is only printed.

I don't understand how ino and dev are useful to you, though, if you're still hoping to be able to look up inodes using this information later on.

Quoted 4 lines

Hi Bruce, I believe having ino+dev is better than nothing. Otherwise we simply have no clue which targets are bound to inotify mark. Sometime (!) we can try to generate fhandle in userspace from this ino+dev bundle and then open the target file.

That's insufficient to generate a filehandle in general.

(Also: there's the usual inode-number aliasing problem: the inode number could get reused by another file. Unless you know the file is being held open the whole time.)

Quoted 20 lines · Cyrill Gorcunov

Quoted 18 lines · J. Bruce Fields

Quoted 8 lines · Cyrill Gorcunov

Quoted 7 lines · Aneesh Kumar K.V

Quoted 4 lines · Cyrill Gorcunov

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

Quoted 5 lines

Quoted 2 lines

doesn't name_to_handle_at() work for you ? It also allows to get a file handle using file descriptor.

Hi, sorry for dealy. Well, the last idea is to get rid of this helper, I've sent out an updated version where ino+dev is only printed.

I don't understand how ino and dev are useful to you, though, if you're still hoping to be able to look up inodes using this information later on.

Quoted 4 lines

Hi Bruce, I believe having ino+dev is better than nothing. Otherwise we simply have no clue which targets are bound to inotify mark. Sometime (!) we can try to generate fhandle in userspace from this ino+dev bundle and then open the target file.

That's insufficient to generate a filehandle in general.

(Also: there's the usual inode-number aliasing problem: the inode number could get reused by another file. Unless you know the file is being held open the whole time.)

--b.
.

Quoted 21 lines · J. Bruce Fields

Quoted 20 lines · Cyrill Gorcunov

Quoted 18 lines · J. Bruce Fields

Quoted 8 lines · Cyrill Gorcunov

Quoted 7 lines · Aneesh Kumar K.V

Quoted 4 lines · Cyrill Gorcunov

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

Quoted 5 lines

Quoted 2 lines

doesn't name_to_handle_at() work for you ? It also allows to get a file handle using file descriptor.

Hi, sorry for dealy. Well, the last idea is to get rid of this helper, I've sent out an updated version where ino+dev is only printed.

I don't understand how ino and dev are useful to you, though, if you're still hoping to be able to look up inodes using this information later on.

Quoted 6 lines

Quoted 4 lines

Hi Bruce, I believe having ino+dev is better than nothing. Otherwise we simply have no clue which targets are bound to inotify mark. Sometime (!) we can try to generate fhandle in userspace from this ino+dev bundle and then open the target file.

That's insufficient to generate a filehandle in general.

Yes, sure, but for live migration having inode and device is enough and that's why. We can use two ways of having a filesystem on the target machine in the same state (from paths points of view) as it was on destination one:

1. copy file tree in a rsync manner
2. copy a virtual disk image file

In the 1st case we can map inode number to path easily, since we iterate over a filesystem anyway. I agree, that rsync is not perfect for migration but still.

In the 2nd case we can generate filehandle out of an inode number only since we _do_ know that inode will not get reused.

However, if you have some better ideas on what information about inode should be exported to the userspace please share.

Quoted 4 lines

(Also: there's the usual inode-number aliasing problem: the inode number could get reused by another file. Unless you know the file is being held open the whole time.)

Quoted 36 lines · Pavel Emelyanov

Quoted 21 lines · J. Bruce Fields

Quoted 20 lines · Cyrill Gorcunov

Quoted 18 lines · J. Bruce Fields

Quoted 8 lines · Cyrill Gorcunov

Quoted 7 lines · Aneesh Kumar K.V

Quoted 4 lines · Cyrill Gorcunov

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

Quoted 5 lines

Quoted 2 lines

doesn't name_to_handle_at() work for you ? It also allows to get a file handle using file descriptor.

Hi, sorry for dealy. Well, the last idea is to get rid of this helper, I've sent out an updated version where ino+dev is only printed.

I don't understand how ino and dev are useful to you, though, if you're still hoping to be able to look up inodes using this information later on.

Quoted 6 lines

Quoted 4 lines

Hi Bruce, I believe having ino+dev is better than nothing. Otherwise we simply have no clue which targets are bound to inotify mark. Sometime (!) we can try to generate fhandle in userspace from this ino+dev bundle and then open the target file.

That's insufficient to generate a filehandle in general.

Yes, sure, but for live migration having inode and device is enough and that's why. We can use two ways of having a filesystem on the target machine in the same state (from paths points of view) as it was on destination one:

1. copy file tree in a rsync manner
2. copy a virtual disk image file

Quoted 5 lines

In the 1st case we can map inode number to path easily, since we iterate over a filesystem anyway. I agree, that rsync is not perfect for migration but still.

In the 2nd case we can generate filehandle out of an inode number only since we _do_ know that inode will not get reused.

If you are going to to use open_by_handle, then that handle is not sufficient right ? Or do you have open_by_inode ? as part of c/r ?

Quoted 4 lines

However, if you have some better ideas on what information about inode should be exported to the userspace please share.

Why not use name_to_handle(fd,...) and open_by_handle(handle,..) ?

Quoted 5 lines

Quoted 5 lines

(Also: there's the usual inode-number aliasing problem: the inode number could get reused by another file. Unless you know the file is being held open the whole time.)

-aneesh

.

Quoted 6 lines

Quoted 4 lines

However, if you have some better ideas on what information about inode should be exported to the userspace please share.

Why not use name_to_handle(fd,...) and open_by_handle(handle,..) ?

Because we don't have an fd at hands by the time we need to know the handle.

Quoted 5 lines · Pavel Emelyanov

Quoted 5 lines

Quoted 4 lines

However, if you have some better ideas on what information about inode should be exported to the userspace please share.

Quoted 3 lines

Quoted 1 line

Why not use name_to_handle(fd,...) and open_by_handle(handle,..) ?

Because we don't have an fd at hands by the time we need to know the handle.

Yeah, this might be not clear from patchset itself but inotify marks carry inodes inside kernel thus it's inodes what we can use when we fetch information about targets and put it into fdinfo output.

Cyrill
.

Quoted 6 lines · Cyrill Gorcunov

Quoted 5 lines · Pavel Emelyanov

Quoted 5 lines

Quoted 4 lines

However, if you have some better ideas on what information about inode should be exported to the userspace please share.

Quoted 7 lines

Quoted 3 lines

Quoted 1 line

Why not use name_to_handle(fd,...) and open_by_handle(handle,..) ?

Because we don't have an fd at hands by the time we need to know the handle.

Yeah, this might be not clear from patchset itself but inotify marks carry inodes inside kernel thus it's inodes what we can use when we fetch information about targets and put it into fdinfo output.

Al, Bruce, Aneesh,

What if we calculate the handle at the time we do have struct path at hands (i.e. when we create the inotify) and store it on the inotify structure purely to be shown later in proc. Would that be acceptable?

Quoted 15 lines · Pavel Emelyanov

Quoted 13 lines · Cyrill Gorcunov

Quoted 8 lines · Pavel Emelyanov

Quoted 6 lines

Quoted 4 lines

However, if you have some better ideas on what information about inode should be exported to the userspace please share.

Why not use name_to_handle(fd,...) and open_by_handle(handle,..) ?

Because we don't have an fd at hands by the time we need to know the handle.

Yeah, this might be not clear from patchset itself but inotify marks carry inodes inside kernel thus it's inodes what we can use when we fetch information about targets and put it into fdinfo output.

Quoted 5 lines

Al, Bruce, Aneesh,

What if we calculate the handle at the time we do have struct path at hands (i.e. when we create the inotify) and store it on the inotify structure purely to be shown later in proc. Would that be acceptable?

Was it the lack of a dentry that was really the problem? I thought it was just the fact that not all filesystems support filehandles.

--b.
.

Quoted 16 lines · J. Bruce Fields

Quoted 15 lines · Pavel Emelyanov

Quoted 13 lines · Cyrill Gorcunov

Quoted 8 lines · Pavel Emelyanov

Quoted 6 lines

Quoted 4 lines

However, if you have some better ideas on what information about inode should be exported to the userspace please share.

Why not use name_to_handle(fd,...) and open_by_handle(handle,..) ?

Because we don't have an fd at hands by the time we need to know the handle.

Yeah, this might be not clear from patchset itself but inotify marks carry inodes inside kernel thus it's inodes what we can use when we fetch information about targets and put it into fdinfo output.

Quoted 8 lines

Quoted 5 lines

Al, Bruce, Aneesh,

What if we calculate the handle at the time we do have struct path at hands (i.e. when we create the inotify) and store it on the inotify structure purely to be shown later in proc. Would that be acceptable?

Was it the lack of a dentry that was really the problem? I thought it was just the fact that not all filesystems support filehandles.

Initial problem -- we don't know what is being watched by an inotify fd.

Having a dentry somewhere was the 1st attempt to solve this -- keep a path in inotify and show it when required. It doesn't work since holding a ref on path changes the behavior of watched inode (we cannot rename/unlink/remount it the same way as we could before patching the kernel).

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

Al, Bruce, Aneesh,

What if we calculate the handle at the time we do have struct path at hands (i.e. when we create the inotify) and store it on the inotify structure purely to be shown later in proc. Would that be acceptable?

Was it the lack of a dentry that was really the problem? I thought it was just the fact that not all filesystems support filehandles.

Quoted 6 lines

Initial problem -- we don't know what is being watched by an inotify fd.

Having a dentry somewhere was the 1st attempt to solve this -- keep a path in inotify and show it when required. It doesn't work since holding a ref on path changes the behavior of watched inode (we cannot rename/unlink/remount it the same way as we could before patching the kernel).

OK. So if you don't mind the fact that there are filesystems with inotify support but not filehandle support, then I think generating a filehandle early as you describe would work. I guess it's a little more memory per watched inode.

--b.
.

OK. So if you don't mind the fact that there are filesystems with inotify support but not filehandle support, then I think generating a filehandle early as you describe would work. I guess it's a little more memory per watched inode.

--b.

On 08/21/2012 03:29 PM, J. Bruce Fields wrote:
<>

Quoted 5 lines

OK. So if you don't mind the fact that there are filesystems with inotify support but not filehandle support, then I think generating a filehandle early as you describe would work. I guess it's a little more memory per watched inode.

For the minority of FSs that do not have a filehandle support it should be easy to generate a generic one, that should work 95% of the time.

Are we guaranteed that after the checkpoint restore the version of the Kernel is the same as the one that did the checkpoint? If yes then I don't see any problem.

Quoted 1 line

--b.

Cheers
Boaz

Quoted 2 lines · Boaz Harrosh

On 08/21/2012 03:29 PM, J. Bruce Fields wrote:
<>

Strictly speaking -- no we don't. Migration should to work across kernel versions (from older to newer). Why kernel version matters in this case?

For the minority of FSs that do not have a filehandle support it should be easy to generate a generic one, that should work 95% of the time.

Quoted 6 lines

Initial problem -- we don't know what is being watched by an inotify fd.

Having a dentry somewhere was the 1st attempt to solve this -- keep a path in inotify and show it when required. It doesn't work since holding a ref on path changes the behavior of watched inode (we cannot rename/unlink/remount it the same way as we could before patching the kernel).

OK. So if you don't mind the fact that there are filesystems with inotify support but not filehandle support, then I think generating a filehandle early as you describe would work. I guess it's a little more memory per watched inode.

Quoted 31 lines · Aneesh Kumar K.V

Quoted 29 lines · Pavel Emelyanov

Quoted 27 lines · J. Bruce Fields

Quoted 16 lines · Cyrill Gorcunov

Quoted 15 lines · J. Bruce Fields

Quoted 13 lines · Cyrill Gorcunov

Quoted 9 lines · Aneesh Kumar K.V

Quoted 4 lines · Cyrill Gorcunov

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

doesn't name_to_handle_at() work for you ? It also allows to get a file handle using file descriptor.

Hi, sorry for dealy. Well, the last idea is to get rid of this helper, I've sent out an updated version where ino+dev is only printed.

Quoted 8 lines

Quoted 3 lines

I don't understand how ino and dev are useful to you, though, if you're still hoping to be able to look up inodes using this information later on.

Hi Bruce, I believe having ino+dev is better than nothing. Otherwise we simply have no clue which targets are bound to inotify mark. Sometime (!) we can try to generate fhandle in userspace from this ino+dev bundle and then open the target file.

That's insufficient to generate a filehandle in general.

Quoted 8 lines

Quoted 8 lines

Yes, sure, but for live migration having inode and device is enough and that's why. We can use two ways of having a filesystem on the target machine in the same state (from paths points of view) as it was on destination one:

1. copy file tree in a rsync manner
2. copy a virtual disk image file

In the 1st case we can map inode number to path easily, since we iterate over a filesystem

Quoted 7 lines

Quoted 4 lines

anyway. I agree, that rsync is not perfect for migration but still.

In the 2nd case we can generate filehandle out of an inode number only since we _do_ know that inode will not get reused.

If you are going to to use open_by_handle, then that handle is not sufficient right ? Or do you have open_by_inode ? as part of c/r ?

Why? For e.g. ext4 you can construct a handle in userspace and open by it.

Quoted 7 lines

Quoted 5 lines

However, if you have some better ideas on what information about inode should be exported to the userspace please share.

Why not use name_to_handle(fd,...) and open_by_handle(handle,..) ?

Because we don't have an fd at hands by the time we need to know the handle.

Quoted 10 lines

Quoted 5 lines

Quoted 5 lines

(Also: there's the usual inode-number aliasing problem: the inode number could get reused by another file. Unless you know the file is being held open the whole time.)

-aneesh

.

Thanks,
Pavel

Quoted 36 lines · Pavel Emelyanov

Quoted 35 lines · Aneesh Kumar K.V

Quoted 33 lines · Pavel Emelyanov

Quoted 21 lines · J. Bruce Fields

Quoted 20 lines · Cyrill Gorcunov

Quoted 18 lines · J. Bruce Fields

Quoted 8 lines · Cyrill Gorcunov

Quoted 7 lines · Aneesh Kumar K.V

Quoted 4 lines · Cyrill Gorcunov

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

Quoted 5 lines

Quoted 2 lines

doesn't name_to_handle_at() work for you ? It also allows to get a file handle using file descriptor.

Hi, sorry for dealy. Well, the last idea is to get rid of this helper, I've sent out an updated version where ino+dev is only printed.

I don't understand how ino and dev are useful to you, though, if you're still hoping to be able to look up inodes using this information later on.

Quoted 6 lines

Quoted 4 lines

Hi Bruce, I believe having ino+dev is better than nothing. Otherwise we simply have no clue which targets are bound to inotify mark. Sometime (!) we can try to generate fhandle in userspace from this ino+dev bundle and then open the target file.

That's insufficient to generate a filehandle in general.

Yes, sure, but for live migration having inode and device is enough and that's why. We can use two ways of having a filesystem on the target machine in the same state (from paths points of view) as it was on destination one:

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

1. copy file tree in a rsync manner
2. copy a virtual disk image file

In the 1st case we can map inode number to path easily, since we iterate over a filesystem

OK. Then you don't care about unlinked files?

If the filesystem's frozen by the time you get here, I suppose you could also just use paths?

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

anyway. I agree, that rsync is not perfect for migration but still.

In the 2nd case we can generate filehandle out of an inode number only since we _do_ know that inode will not get reused.

Quoted 4 lines

Quoted 2 lines

If you are going to to use open_by_handle, then that handle is not sufficient right ? Or do you have open_by_inode ? as part of c/r ?

Why? For e.g. ext4 you can construct a handle in userspace and open by it.

If it's a real filehandle you want, in general you don't want to construct it in userspace--depending on the filesystem it may require filesystem-specific knowledge.

--b.

Quoted 41 lines · Aneesh Kumar K.V

Quoted 39 lines · Pavel Emelyanov

Quoted 21 lines · J. Bruce Fields

Quoted 20 lines · Cyrill Gorcunov

Quoted 18 lines · J. Bruce Fields

Quoted 8 lines · Cyrill Gorcunov

Quoted 7 lines · Aneesh Kumar K.V

Quoted 4 lines · Cyrill Gorcunov

To provide fsnotify object inodes being watched without binding to alphabetical path we need to encode them with exportfs help. This patch adds a helper which operates with plain inodes directly.

Quoted 5 lines

Quoted 2 lines

doesn't name_to_handle_at() work for you ? It also allows to get a file handle using file descriptor.

Hi, sorry for dealy. Well, the last idea is to get rid of this helper, I've sent out an updated version where ino+dev is only printed.

I don't understand how ino and dev are useful to you, though, if you're still hoping to be able to look up inodes using this information later on.

Quoted 6 lines

Quoted 4 lines

Hi Bruce, I believe having ino+dev is better than nothing. Otherwise we simply have no clue which targets are bound to inotify mark. Sometime (!) we can try to generate fhandle in userspace from this ino+dev bundle and then open the target file.

That's insufficient to generate a filehandle in general.

Yes, sure, but for live migration having inode and device is enough and that's why. We can use two ways of having a filesystem on the target machine in the same state (from paths points of view) as it was on destination one:

1. copy file tree in a rsync manner
2. copy a virtual disk image file

In the 1st case we can map inode number to path easily, since we iterate over a filesystem anyway. I agree, that rsync is not perfect for migration but still.

Quoted 5 lines

Quoted 2 lines

In the 2nd case we can generate filehandle out of an inode number only since we _do_ know that inode will not get reused.

If you are going to to use open_by_handle, then that handle is not sufficient right ? Or do you have open_by_inode ? as part of c/r ?

Why? For e.g. ext4 you can construct a handle in userspace and open by it.

Quoted 2 lines · Pavel Emelyanov

Why? For e.g. ext4 you can construct a handle in userspace and open by it.

open_by_handle use exportfs_decode_fh which use file system specific fh_to_dentry

foe ext4 we require a generation number

		inode = get_inode(sb, fid->i32.ino, fid->i32.gen);

For brtfs

	objectid = fid->objectid;
	root_objectid = fid->root_objectid;
	generation = fid->gen;

	return btrfs_get_dentry(sb, objectid, root_objectid, generation, 1);

Quoted 5 lines

Quoted 2 lines · Pavel Emelyanov

Why? For e.g. ext4 you can construct a handle in userspace and open by it.

Quoted 7 lines

open_by_handle use exportfs_decode_fh which use file system specific fh_to_dentry

foe ext4 we require a generation number

		inode = get_inode(sb, fid->i32.ino, fid->i32.gen);

Hi Aneesh, yes we need i_generation but for ext3/4 it could be fetched with ioctl as far as i see.

Quoted 7 lines

For brtfs

	objectid = fid->objectid;
	root_objectid = fid->root_objectid;
	generation = fid->gen;

	return btrfs_get_dentry(sb, objectid, root_objectid, generation, 1);

For btrfs it become more complex. But still the last version I'm about to send for review today (once everything get tested) will provide fhandle carried with inotify mark _and_ inode number and device. This information should be enough for us. After all having inode and device should allow us to figure out the fs used on inotify target.

Cyrill
--

Quoted 6 lines

Quoted 6 lines

For brtfs

	objectid = fid->objectid;
	root_objectid = fid->root_objectid;
	generation = fid->gen;

Quoted 7 lines

Quoted 1 line

	return btrfs_get_dentry(sb, objectid, root_objectid, generation, 1);

For btrfs it become more complex. But still the last version I'm about to send for review today (once everything get tested) will provide fhandle carried with inotify mark _and_ inode number and device. This information should be enough for us. After all having inode and device should allow us to figure out the fs used on inotify target.

Sorry if it's a really stupid question but I didn't follow deeply the thread. How can you provide a device if the fs is mounted on "none" (ex tmpfs)? In this case you can't associate device <=> fs, because you haven't got a /dev/something.