Attached patch makes dnotify compile-time configurable via
CONFIG_DNOTIFY. The patch stands alone from the inotify patch, although
it really makes most sense in that context. Maybe the tiny kernel will
find it useful as well.
On a desktop system with inotify, there is no reason to enable dnotify
support. The primary user of dnotify (FAM) already supports inotify
(via Gamin). Many non-desktop systems do not use dnotify.
There was a sysctl, dir_notify_enable, for run-time configuration of
dnotify. I removed it. It makes no sense to me, even without
compile-time configuration, to disable this feature. With the ability
to compile dnotify away, a sysctl makes even less sense (which, as it
made zero sense before, makes no sense in and of itself now).
Disabling CONFIG_DNOTIFY saves a couple hundred bytes off of vmlinux.
When disabled, fcntl(fd, F_NOTIFY, ...) returns -EINVAL.
Best,
Robert Love
On Fri, Oct 01, 2004 at 02:24:34AM -0400, Robert Love wrote:
> Attached patch makes dnotify compile-time configurable via
> CONFIG_DNOTIFY. The patch stands alone from the inotify patch, although
> it really makes most sense in that context. Maybe the tiny kernel will
> find it useful as well.
Indeed, it's been useful for months. Unfortunately my development
boxes are still mothballed so progress upstream is stalled.
> Disabling CONFIG_DNOTIFY saves a couple hundred bytes off of vmlinux.
Hmm, thought I saved at least 1 or 2k.
--
Mathematics is the supreme nostalgia of our time.
On Fri, 2004-10-01 at 10:11 -0500, Matt Mackall wrote:
> Indeed, it's been useful for months. Unfortunately my development
> boxes are still mothballed so progress upstream is stalled.
Oh, great...
/me finds your broken out patches ...
Ah, you ifdef'ed out the dnotify variables in the inode structure. That
was the original reason I did this--doh.
In mine, I remove the sysctl entirely, which you don't. I also add some
documentation to dnotify.txt. Not a big difference.
> > Disabling CONFIG_DNOTIFY saves a couple hundred bytes off of vmlinux.
>
> Hmm, thought I saved at least 1 or 2k.
I thought my number sounded suspect. Dunno.
Here is a respun patch ifdef'ing out the dnotify member variables in
struct inode. This one is diff'ed against my inode tree since the
struct inode changes break one path or the other and, well, I like to
mix things up.
John, want to merge this into the inotify patch?
Thanks, Matt.
Robert Love
On Fri, 01 Oct 2004 11:21:16 -0400 Robert Love wrote:
| On Fri, 2004-10-01 at 10:11 -0500, Matt Mackall wrote:
|
| > Indeed, it's been useful for months. Unfortunately my development
| > boxes are still mothballed so progress upstream is stalled.
|
| Oh, great...
|
| /me finds your broken out patches ...
|
| Ah, you ifdef'ed out the dnotify variables in the inode structure. That
| was the original reason I did this--doh.
|
| In mine, I remove the sysctl entirely, which you don't. I also add some
| documentation to dnotify.txt. Not a big difference.
|
| > > Disabling CONFIG_DNOTIFY saves a couple hundred bytes off of vmlinux.
| >
| > Hmm, thought I saved at least 1 or 2k.
|
| I thought my number sounded suspect. Dunno.
|
| Here is a respun patch ifdef'ing out the dnotify member variables in
| struct inode. This one is diff'ed against my inode tree since the
| struct inode changes break one path or the other and, well, I like to
| mix things up.
|
| John, want to merge this into the inotify patch?
I'd rather see inotify additions and dnotify config options kept
separate. They may serve a similar purpose, but inotify doesn't
replace the dnotify API. If the latter were true, combining
them would make sense IMO.
| Thanks, Matt.
--
~Randy
On Fri, 2004-10-01 at 08:31 -0700, Randy.Dunlap wrote:
> I'd rather see inotify additions and dnotify config options kept
> separate. They may serve a similar purpose, but inotify doesn't
> replace the dnotify API. If the latter were true, combining
> them would make sense IMO.
I'm not really following.
Whether or not dnotify is a configuration option is separate, and could
go into the kernel either way.
But what matters if our inotify patch also carries the change? People
with inotify definitely DO want this patch, because they don't need
dnotify. Not much uses dnotify--it is a pain to use--and inotify
replaces its functionality.
It is also a practical move: the diffs conflict.
Robert Love
On Fri, 01 Oct 2004 11:44:39 -0400 Robert Love wrote:
| On Fri, 2004-10-01 at 08:31 -0700, Randy.Dunlap wrote:
|
| > I'd rather see inotify additions and dnotify config options kept
| > separate. They may serve a similar purpose, but inotify doesn't
| > replace the dnotify API. If the latter were true, combining
| > them would make sense IMO.
|
| I'm not really following.
|
| Whether or not dnotify is a configuration option is separate, and could
| go into the kernel either way.
Sorry, that's about all that I was trying to say. If patches A & B
are logically separate, don't combine them. Nothing new there.
| But what matters if our inotify patch also carries the change? People
| with inotify definitely DO want this patch, because they don't need
| dnotify. Not much uses dnotify--it is a pain to use--and inotify
| replaces its functionality.
Well, the patch shouldn't remove dnotify unconditionally, or not
until we have that elusive stable kernel series that people keep
mentioning elsewhere.
| It is also a practical move: the diffs conflict.
I see.
--
~Randy
MOTD: Always include version info.
(Again. Sometimes I think ln -s /usr/src/linux/.config .signature)
On Fri, 2004-10-01 at 08:58 -0700, Randy.Dunlap wrote:
> Sorry, that's about all that I was trying to say. If patches A & B
> are logically separate, don't combine them. Nothing new there.
In this case I offer A or A&B.
> Well, the patch shouldn't remove dnotify unconditionally, or not
> until we have that elusive stable kernel series that people keep
> mentioning elsewhere.
No patch I posted removes dnotify unconditionally.
Robert Love
On Fri, Oct 01, 2004 at 01:01:55PM -0400, Robert Love wrote:
> On Fri, 2004-10-01 at 08:58 -0700, Randy.Dunlap wrote:
>
> > Sorry, that's about all that I was trying to say. If patches A & B
> > are logically separate, don't combine them. Nothing new there.
>
> In this case I offer A or A&B.
The configurable dnotify patch makes sense on its own and is perhaps
less contentious. Push it first and resolve the conflicts with inotify
later..
--
Mathematics is the supreme nostalgia of our time.
On Fri, 2004-10-01 at 12:27 -0500, Matt Mackall wrote:
> > In this case I offer A or A&B.
>
> The configurable dnotify patch makes sense on its own and is perhaps
> less contentious. Push it first and resolve the conflicts with inotify
> later..
..the A above is dnotify by itself. All I said--and I don't know why
anyone questioned it--is that I want to put dnotify's configurability in
the inotify patch. They make sense together, and the patches conflict
anyways. I can do this.
CONFIG_DNOTIFY makes sense either way, on its own or (more so) with
inotify, and I already posted the patch separately.
Why are we even talking about this?!?
Robert Love
On Fri, 01 Oct 2004 13:30:08 -0400 Robert Love wrote:
| On Fri, 2004-10-01 at 12:27 -0500, Matt Mackall wrote:
|
| > > In this case I offer A or A&B.
| >
| > The configurable dnotify patch makes sense on its own and is perhaps
| > less contentious. Push it first and resolve the conflicts with inotify
| > later..
|
| ..the A above is dnotify by itself. All I said--and I don't know why
| anyone questioned it--is that I want to put dnotify's configurability in
| the inotify patch. They make sense together, and the patches conflict
| anyways. I can do this.
You can do that. Go ahead.
Even if it isn't clear that they make sense together.
Lots of patches conflict, but that's no grand reason to combine them.
| CONFIG_DNOTIFY makes sense either way, on its own or (more so) with
| inotify, and I already posted the patch separately.
|
| Why are we even talking about this?!?
I'm going quiet on it...
--
~Randy
On Fri, 2004-10-01 at 10:30 -0700, Randy.Dunlap wrote:
> You can do that. Go ahead.
> Even if it isn't clear that they make sense together.
It is not clear that users of inotify probably don't need dnotify?
Robert Love
On Fri, 01 Oct 2004 13:37:38 -0400 Robert Love wrote:
| On Fri, 2004-10-01 at 10:30 -0700, Randy.Dunlap wrote:
|
| > You can do that. Go ahead.
| > Even if it isn't clear that they make sense together.
|
| It is not clear that users of inotify probably don't need dnotify?
I expect that much is clear.
It is not clear that there are only applications on the system
which use inotify and that there are none that use dnotify.
I wanted this to end, too.
--
~Randy