Re: [RFC][PATCH] inotify 0.10.0

[John McCutchan <ttb@tentacle.dhs.org>]

On Mon, 2004-09-27 at 22:14, Robert Love wrote:
> On Mon, 2004-09-27 at 21:41 -0700, Andrew Morton wrote:
> 
> > Can you expand on that?  Why do we need such a bitmap?
> 
> It is a unique cookie that identifies the exact object being watched
> (e.g. /home/rml or /etc/fstab).  I suspect John introduced it in lieu of
> the (device,inode) tuple when Al bitched, which makes sense.  Because we
> have only a single fd (this is one of the problems with dnotify, the 1:1
> relation between objects and file descriptors consumed) we need some
> other object to identify each watched object.
> 
> So John introduced watcher descriptors.  This bitmask keeps track of
> which descriptors are used versus unused.
> 
> > Would an idr tree be more appropriate?
> 
> Quite possibly.  I was originally thinking that idr's were too heavy,
> but if we can make the wd <-> inotify_watcher relation then they make
> perfect sense.
> 
> I'll look at making the conversion.

I only first heard about idr in last weeks LWN, I thought they might be
useful.

> 
> > In that case it looks rather 64-bit-unfriendly.  A 32-bit compiler will lay
> > that structure out differently from a 64-bit compiler.  Or not.  Hard to
> > say.  Perhaps something more defensive is needed here.
> 
> Well, no, since all known architectures are everything-is-32bit or LP64,
> as far as I know.  And padding would be the same.
> 
> And even if not, the only problem would be with 64-bit architectures and
> a 32-bit user-space.
> 
> Nonetheless, we should probably make the three int types be s32 or
> u32's, eh?  I will submit a patch.
> 
> > One other thing: the patch adds 16 bytes to struct inode, for a feature
> > which many users and most inodes will not use.  Unfortunate.
> > 
> > Is it possible to redesign things so that those four new fields are in a
> > standalone struct which points at the managed inode?  Joined at the hip
> > like journal_head and buffer_head?
> 
> We could probably get away with a single word-sized variable in the
> inode structure.
> 

Yep, we could toss everything in to a structure and only have a pointer
to it from the inode.

> > Bonus marks for not having a backpointer from the inode to the new struct ;)
> 
> Don't push your luck. ;-)
> 
> In school, I always felt the bonus was just showing off, what with the
> perfect score on the normal assignment.  But I will investigate.
> 
> > (Still wondering what those timers are doing in there, btw)
> 
> John?  I see what the timer does, but I am wondering why a timer _has_
> to do it?

We need a timer to wake up any processes blocking on a read() call. The
reason it has to be a timer is because the code paths that get run when
an event is queued are not safe places to wake up blocked processes (But
I a kernel amateur so I am probably wrong).

John