On Wed, Jun 07, 2000 at 05:25:13AM -0400, Jeff Garzik wrote:
> Andi Kleen wrote:
> > Jeff Garzik <jgarzik@mandrakesoft.com> writes:
> > > int unregister_reboot_notifier(struct notifier_block * nb)
> > > +{
> > > + int i;
> > > +
> > > + spin_lock(¬ifier_lock);
> > > + i = notifier_chain_unregister(&reboot_notifier_list, nb);
> > > + spin_unlock(¬ifier_lock);
> >
> > Shouldn't you move the spinlock that into the notifier_chain_* functions
> > itself ? In this case every user would be safe.
>
> Not a bad idea..
>
> > Also while you're on it
> > you could move the notifier_* functions out of line. They are currently
> > inline (I think it was some plot of Alan to force some module writers
> > to go GPL ;). Anyways, with spinlocks they are far too big to be
> > inlined now. I guess that could save a few hundred bytes.
>
> Any places lurking where a notifier func needs to be called rapidly?
Not really. They are near everywhere used for slow paths.
It does not make much difference anyways: Function calls are fast enough
on modern CPUs, and having a smaller text segment is probably even faster
(because it increases the cache hits). When the function is called multiple
times and is longer than a few instructions it is usually a loss
[at least according to the AMD K7 and modern Intel optimizations guides]
>
> Two more notes while I'm on the subject--
>
> My previous patch was missing a necessary but simple mm.h change.
> Updated patch attached (not including the stuff above.. yet).
>
> I've seen some comments saying that a VM pressure operation should occur
> in address_space or some VFS or buffer-related area. It would be nice
> to be able to notify some of the smarter device drivers of VM pressure
> as well... the interface presented here is independent of any VFS,
> address_space, or buffer-related operations, while at the same time
> being called at a familiar place (do_try_to_free_pages) with familiar
> arguments (priority, gfp_mask) and a familiar return value (pages freed,
> if any).
I personally think that doing it via address_space is the wrong way
(because it is not really a good way to express multiple pages that are
flushed together), but again I leave it for the MM gurus to hash out
the details.
-Andi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Wed Jun 07 2000 - 21:00:27 EST