Re: [RFC][PATCH] inotify 0.10.0

Andrew Morton
2004-09-28T01:51:17Z

Robert Love <rml@novell.com> 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)