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 <[email protected]>
---
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
On Fri 03-10-14 10:19:30, Heinrich Schuchardt wrote:
> 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
> Signed-off-by: Heinrich Schuchardt <[email protected]>
> ---
> 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 <[email protected]>
SUSE Labs, CR
On 06.10.2014 16:12, Jan Kara wrote:
> On Fri 03-10-14 10:19:30, Heinrich Schuchardt wrote:
>> 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
>
>> Signed-off-by: Heinrich Schuchardt <[email protected]>
>> ---
>> 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
>>
On Mon 06-10-14 21:10:25, Heinrich Schuchardt wrote:
> On 06.10.2014 16:12, Jan Kara wrote:
> >On Fri 03-10-14 10:19:30, Heinrich Schuchardt wrote:
> >>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.
OK, makes sense. You can add:
Reviewed-by: Jan Kara <[email protected]>
Honza
> >>Signed-off-by: Heinrich Schuchardt <[email protected]>
> >>---
> >> 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 <[email protected]>
SUSE Labs, CR
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
On 07.10.2014 20:05, Jan Kara wrote:
> On Mon 06-10-14 21:10:25, Heinrich Schuchardt wrote:
>> On 06.10.2014 16:12, Jan Kara wrote:
>>> On Fri 03-10-14 10:19:30, Heinrich Schuchardt wrote:
>>>> 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.
> OK, makes sense. You can add:
> Reviewed-by: Jan Kara <[email protected]>
>
> Honza
>>>> Signed-off-by: Heinrich Schuchardt <[email protected]>
>>>> ---
>>>> 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
>>>>
>>
On Tue, 07 Oct 2014 20:24:02 +0200 Heinrich Schuchardt <[email protected]> wrote:
> 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.
:: 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 <[email protected]>
Signed-off-by: Heinrich Schuchardt <[email protected]>
---
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