Re: [PATCH] Add FS_CLOSE_READ and FS_CLOSE_NOREAD flag to fsnotify

[Jan Kara]

On Wed 23-09-15 20:32:02, Patrick Puzik wrote:
> 1)They should be additionly sent. But at this point in time, there is
> no possibility to get directly notified if a readable writable was
> closed. Using these flags in combination with the existing ones is a
> way to handle this lack of functionality.
> 
> 2) I currently don't have an application that uses this new
> functionality, but it may come to an aid in the future for some
> developers who would find it avaiable whenever they need it.
> They can spare time and effort, don't you think?
> 
> In my opinion it would be better to have a:
>         1- CLOSE_READ
>         2- CLOSE_WRITE
>         3- CLOSE_READ_WRITE
> flag or so but I guess this wouldn't be possible because of compability
> issues. right?

You are right that it isn't possible to find out from the CLOSE_WRITE or
CLOSE_NOWRITE event whether the closed descriptor was readable or not.
However we don't merge new features in the kernel just because someone may
find some use for them. Usually we want to have at least a good idea what
the feature would be useful for (if not directly application that wants to
use such feature).

For example distinguishing between writeable and non-writeable desriptors
is useful for applications like virus scanners or caching daemons which can
recheck the file after writeable descriptor is closed (and don't have to
burn the cpu when non-writeable descriptor was closed). But for readable vs
non-readable descriptor I fail to see the usefulness... After all fsnotify
interface is about notification about changes in the filesystem and
readable vs non-readable is irelevant to that. So unless someone comes up
with a reasonable justification, I'm against merging such feature just
because it can be implemented.

								Honza

> On Tue, 2015-09-22 at 11:06 +0200, Jan Kara wrote:
> > On Sun 20-09-15 23:07:35, Patrick Puzik wrote:
> > > Attached is a patch which is a preparation for the later support of
> > > an
> > > event notification, when an unreadable or readable file was closed.
> > 
> > So I'm missing two things in this patch (or rather changelog):
> > 
> > 1) When should these events be generated? I can kind of infer this
> > from the
> > comment and event name but for example FS_CLOSE_WRITE and
> > FS_CLOSE_NOWRITE
> > are exclusive. I suppose we will generate FS_CLOSE_READ or
> > FS_CLOSE_NOREAD
> > in addition to that?
> > 
> > 2) What is the use case? I.e. what is your application doing that
> > current
> > notification events are not enough?
> > 
> > 								Honza
> > > 
> > > Signed-off-by: Patrick Puzik <p.puzik91@xxxxxxxxx>
> > > ---
> > >  include/linux/fsnotify_backend.h | 12 ++++++++----
> > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/include/linux/fsnotify_backend.h
> > > b/include/linux/fsnotify_backend.h
> > > index 533c440..7563679 100644
> > > --- a/include/linux/fsnotify_backend.h
> > > +++ b/include/linux/fsnotify_backend.h
> > > @@ -28,6 +28,8 @@
> > >  #define FS_ATTRIB		0x00000004	/* Metadata
> > > changed */
> > >  #define FS_CLOSE_WRITE		0x00000008	/*
> > > Writtable file was closed */
> > >  #define FS_CLOSE_NOWRITE	0x00000010	/* Unwrittable
> > > file closed */
> > > +#define FS_CLOSE_READ           0x00000011      /* Unreadable file
> > > closed */
> > > +#define FS_CLOSE_NOREAD         0x00000012      /* Readable file
> > > was closed */
> > >  #define FS_OPEN			0x00000020	/* File
> > > was opened */
> > >  #define FS_MOVED_FROM		0x00000040	/* File was
> > > moved from X */
> > >  #define FS_MOVED_TO		0x00000080	/* File was
> > > moved to Y */
> > > @@ -56,9 +58,10 @@
> > >  
> > >  /* This is a list of all events that may get sent to a parernt
> > > based on fs event
> > >   * happening to inodes inside that directory */
> > > -#define FS_EVENTS_POSS_ON_CHILD   (FS_ACCESS | FS_MODIFY |
> > > FS_ATTRIB |\
> > > -				   FS_CLOSE_WRITE |
> > > FS_CLOSE_NOWRITE | FS_OPEN |\
> > > -				   FS_MOVED_FROM | FS_MOVED_TO |
> > > FS_CREATE |\
> > > +#define FS_EVENTS_POSS_ON_CHILD   (FS_ACCESS | FS_MODIFY |
> > > FS_ATTRIB | \
> > > +				   FS_CLOSE_WRITE |
> > > FS_CLOSE_NOWRITE | \
> > > +				   FS_CLOSE_READ | FS_CLOSE_NOREAD
> > > | FS_OPEN | \
> > > +				   FS_MOVED_FROM | FS_MOVED_TO |
> > > FS_CREATE | \
> > >  				   FS_DELETE | FS_OPEN_PERM |
> > > FS_ACCESS_PERM)
> > >  
> > >  #define FS_MOVE			(FS_MOVED_FROM |
> > > FS_MOVED_TO)
> > > @@ -66,7 +69,8 @@
> > >  #define ALL_FSNOTIFY_PERM_EVENTS (FS_OPEN_PERM | FS_ACCESS_PERM)
> > >  
> > >  #define ALL_FSNOTIFY_EVENTS (FS_ACCESS | FS_MODIFY | FS_ATTRIB | \
> > > -			     FS_CLOSE_WRITE | FS_CLOSE_NOWRITE |
> > > FS_OPEN | \
> > > +			     FS_CLOSE_WRITE | FS_CLOSE_NOWRITE | \
> > > +			     FS_CLOSE_READ | FS_CLOSE_NOREAD |
> > > FS_OPEN | \
> > >  			     FS_MOVED_FROM | FS_MOVED_TO |
> > > FS_CREATE | \
> > >  			     FS_DELETE | FS_DELETE_SELF |
> > > FS_MOVE_SELF | \
> > >  			     FS_UNMOUNT | FS_Q_OVERFLOW |
> > > FS_IN_IGNORED | \
> > > -- 
> > > 2.4.3
> > > 
> > > 
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/