* [PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events [not found] <1412287587-5392-1-git-send-email-xypron.glpk@gmx.de> @ 2014-10-03 8:19 ` Heinrich Schuchardt 2014-10-06 14:12 ` Jan Kara 0 siblings, 1 reply; 7+ messages in thread From: Heinrich Schuchardt @ 2014-10-03 8:19 UTC (permalink / raw) To: Alexander Viro Cc: linux-fsdevel, linux-kernel, Jan Kara, Eric Paris, John McCutchan, Robert Love, Michael Kerrisk, 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events 2014-10-03 8:19 ` [PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events Heinrich Schuchardt @ 2014-10-06 14:12 ` Jan Kara 2014-10-06 19:10 ` Heinrich Schuchardt 0 siblings, 1 reply; 7+ messages in thread From: Jan Kara @ 2014-10-06 14:12 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Alexander Viro, linux-fsdevel, linux-kernel, Jan Kara, Eric Paris, John McCutchan, Robert Love, Michael Kerrisk 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events 2014-10-06 14:12 ` Jan Kara @ 2014-10-06 19:10 ` Heinrich Schuchardt 2014-10-07 18:05 ` Jan Kara 0 siblings, 1 reply; 7+ messages in thread From: Heinrich Schuchardt @ 2014-10-06 19:10 UTC (permalink / raw) To: Jan Kara Cc: Alexander Viro, linux-fsdevel, linux-kernel, Eric Paris, John McCutchan, Robert Love, Michael Kerrisk 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 >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events 2014-10-06 19:10 ` Heinrich Schuchardt @ 2014-10-07 18:05 ` Jan Kara 2014-10-07 18:24 ` Heinrich Schuchardt 0 siblings, 1 reply; 7+ messages in thread From: Jan Kara @ 2014-10-07 18:05 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Jan Kara, Alexander Viro, linux-fsdevel, linux-kernel, Eric Paris, John McCutchan, Robert Love, Michael Kerrisk 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events 2014-10-07 18:05 ` Jan Kara @ 2014-10-07 18:24 ` Heinrich Schuchardt 2014-10-14 22:43 ` Andrew Morton 0 siblings, 1 reply; 7+ messages in thread From: Heinrich Schuchardt @ 2014-10-07 18:24 UTC (permalink / raw) To: Andrew Morton Cc: Jan Kara, Alexander Viro, linux-fsdevel, linux-kernel, Eric Paris, John McCutchan, Robert Love, Michael Kerrisk 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 >>>> >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events 2014-10-07 18:24 ` Heinrich Schuchardt @ 2014-10-14 22:43 ` Andrew Morton 2014-10-17 17:08 ` [PATCH v2 " Heinrich Schuchardt 0 siblings, 1 reply; 7+ messages in thread From: Andrew Morton @ 2014-10-14 22:43 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Jan Kara, Alexander Viro, linux-fsdevel, linux-kernel, Eric Paris, John McCutchan, Robert Love, Michael Kerrisk 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events 2014-10-14 22:43 ` Andrew Morton @ 2014-10-17 17:08 ` Heinrich Schuchardt 0 siblings, 0 replies; 7+ messages in thread From: Heinrich Schuchardt @ 2014-10-17 17:08 UTC (permalink / raw) To: Andrew Morton Cc: Jan Kara, Alexander Viro, linux-fsdevel, linux-kernel, Eric Paris, John McCutchan, Robert Love, Michael Kerrisk, 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-10-17 17:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1412287587-5392-1-git-send-email-xypron.glpk@gmx.de>
2014-10-03 8:19 ` [PATCH 1/1] fallocate: create FAN_MODIFY and IN_MODIFY events Heinrich Schuchardt
2014-10-06 14:12 ` Jan Kara
2014-10-06 19:10 ` Heinrich Schuchardt
2014-10-07 18:05 ` Jan Kara
2014-10-07 18:24 ` Heinrich Schuchardt
2014-10-14 22:43 ` Andrew Morton
2014-10-17 17:08 ` [PATCH v2 " Heinrich Schuchardt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).