mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Eric Paris <eparis@redhat.com>
To: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: linux-kernel@vger.kernel.org, hch@infradead.org,
	akpm@linux-foundation.org
Subject: Re: [PATCH 3/3] fsnotify: use the new open-exec hook for inotify and dnotify
Date: Mon, 15 Dec 2008 20:52:45 -0500	[thread overview]
Message-ID: <1229392365.23523.19.camel@localhost.localdomain> (raw)
In-Reply-To: <20081216100418.06CD.KOSAKI.MOTOHIRO@jp.fujitsu.com>

On Tue, 2008-12-16 at 10:12 +0900, KOSAKI Motohiro wrote:
> > diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> > index 88265dd..a7122c6 100644
> > --- a/include/linux/fsnotify.h
> > +++ b/include/linux/fsnotify.h
> > @@ -170,6 +170,12 @@ static inline void fsnotify_modify(struct dentry *dentry)
> >   */
> >  static inline void fsnotify_open_exec(struct file *file)
> >  {
> > +	struct dentry *dentry = file->f_path.dentry;
> > +	struct inode *inode = dentry->d_inode;
> > +
> > +	dnotify_parent(dentry, DN_ACCESS);
> > +	inotify_dentry_parent_queue_event(dentry, IN_ACCESS, 0, dentry->d_name.name);
> > +	inotify_inode_queue_event(inode, IN_ACCESS, 0, NULL, NULL);
> >  }
> 
> Current fsnotify_open() has following code
> 
> 	static inline void fsnotify_open(struct dentry *dentry)
> 	{
> 	        struct inode *inode = dentry->d_inode;
> 	        u32 mask = IN_OPEN;
> 	
> 	        if (S_ISDIR(inode->i_mode))
> 	                mask |= IN_ISDIR;
> 	
> 	        inotify_dentry_parent_queue_event(dentry, mask, 0, dentry->d_name.name);
> 	        inotify_inode_queue_event(inode, mask, 0, NULL, NULL);
> 	}
> 
> they are two different.
> 
> 1) Call dnotify_parent() or not
> 2) Use IN_OPEN or IN_ACCESS
> 
> The patch description doesn't explain any reason.
> 
> 
> IOW, IN_ACCESS is usually used by read(). but linux has demand paging
> mechanism. then exec() only do open and mmap.
> actual reading is processed by page fault.
> 
> I guess you have the reason of this design choice.
> but it isn't described.

The original logic was all predicated on my thoughts on how my new
fanotify would want these events and how I felt that open for exec was
worth the separate hook.  None of that is useful at this time and in any
case IN_OPEN makes a lot more sense than IN_ACCESS.  Since you've got me
looking at these as freestanding patchs I do tend to think that the
easiest thing for now would be to just drop patch 2 and make the call
sites from patch 2 call fsnotify_open directly.

I'll resend in the morning a single patch to call directly to
fsnotify_open.  (and another single patch to immediately do the rename
that I want done which I'll send as the full normal diff since it'll be
freestanding)


  reply	other threads:[~2008-12-16  1:53 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-15 16:44 [PATCH 0/3] send notification events about exec Eric Paris
2008-12-15 16:44 ` [PATCH 1/3] filesystem notification: create fs/notify to contain all fs notification Eric Paris
2008-12-16  1:02   ` KOSAKI Motohiro
2008-12-16  1:37     ` Eric Paris
2008-12-16  1:46       ` KOSAKI Motohiro
2008-12-15 16:44 ` [PATCH 2/3] fsnotify: sys_execve and sys_uselib do not call into fsnotify Eric Paris
2008-12-16  1:02   ` KOSAKI Motohiro
2008-12-15 16:44 ` [PATCH 3/3] fsnotify: use the new open-exec hook for inotify and dnotify Eric Paris
2008-12-16  1:12   ` KOSAKI Motohiro
2008-12-16  1:52     ` Eric Paris [this message]
2008-12-16  2:01       ` KOSAKI Motohiro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1229392365.23523.19.camel@localhost.localdomain \
    --to=eparis@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=hch@infradead.org \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®