Re: [RFC][PATCH] inotify 0.10.0

[Andrew Morton]

Robert Love <rml@xxxxxxxxxx> wrote:
>
> > > +	memset(dev->bitmask, 0,
> > > +	  sizeof(unsigned long) * MAX_INOTIFY_DEV_WATCHERS / BITS_PER_LONG);
> > 
> > What purpose does this bitmask serve, anyway??
> 
> Bitmask of allocated/unallocated watcher descriptors.

Can you expand on that?  Why do we need such a bitmap?

Would an idr tree be more appropriate?

> We _could_ take a fixed minor...
> 
> > > +struct inotify_event {
> > > +	int wd;
> > > +	int mask;
> > > +	int cookie;
> > > +	char filename[INOTIFY_FILENAME_MAX];
> > > +};
> > 
> > yeah, that's not very nice.  Better to kmalloc the pathname.
> 
> That is the structure that we communicate with to user-space.

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.


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?

Bonus marks for not having a backpointer from the inode to the new struct ;)

(Still wondering what those timers are doing in there, btw)
-
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/