Date: Mon, 15 Feb 1999 23:10:25 -0500 (EST) From: "John S. Dyson" <dyson@iquest.net> To: julian@whistle.com (Julian Elischer) Cc: dyson@iquest.net, dillon@apollo.backplane.com, freebsd-hackers@FreeBSD.ORG Subject: Re: inode / exec_map interlock ? (follow up) Message-ID: <199902160410.XAA00350@y.dyson.net> In-Reply-To: <Pine.BSF.3.95.990215183646.12933M-100000@current1.whistle.com> from Julian Elischer at "Feb 15, 99 06:52:19 pm"
next in thread | previous in thread | raw e-mail | index | archive | help
Julian Elischer said:
> unreadable, and sorely lacking in comments and documentation. I for one am
> grateful for Matt for doing ht ework of cleaning and commenting. I can
> follow matt's commented code a lot easier than I could before, and that
> means that I can now contructively comment and help.
>
I don't agree that the code is easier to read. Perhaps I know the
structure of the code, and understand how the MACH VM system works.
However, in order to understand code, I don't have to rewrite sections
of it, changing things while recoding.
If one wanted to write a document describing the code, they should
do the Lyons's book thing. This isn't what is happening. A few added
comments wouldn't be bad either.
>
> I agree that he might
> do well to involve more people in this, but consider John that you DID
> resign involvement in this code, and people, while sad to see you go, did
> take you at your word.
>
Yep, but I don't want to see FreeBSD destroyed with hackery.
>
> If you are coming back to this then you will be
> welcomed with oepn arms, but you need to realise that some people find it
> hard sitting on their hands for 2 days waiting for an answer, and that if
> you are not officially involved then they will be doing so as a courtesy.
>
There is missing context, and it isn't just me.
>
> I for one want you involved in discussing these changes because your
> experience is invaluable.. however Matt is in the stage in his life at the
> moment wher he has the energy of 10 raging bulls (hmm I think I'm going
> overboard here), so you need to make sure that that is harnessed and used
> constructively.
>
Bulls in a china shop more likely.
>
> It would be better if you were to take on the role of design adviser to
> Matt, let him take responsibility for his own decisions and give him
> ideas of how you would want to see things done..
>
If he'd listen, it wouldn't be a problem. There is too much unidirectional
communication going on, and if I didn't care about FreeBSD, I'd kind of
chuckle and walk away. More than I have tried to make sure that there
isn't damage...
>
> My opinion is that if you outline to him all the improvements you've been
> envisioning, and let him do the coding, we will all come out ahead..
> you will have time to do what you need to do, Matt will get to learn your
> experience, and FreeBSD will get well commented and dovumented code, that
> is readble, and designed by someone that knows what they are doing (you).
>
Matt doesn't listen. That is the problem. I have tried put myself into
a position of being listened to, and it ends up that I have to become
intense. I don't like being intense, it is quite painful to me.
>
> You are coming across as confronatational (to me anyhow). I know you so I
> know that you are not being so, but you and matt need to work out a formal
> arangement. I know Matt is also a very reasonable guy, and since he
> presently has some time on his hands, I think you should make use of it to
> all our advantages..
>
I have found Matt to be not listening. If arguments come up, it should
be that *he* has to prove the significant changes that he intends to make.
If there was more cooperation between those who have been involved in
the VM code and him, then the need for more formal proof could be
softened. The lack of cooperation is not due the original authors. At
the end of this email are commit messages from vm_page.c... Are there
many review by lines? If there aren't, then it is either because the
developers who designed and understand the purpose of the design of
the code aren't being: 1) credited, 2) consulted or 3) they aren't
agreeing to the changes which are happening anyway. Frankly, I don't
agree with the concepts of some of the changes, and assertions are
being made without authority. This would not be so important, but
Matt is a new hacker on the code. Just because one set of changes
were okayed, not all of them are.
It seems that 2 others and myself (probably the only three people
who understand the code), have had problems in dealing with this thing.
In not listening to (heeding) mine and others suggestions, it seems that
there is a problem of ego, and proving oneself. That doesn't help
communications. This set of public responses is a result of 3-4 wks
of frustration. I am trying to open communications after a long amount
of frustration, and peer pressure will be the last viable approach for
me.
By adopting too many quick fixes, I can truthfully say that FreeBSD
will be cutting itself out of some other, more interesting things...
Too much energy is being spent on an expensive "indent" process.
--
John | Never try to teach a pig to sing,
dyson@iquest.net | it makes one look stupid
jdyson@nc.com | and it irritates the pig.
----------------------------
revision 1.126
date: 1999/02/15 06:52:14; author: dillon; state: Exp; lines: +82 -112
Minor reorganization of vm_page_alloc(). No functional changes have
been made but the code has been reorganized and documented to make
it more readable, reduce the size of the code, and optimize the branch
path caching capabilities that most modern processors have.
----------------------------
revision 1.125
date: 1999/02/08 00:37:35; author: dillon; state: Exp; lines: +34 -91
Rip out PQ_ZERO queue. PQ_ZERO functionality is now combined in with
PQ_FREE. There is little operational difference other then the kernel
being a few kilobytes smaller and the code being more readable.
* vm_page_select_free() has been *greatly* simplified.
* The PQ_ZERO page queue and supporting structures have been removed
* vm_page_zero_idle() revamped (see below)
PG_ZERO setting and clearing has been migrated from vm_page_alloc()
to vm_page_free[_zero]() and will eventually be guarenteed to remain
tracked throughout a page's life ( if it isn't already ).
When a page is freed, PG_ZERO pages are appended to the appropriate
tailq in the PQ_FREE queue while non-PG_ZERO pages are prepended.
When locating a new free page, PG_ZERO selection operates from within
vm_page_list_find() ( get page from end of queue instead of beginning
of queue ) and then only occurs in the nominal critical path case. If
the nominal case misses, both normal and zero-page allocation devolves
into the same _vm_page_list_find() select code without any specific
zero-page optimizations.
Additionally, vm_page_zero_idle() has been revamped. Hysteresis has been
added and zero-page tracking adjusted to conform with the other changes.
Currently hysteresis is set at 1/3 (lo) and 1/2 (hi) the number of free
pages. We may wish to increase both parameters as time permits. The
hysteresis is designed to avoid silly zeroing in borderline allocation/free
situations.
----------------------------
revision 1.124
date: 1999/02/07 20:45:15; author: dillon; state: Exp; lines: +75 -188
Remove L1 cache coloring optimization ( leave L2 cache coloring opt ).
Rewrite vm_page_list_find() and vm_page_select_free() - make inline out
of nominal case.
----------------------------
revision 1.123
date: 1999/01/28 00:57:57; author: dillon; state: Exp; lines: +16 -16
Fix warnings in preparation for adding -Wall -Wcast-qual to the
kernel compile
----------------------------
revision 1.122
date: 1999/01/24 07:06:52; author: dillon; state: Exp; lines: +1 -2
Undo last commit - not a bug, just duplicate code. PG_MAPPED and
PG_WRITEABLE are already cleared by vm_page_protect().
----------------------------
revision 1.121
date: 1999/01/24 06:00:31; author: dillon; state: Exp; lines: +13 -4
vm_map_split() used to dirty the page manually after calling
vm_page_rename(), but never pulled the page off PQ_CACHE if it was on
PQ_CACHE. Dirty pages in PQ_CACHE are not allowed and a KASSERT was
added in -4.x to test for this... and got hit.
In -4.x, vm_page_rename() automatically dirties the page. This commit
also has it deal with the PQ_CACHE case, deactivating the page in that
case.
----------------------------
revision 1.120
date: 1999/01/24 02:29:26; author: dillon; state: Exp; lines: +3 -3
Clear PG_MAPPED as well as PG_WRITEABLE when a page is moved to the
cache.
----------------------------
revision 1.119
date: 1999/01/24 01:04:04; author: dillon; state: Exp; lines: +7 -2
Clear PG_WRITEABLE in vm_page_cache(). This may or may not be a bug,
but the bit should definitely be cleared.
----------------------------
revision 1.118
date: 1999/01/21 10:01:49; author: dillon; state: Exp; lines: +1 -1
The hash table used to be a table of doubly-link list headers ( two
pointers per entry ). The table has been changed to a singly linked
list of vm_page_t pointers. The table has been doubled in size, but
the entries only take half the space so a net-zero change in memory use.
The hash function has been changed, hopefully for the better. The
combination of the larger hash table size of changed function should
keep the chain length down to a reasonable number (0-3, average 1).
vm_object->page_hint has been removed. This 'optimization' was not
only never needed, but costs as much as a hash chain link to implement.
While having page_hint in vm_object might result in better locality
of reference, the cost is not worth the space in vm_object or the
extra instructions in my view.
vm_page_alloc*() functions have been inlined and call a generalized
non-inlined vm_page_alloc_toq() which combines the standard alloc
and zero-page alloc functions together, reducing code size and the L1
cache footprint. Some reordering has been done... not much. The
delinking code should be faster ( because unlinking a doubly-linked list
requires four memory ops and unlinking a singly linked list only requires
two ), and we get a hash consistancy check for free.
vm_page_rename() now automatically sets the page's dirty bits.
vm_page_alloc() does not try to manually inline freeing a cache page.
Instead, it now properly calls vm_page_free(m) ... vm_page_free() is
really too complex to manually inline.
vm_await(), supporting asleep(), has been added.
----------------------------
revision 1.117
date: 1999/01/21 08:29:12; author: dillon; state: Exp; lines: +288 -148
This is a rather large commit that encompasses the new swapper,
changes to the VM system to support the new swapper, VM bug
fixes, several VM optimizations, and some additional revamping of the
VM code. The specific bug fixes will be documented with additional
forced commits. This commit is somewhat rough in regards to code
cleanup issues.
Reviewed by: "John S. Dyson" <root@dyson.iquest.net>, "David Greenman" <dg@root.com>
----------------------------
revision 1.116
date: 1999/01/10 01:58:29; author: eivind; state: Exp; lines: +5 -5
KNFize, by bde.
----------------------------
revision 1.115
date: 1999/01/08 17:31:27; author: eivind; state: Exp; lines: +7 -22
Split DIAGNOSTIC -> DIAGNOSTIC, INVARIANTS, and INVARIANT_SUPPORT as
discussed on -hackers.
Introduce 'KASSERT(assertion, ("panic message", args))' for simple
check + panic.
Reviewed by: msmith
----------------------------
revision 1.114
date: 1998/12/23 01:52:47; author: dillon; state: Exp; lines: +106 -19
Update comments to routines in vm_page.c, most especially whether a
routine can block or not as part of a general effort to carefully
document blocking/non-blocking calls in the kernel.
----------------------------
revision 1.113
date: 1998/11/11 15:07:57; author: dg; state: Exp; lines: +4 -4
Closed a small race condition between wiring/unwiring pages that involved
the page's wire_count.
----------------------------
revision 1.112
date: 1998/10/28 13:41:43; author: dg; state: Exp; lines: +3 -13
Fixed wrong comments in and about vm_page_deactivate().
----------------------------
revision 1.111
date: 1998/10/28 13:37:02; author: dg; state: Exp; lines: +14 -6
Added a second argument, "activate" to the vm_page_unwire() call so that
the caller can select either inactive or active queue to put the page on.
----------------------------
revision 1.110
date: 1998/10/25 17:44:59; author: phk; state: Exp; lines: +1 -5
Nitpicking and dusting performed on a train. Removes trivial warnings
about unused variables, labels and other lint.
----------------------------
revision 1.109
date: 1998/10/21 14:46:41; author: dg; state: Exp; lines: +4 -8
Nuked PG_TABLED flag. Replaced with m->object != NULL.
----------------------------
revision 1.108
date: 1998/10/21 11:43:04; author: dg; state: Exp; lines: +2 -1
Add a diagnostic printf for freeing a wired page. This will eventually
be turned into a panic, but I want to make sure that all cases of freeing
pages with wire_count==1 (which is/was allowed) have first been fixed.
----------------------------
revision 1.107
date: 1998/09/04 08:06:57; author: dfr; state: Exp; lines: +11 -11
Cosmetic changes to the PAGE_XXX macros to make them consistent with
the other objects in vm.
...
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199902160410.XAA00350>