lkml.org 
[lkml]   [2006]   [Apr]   [10]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC][PATCH] inotify kernel api
FromJohn McCutchan <>
DateMon, 10 Apr 2006 14:36:28 -0400
On Thu, 2006-04-06 at 13:06 -0400, Amy Griffis wrote:
> The following patch against 2.6.17-rc1-mm1 introduces a kernel API for inotify.
>
> Event processing is unchanged except for the indirection added for the
> callback.  In inotify_user.c, an additional per-device mutex is held
> while adding watches to prevent adding the same watch twice.  The
> design retains the original assumption that there will be more watches
> per inotify_handle than watches on any given inode, and performs the
> search for existing watches accordingly.
> 

Why do we need the 'up_mutex' mutex? Was this a bug in the old code? 

> This patch makes the inotify_watch public so it can be embedded in callers' own
> watch structures, which avoids the use of a void ptr to caller data.
> Even though inotify_watch is public, callers must use the established
> interfaces to access inotify_watch contents.  Was this the best
> choice?
> 

I suppose this is an alright change. As long as it is understood that
there is no guarantee about the layout of the inotify_watch structure. A
comment in inotify.h should do.

> I think the locking may be less than ideal, as the
> inode->inotify_mutex must be held to traverse the inode's watchlist,
> and thus must be held during the callback.  The result is that the
> caller can't hold any locks taken during callback processing while
> calling any of the published inotify interfaces, making
> synchronization a little more difficult.
> 

Well, I think it's up for the kernel consumers to decide whether or not
this is acceptable. It's probably a good idea to come up with some use
cases for the kernel API and see if this _is_ a problem. Since the
callbacks will be run inside the VFS ops, they need to be small and
fast, so they probably should just be putting the event on a list and
handling it later.

Looking over the patch, nothing jumps out at me as being wrong. But some
stress testing would convince me faster than my eyes can.

-- 
John McCutchan <john@johnmccutchan.com>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-04-10 20:38    [from the cache]
©2003-2008