[PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events

[PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events

[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.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@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

Re: [PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events

[Jan Kara]

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 <xypron.glpk@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
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events

[Heinrich Schuchardt]

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 <xypron.glpk@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
>>

Re: [PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events

[Jan Kara]

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 <jack@suse.cz>

								Honza
> >>Signed-off-by: Heinrich Schuchardt <xypron.glpk@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
> >>
> 
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events

[Heinrich Schuchardt]

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 <jack@suse.cz>
>
> 								Honza
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@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
>>>>
>>

Re: [PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events

[Andrew Morton]

On Tue, 07 Oct 2014 20:24:02 +0200 Heinrich Schuchardt <xypron.glpk@gmx.de> 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.

[PATCH v2 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events

[Heinrich Schuchardt]

:: 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 <jack@suse.cz>
Signed-off-by: Heinrich Schuchardt <xypron.glpk@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