Discussion:
[PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events
(too old to reply)
Heinrich Schuchardt
2014-10-03 08:19:30 UTC
Permalink
Raw Message
The fanotify and the inotify API can used to monitor changes of the file
system.

System call fallocate modifies files. Hence it should trigger the corresponding
fanotify (FAN_MODIFY) and inotify (IN_MODIFY) events.

This patch adds the missing call to fsnotify_modify.

Signed-off-by: Heinrich Schuchardt <***@gmx.de>
---
fs/open.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/fs/open.c b/fs/open.c
index d6fd3ac..03aa8e5 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -295,6 +295,11 @@ int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)

sb_start_write(inode->i_sb);
ret = file->f_op->fallocate(file, mode, offset, len);
+
+ /* Create inotify and fanotify events. */
+ if (ret == 0)
+ fsnotify_modify(file);
+
sb_end_write(inode->i_sb);
return ret;
}
--
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Jan Kara
2014-10-06 14:12:27 UTC
Permalink
Raw Message
Post by Heinrich Schuchardt
The fanotify and the inotify API can used to monitor changes of the file
system.
System call fallocate modifies files. Hence it should trigger the corresponding
fanotify (FAN_MODIFY) and inotify (IN_MODIFY) events.
This patch adds the missing call to fsnotify_modify.
Well, there are different fallocate() commands and e.g. pure
FALLOC_FL_KEEP_SIZE call will not change any data in the file. I'm not sure
how much we care but I wanted to point that out...

Honza
Post by Heinrich Schuchardt
---
fs/open.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/open.c b/fs/open.c
index d6fd3ac..03aa8e5 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -295,6 +295,11 @@ int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
sb_start_write(inode->i_sb);
ret = file->f_op->fallocate(file, mode, offset, len);
+
+ /* Create inotify and fanotify events. */
+ if (ret == 0)
+ fsnotify_modify(file);
+
sb_end_write(inode->i_sb);
return ret;
}
--
2.1.0
--
Jan Kara <***@suse.cz>
SUSE Labs, CR
Heinrich Schuchardt
2014-10-06 19:10:25 UTC
Permalink
Raw Message
Post by Jan Kara
Post by Heinrich Schuchardt
The fanotify and the inotify API can used to monitor changes of the file
system.
System call fallocate modifies files. Hence it should trigger the corresponding
fanotify (FAN_MODIFY) and inotify (IN_MODIFY) events.
This patch adds the missing call to fsnotify_modify.
Well, there are different fallocate() commands and e.g. pure
FALLOC_FL_KEEP_SIZE call will not change any data in the file. I'm not sure
how much we care but I wanted to point that out...
The most interesting case is FALLOC_FL_COLLAPSE_RANGE because this value
allows to create arbitrary file content from random data. Hence I think
we really need to create FAN_MODIFY in this case.

As the fallocate(2) man page teaches:
After a successful call, subsequent writes into the range specified by
offset and len are guaranteed not to fail because of lack of disk space.

So calling fallocate(fd, FALLOC_FL_KEEP_SIZE, offset, len) may result in
different outcomes of a subsequent write depending on the values of
offset and len.

Calling fallocate for a region already zeroed will not result in any
data change.

I would like to compare fallocate() with write().

When we call write() we always create a FAN_MODIFY event even in the
case of overwriting with identical data.

So event FAN_MODIFY does not provide any guarantee that data was
actually changed.

In analogy to write() I suggest to keep the logic for fallocate() as
trivial as possible:
If fallocate() succeeds, create IN_MODIFY and FAN_MODIFY events.

Best regards

Heinrich Schuchardt
Post by Jan Kara
Post by Heinrich Schuchardt
---
fs/open.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/open.c b/fs/open.c
index d6fd3ac..03aa8e5 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -295,6 +295,11 @@ int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
sb_start_write(inode->i_sb);
ret = file->f_op->fallocate(file, mode, offset, len);
+
+ /* Create inotify and fanotify events. */
+ if (ret == 0)
+ fsnotify_modify(file);
+
sb_end_write(inode->i_sb);
return ret;
}
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Jan Kara
2014-10-07 18:05:59 UTC
Permalink
Raw Message
Post by Heinrich Schuchardt
Post by Jan Kara
Post by Heinrich Schuchardt
The fanotify and the inotify API can used to monitor changes of the file
system.
System call fallocate modifies files. Hence it should trigger the corresponding
fanotify (FAN_MODIFY) and inotify (IN_MODIFY) events.
This patch adds the missing call to fsnotify_modify.
Well, there are different fallocate() commands and e.g. pure
FALLOC_FL_KEEP_SIZE call will not change any data in the file. I'm not sure
how much we care but I wanted to point that out...
The most interesting case is FALLOC_FL_COLLAPSE_RANGE because this
value allows to create arbitrary file content from random data.
Hence I think we really need to create FAN_MODIFY in this case.
After a successful call, subsequent writes into the range specified
by offset and len are guaranteed not to fail because of lack of disk
space.
So calling fallocate(fd, FALLOC_FL_KEEP_SIZE, offset, len) may
result in different outcomes of a subsequent write depending on the
values of offset and len.
Calling fallocate for a region already zeroed will not result in any
data change.
I would like to compare fallocate() with write().
When we call write() we always create a FAN_MODIFY event even in the
case of overwriting with identical data.
So event FAN_MODIFY does not provide any guarantee that data was
actually changed.
In analogy to write() I suggest to keep the logic for fallocate() as
If fallocate() succeeds, create IN_MODIFY and FAN_MODIFY events.
OK, makes sense. You can add:
Reviewed-by: Jan Kara <***@suse.cz>

Honza
Post by Heinrich Schuchardt
Post by Jan Kara
Post by Heinrich Schuchardt
---
fs/open.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/open.c b/fs/open.c
index d6fd3ac..03aa8e5 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -295,6 +295,11 @@ int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
sb_start_write(inode->i_sb);
ret = file->f_op->fallocate(file, mode, offset, len);
+
+ /* Create inotify and fanotify events. */
+ if (ret == 0)
+ fsnotify_modify(file);
+
sb_end_write(inode->i_sb);
return ret;
}
--
2.1.0
--
Jan Kara <***@suse.cz>
SUSE Labs, CR
Heinrich Schuchardt
2014-10-07 18:24:02 UTC
Permalink
Raw Message
Hello Andrew,

the patch in
https://lkml.org/lkml/2014/10/3/56
and cited below was reviewed by Jan Kara.

Please, add it to the MM tree.

Best regards

Heinrich Schuchardt
Post by Heinrich Schuchardt
Post by Jan Kara
Post by Heinrich Schuchardt
The fanotify and the inotify API can used to monitor changes of the file
system.
System call fallocate modifies files. Hence it should trigger the corresponding
fanotify (FAN_MODIFY) and inotify (IN_MODIFY) events.
This patch adds the missing call to fsnotify_modify.
Well, there are different fallocate() commands and e.g. pure
FALLOC_FL_KEEP_SIZE call will not change any data in the file. I'm not sure
how much we care but I wanted to point that out...
The most interesting case is FALLOC_FL_COLLAPSE_RANGE because this
value allows to create arbitrary file content from random data.
Hence I think we really need to create FAN_MODIFY in this case.
After a successful call, subsequent writes into the range specified
by offset and len are guaranteed not to fail because of lack of disk
space.
So calling fallocate(fd, FALLOC_FL_KEEP_SIZE, offset, len) may
result in different outcomes of a subsequent write depending on the
values of offset and len.
Calling fallocate for a region already zeroed will not result in any
data change.
I would like to compare fallocate() with write().
When we call write() we always create a FAN_MODIFY event even in the
case of overwriting with identical data.
So event FAN_MODIFY does not provide any guarantee that data was
actually changed.
In analogy to write() I suggest to keep the logic for fallocate() as
If fallocate() succeeds, create IN_MODIFY and FAN_MODIFY events.
Honza
Post by Heinrich Schuchardt
Post by Jan Kara
Post by Heinrich Schuchardt
---
fs/open.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/fs/open.c b/fs/open.c
index d6fd3ac..03aa8e5 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -295,6 +295,11 @@ int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
sb_start_write(inode->i_sb);
ret = file->f_op->fallocate(file, mode, offset, len);
+
+ /* Create inotify and fanotify events. */
+ if (ret == 0)
+ fsnotify_modify(file);
+
sb_end_write(inode->i_sb);
return ret;
}
--
2.1.0
Andrew Morton
2014-10-14 22:43:49 UTC
Permalink
Raw Message
Post by Heinrich Schuchardt
Hello Andrew,
the patch in
https://lkml.org/lkml/2014/10/3/56
and cited below was reviewed by Jan Kara.
Jan wondered why we generate events for FALLOC_FL_KEEP_SIZE, so other
people will wonder the same thing. We should tell them. Via code
comments and/or changelogging.

Any question which a reviewer asks should be viewed as a defect in the
patch. The patch isn't finished until people can read it without
having questions.

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Heinrich Schuchardt
2014-10-17 17:08:21 UTC
Permalink
Raw Message
:: Andrew Morton wrote:
::
:: Jan wondered why we generate events for FALLOC_FL_KEEP_SIZE, so other
:: people will wonder the same thing. We should tell them. Via code
:: comments and/or changelogging.
::
:: Any question which a reviewer asks should be viewed as a defect in the
:: patch. The patch isn't finished until people can read it without
:: having questions.

Please, find below a new version of the patch with an updated message and
additional comment lines in the code.



The fanotify and the inotify API can be used to monitor changes of the file
system. System call fallocate() modifies files. Hence it should trigger the
corresponding fanotify (FAN_MODIFY) and inotify (IN_MODIFY) events.
The most interesting case is FALLOC_FL_COLLAPSE_RANGE because this value
allows to create arbitrary file content from random data.

This patch adds the missing call to fsnotify_modify().

The FAN_MODIFY and IN_MODIFY event will be created when fallocate() succeeds.
It will even be created if the file length remains unchanged, e.g. when
calling fanotify with flag FALLOC_FL_KEEP_SIZE.

This logic was primarily chosen to keep the coding simple.

It resembles the logic of the write() system call.

When we call write() we always create a FAN_MODIFY event, even in the
case of overwriting with identical data.

Events FAN_MODIFY and IN_MODIFY do not provide any guarantee that data was
actually changed.

Furthermore even if if the filesize remains unchanged, fallocate() may influence
whether a subsequent write() will succeed and hence the fallocate() call
may be considered a modification.

The fallocate(2) man page teaches:
After a successful call, subsequent writes into the range specified by
offset and len are guaranteed not to fail because of lack of disk space.

So calling fallocate(fd, FALLOC_FL_KEEP_SIZE, offset, len) may result in
different outcomes of a subsequent write depending on the values of
offset and len.

Reviewed-by: Jan Kara <***@suse.cz>
Signed-off-by: Heinrich Schuchardt <***@gmx.de>
---
fs/open.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/fs/open.c b/fs/open.c
index d6fd3ac..550d464 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -295,6 +295,17 @@ int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)

sb_start_write(inode->i_sb);
ret = file->f_op->fallocate(file, mode, offset, len);
+
+ /*
+ * Create inotify and fanotify events.
+ *
+ * To keep the logic simple always create events if fallocate succeeds.
+ * This implies that events are even created if the file size remains
+ * unchanged, e.g. when using flag FALLOC_FL_KEEP_SIZE.
+ */
+ if (ret == 0)
+ fsnotify_modify(file);
+
sb_end_write(inode->i_sb);
return ret;
}
--
2.1.1

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