Date: Mon, 21 Dec 1998 02:11:18 -0800 (PST) From: Matthew Dillon <dillon@apollo.backplane.com> To: Julian Elischer <julian@whistle.com> Cc: zhihuizhang <bf20761@binghamton.edu>, hackers <freebsd-hackers@FreeBSD.ORG> Subject: Re: questions/problems with vm_fault() in Stable Message-ID: <199812211011.CAA42292@apollo.backplane.com> References: <Pine.BSF.3.95.981220152125.8357N-100000@current1.whistle.com>
next in thread | previous in thread | raw e-mail | index | archive | help
:very interesting..
:there is a lack of people who understand the VM system very well at this
:time so it may take a while for people to get back to you.. but.. I'm sure
:I'm not the only person lokking at your suggestions...
: (you are talking about -current, right?)
:
:julian
:
:On Sat, 19 Dec 1998, zhihuizhang wrote:
:
:> Hi,
:>
:> I have some questions about the routine vm_fault() in the file vm_fault.c:
:>
:> (1) The condition (!change_wiring || wired) is always TRUE! Three possible
:> values of change_wiring are FALSE (0), VM_FAULT_CHANGE_WIRING (1), and
:> VM_FAULT_USER_WIRE (2). When its value is VM_FAULT_CHANGE_WIRING or
:> VM_FAULT_USER_WIRE, the wired count of the map entry has already been
:> incremented (see vm_map_user_pageable() and vm_map_pageable()), so
:> vm_map_lookup() will set wired as non-zero. If the argment's value is
:> FALSE, the condition is trivially true.
What line and RCS rev are you looking at here? I don't see this
anywhere in either -stable or -current in vm_fault.c
:> (2) Following the label readrest: in the source code, there are some codes
:> trying to handle read ahead for sequential objects. However, the following
:> statement is wrong:
:>
:> for (tmppindex = first_index - 1; tmppindex >=first_pindex; -- tmppindex)
:> We should probably use firstpindex instead of first_pindex in the comparison
:> (pay attention to the underscore here). However, the for loop will not loop
:> forever, because vm_page_lookup() called within the loop will return NULL
:> anyway.
This was fixed in -current. It does NOT appear to be fixed in -stable.
Hmm. The entire read-ahead codeblock is disabled due to this error,
but since I am not running a -stable system any more I can't just commit
the change because it will bring in a whole block of code that's never
been run before. Maybe one of the other guys can. It isn't in a critical
code section for -stable.. that is, the bug only makes the code a little
less efficient so I'm loath to actually make the change in -stable myself.
Maybe if DG or Luoqi or someone similar reviews it for -stable.
:> (3) If the pager fails to bring in a page for the very first object in the
:> shadow chain, the page will contain invalid content. If the pager fails to
:> bring in a page for other objects in the shadow chain, the page is freed
:> and invalid. Yet, we still reference to that page (m) later. This means
:> that after the page fault, we could get a page with invalid contents.
:> There is a XXX near the related comment in the source code.
What line? Line 483 of vm_fault.c (from -current) ?:
/*
* XXX - we cannot just fall out at this
* point, m has been freed and is invalid!
*/
This ? Could you be a bit more specific about the problem, like give
line numbers and such? The code does look somewhat odd but I don't
see where m is reused.
:> (4) The comment in the source code says that the pager can move a page, so
:> we must relookup the page by calling vm_page_lookup(). How could this be
:> the case? Why move the page?
Line? and file version too... I don't think you are looking at a recent
version of vm_fault.c
:> (5) The comment in the source code says that we do not COW read-only region
:> on a user wire. "If we do not make this restriction, the bookkeeping would
:> be nearly impossible." Can anyone explain this for me?
I think this pertains to the fact that a COW page is made read+write, and
trying to do it and then leave a page read+only confuses other parts of
the code. This isn't a bug or anything.
:> (6) The comment in the source code says map entries may be pageable. I really
:> doubt this because vm_map_entry_create() always allocates wired down memory
:> for new map entries and enter them into pmap immediately.
Most of the pmap system operates with throw-away pages and page table
directories. If a piece of code sleeps anywhere, the pagemap may not
still exist after it wakes up again. I think that is what this comment
is refering to.
:> (7) The comment in the source code says pmap_enter() may cause other faults.
:> I can not see any reason for this to happen.
pmap_enter() may have to allocate a page table page (see i386/i386/pmap.c).
If the allocation fails, the process will block there until memory is
available.
-Matt
:> I am just wondering how this important routine in VM system can have these
:> imperfect things (except (4) through (7)). I hope I am wrong. Please help
:> me out with understanding of these points.
:>
:> Any help is appreciated.
:>
:> --------------------------------------------------
:> | Zhihui Zhang, http://cs.binghamton.edu/~zzhang |
:> | Dept. of Computer Science, SUNY at Binghamton |
:> --------------------------------------------------
:>
:>
:> To Unsubscribe: send mail to majordomo@FreeBSD.org
:> with "unsubscribe freebsd-hackers" in the body of the message
:>
:
:
:To Unsubscribe: send mail to majordomo@FreeBSD.org
:with "unsubscribe freebsd-hackers" in the body of the message
:
Matthew Dillon Engineering, HiWay Technologies, Inc. & BEST Internet
Communications & God knows what else.
<dillon@backplane.com> (Please include original email in any response)
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?199812211011.CAA42292>