alc (Alan Cox)
User

Projects

User Details

User Since
Dec 14 2014, 5:52 AM (161 w, 2 d)

Recent Activity

Thu, Jan 4

alc added a comment to D13693: Restructure swapout tests after vm map locking was removed..
In D13693#287667, @kib wrote:
In D13693#287574, @alc wrote:

Please take a look at the comment at the head of this function. Every sentence after the first describes behavior different from what the function actually implements.

Do you mean that the comment should be fixed, or that we should make the code match the comment ?

Thu, Jan 4, 5:43 PM
alc added a comment to D13693: Restructure swapout tests after vm map locking was removed..
In D13693#288066, @kib wrote:
In D13693#288058, @pho wrote:
In D13693#287571, @kib wrote:

Peter, could you, please, test this diff ? Ideally, it should demonstrate exactly the same behavior as stock HEAD. In other words, it does not fix any of OOM scenarios

I ran all of the stress2 tests and a buildworld/installworld on amd64. No problems seen.

Thank you, Peter.

Alan. Mark. Should I go with the commit of the current patch ? There are two supposed comments changing which can be done later.

Thu, Jan 4, 4:35 PM
alc committed rS327538: Once we have decided to swap out a process, don't delay the laundering of.
Once we have decided to swap out a process, don't delay the laundering of
Thu, Jan 4, 3:16 AM
alc added a comment to D12635: Allow allocations across meta boundaries.

I've tested this patch under a parallel 'buildworld" workload on a machine with extremely limited memory. It appears to have the expected effect, specifically, a small but consistent reduction in swap fragmentation.

Thu, Jan 4, 12:19 AM

Wed, Jan 3

alc added inline comments to D12635: Allow allocations across meta boundaries.
Wed, Jan 3, 7:24 PM
alc added a comment to D13671: Collection of fixes for OOM handling of some corner cases..

When we unmap a thread's stack, I think that the pages should go directly to the laundry.

Index: vm/vm_swapout.c
===================================================================
--- vm/vm_swapout.c     (revision 327509)
+++ vm/vm_swapout.c     (working copy)
@@ -546,7 +546,7 @@ vm_thread_swapout(struct thread *td)
                        panic("vm_thread_swapout: kstack already missing?");
                vm_page_dirty(m);
                vm_page_lock(m);
-               vm_page_unwire(m, PQ_INACTIVE);
+               vm_page_unwire(m, PQ_LAUNDRY);
                vm_page_unlock(m);
        }
        VM_OBJECT_WUNLOCK(ksobj);

The whole point of swapping out the process is to make its pages more quickly available for other uses.

Wed, Jan 3, 7:13 PM
alc added inline comments to D13671: Collection of fixes for OOM handling of some corner cases..
Wed, Jan 3, 6:51 PM
alc added inline comments to D12635: Allow allocations across meta boundaries.
Wed, Jan 3, 6:33 PM

Tue, Jan 2

alc added a comment to D13693: Restructure swapout tests after vm map locking was removed..

Please take a look at the comment at the head of this function. Every sentence after the first describes behavior different from what the function actually implements.

Tue, Jan 2, 10:58 PM
alc added inline comments to D13693: Restructure swapout tests after vm map locking was removed..
Tue, Jan 2, 10:41 PM
alc accepted D13693: Restructure swapout tests after vm map locking was removed..
Tue, Jan 2, 10:30 PM
alc added inline comments to D13693: Restructure swapout tests after vm map locking was removed..
Tue, Jan 2, 8:56 PM
alc added a comment to D13693: Restructure swapout tests after vm map locking was removed..
In D13693#287221, @kib wrote:
In D13693#287219, @alc wrote:

Exactly. I see nothing that makes this loop body of FOREACH_PROC particularly more expensive than some of the others. In essence, I'm looking for a reason why we should be concerned about lock contention here, but not in the other cases.

I think with bigger and bigger systems and gradually bigger average workloads, were current hundreds of processes become thousands, we want to avoid holding the global lock for the whole FOREACH_PROC (esp. nesting FOREACH_THREAD_IN_PROC() three times). We should not regress the code which already does not hold the lock for whole loop.

I'm a bit skeptical that this would represent a true regression, seeing as how swapout_procs() will only be called at most once per second by default, and then only under extreme conditions where lock contention isn't a large concern. OTOH, the single-threaded performance of swapout_procs() is not very important, so I think it would be sufficient to add /* Avoid blocking writers. */ or so to indicate that the unlock is not strictly necessary.

Tue, Jan 2, 5:21 PM
alc added inline comments to D13693: Restructure swapout tests after vm map locking was removed..
Tue, Jan 2, 5:18 AM

Mon, Jan 1

alc added a comment to D13693: Restructure swapout tests after vm map locking was removed..
In D13693#287216, @kib wrote:
In D13693#287136, @kib wrote:
In D13693#287127, @alc wrote:

To be clear, I am saying that the original reason for releasing the lock doesn't apply to this code anymore. If we are going to release it, we should be able to articulate a new reason for doing so.

As I said above, not holding the lock for whole scan time makes it possible for other numerous consumers of the lock to make the progress. We are reducing the contention on allproc_lock by dropping it, either on each iteration or on should_yeild().

Releasing it on each iteration seems a bit excessive, especially since users can trigger such scans anyway by calling sysctl_kern_proc(), and vm_daemon() performs another allproc scan after calling swapout_procs(). It does seem desirable to release the lock around swapout() calls though.

Initially I also thought that it is bad to hold allproc during swapout(), but really we only unmap the kstacks and unwire the stack pages.

Mon, Jan 1, 10:28 PM
alc added a comment to D13693: Restructure swapout tests after vm map locking was removed..

I observe that we test for the P_SYSTEM flag without the benefit of the process lock in vmtotal(). Is that correct?

Mon, Jan 1, 10:12 PM
alc accepted D13725: Avoid re-check of usermode condition..
Mon, Jan 1, 8:25 PM
alc added inline comments to D13725: Avoid re-check of usermode condition..
Mon, Jan 1, 8:17 PM
alc added a comment to D13693: Restructure swapout tests after vm map locking was removed..
In D13693#287118, @kib wrote:
In D13693#287095, @alc wrote:

I've been looking at the history of changes to this code. Once upon a time, allproc_lock was held until either a process was swapped out, in which case, we did a "goto retry;" after releasing allproc_lock, or we completed the FOREACH_PROC loop. Then, a year and a half ago, we introduced the PHOLD and early allproc_lock release to address a lock ordering issue with vnodes. Elsewhere in this file, that change makes sense. Here, I don't see the need, especially now that we are no longer acquiring the map lock. Or, am I missing something?

Are you proposing to keep allproc_lock over the whole duration of FOREACH_PROC() loop ?

Mon, Jan 1, 7:38 PM
alc added inline comments to D13671: Collection of fixes for OOM handling of some corner cases..
Mon, Jan 1, 6:04 PM
alc added a comment to D13693: Restructure swapout tests after vm map locking was removed..

I've been looking at the history of changes to this code. Once upon a time, allproc_lock was held until either a process was swapped out, in which case, we did a "goto retry;" after releasing allproc_lock, or we completed the FOREACH_PROC loop. Then, a year and a half ago, we introduced the PHOLD and early allproc_lock release to address a lock ordering issue with vnodes. Elsewhere in this file, that change makes sense. Here, I don't see the need, especially now that we are no longer acquiring the map lock. Or, am I missing something?

Mon, Jan 1, 5:54 PM

Sun, Dec 31

alc committed rS327450: The variable "minslptime" is pointless and always has been, ever since its.
The variable "minslptime" is pointless and always has been, ever since its
Sun, Dec 31, 9:37 PM
alc added a comment to D13693: Restructure swapout tests after vm map locking was removed..

Ugh. I claim that the variable "minslptime" is pointless and always has been, ever since its introduction in r83366. Essentially, when the FOREACH_THREAD loop completes, we can assert than the sleep time for every thread is above whichever threshold is being applied. We don't need to maintain and check a min sleep time for the collection of threads.

Index: vm/vm_swapout.c
===================================================================
--- vm/vm_swapout.c     (revision 327361)
+++ vm/vm_swapout.c     (working copy)
@@ -729,10 +729,9 @@ swapout_procs(int action)
 {
        struct proc *p;
        struct thread *td;
-       int minslptime, slptime;
+       int slptime;
        bool didswap;
Sun, Dec 31, 7:15 PM
alc added inline comments to D12635: Allow allocations across meta boundaries.
Sun, Dec 31, 4:37 AM
alc committed rS327410: Previously, swap_pager_copy() freed swap blocks one at at time, via.
Previously, swap_pager_copy() freed swap blocks one at at time, via
Sun, Dec 31, 4:01 AM
alc closed D13290: allow frees to aggregate in swap_pager_copy.
Sun, Dec 31, 4:01 AM
alc accepted D13290: allow frees to aggregate in swap_pager_copy.
Sun, Dec 31, 3:45 AM
alc added inline comments to D13290: allow frees to aggregate in swap_pager_copy.
Sun, Dec 31, 3:31 AM
alc added inline comments to D13290: allow frees to aggregate in swap_pager_copy.
Sun, Dec 31, 12:52 AM
alc added a comment to D13290: allow frees to aggregate in swap_pager_copy.

Is there some reason why we shouldn't do

srcaddr = swp_pager_meta_ctl(srcobject, i + offset, SWM_POP);
if (srcaddr == SWAPBLK_NONE)
        continue;

before the

dstaddr = swp_pager_meta_ctl(dstobject, i, 0);

?

Sun, Dec 31, 12:51 AM
alc added inline comments to D13290: allow frees to aggregate in swap_pager_copy.
Sun, Dec 31, 12:25 AM

Sat, Dec 30

alc added inline comments to D13290: allow frees to aggregate in swap_pager_copy.
Sat, Dec 30, 7:31 PM
alc added inline comments to D13693: Restructure swapout tests after vm map locking was removed..
Sat, Dec 30, 7:05 PM
alc added inline comments to D13693: Restructure swapout tests after vm map locking was removed..
Sat, Dec 30, 6:57 PM

Fri, Dec 29

alc added a comment to D13681: Do not lock vm map in swapout_procs()..

Hmm. Do we still need to unlock and relock the process?

_PHOLD_LITE(p);
PROC_UNLOCK(p);
sx_sunlock(&allproc_lock);
Fri, Dec 29, 10:44 PM
alc added a comment to D13398: Complete zig-zag splay, rather than zigging and looping again.

Here are results from a different Java application.

x javamark_1.5GB_before
+ javamark_1.5GB_after
+--------------------------------------------------------------------------------------------------+
|+                          + +                    +            x                    +xx   x x  x +|
|           |____________________________________A_M_______________________|__________|____M______||
+--------------------------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   6       7180627       7524006       7463407     7416031.3     121934.89
+   6       6513706       7541927       7042828     7020509.2     391280.84
Difference at 95.0% confidence
        -395522 +/- 372781
        -5.33334% +/- 5.02669%
        (Student's t, pooled s = 289801)
Fri, Dec 29, 9:55 PM
alc added a comment to D13398: Complete zig-zag splay, rather than zigging and looping again.

A problem with using any SPECjvm2008 test for evaluating this patch is that by default the SPECjvm2008 tests run for a fixed amount of time. Essentially, speeding up vm_map_entry_splay() just means that there will be more calls to vm_map_entry_splay(), and so it's not entirely surprisingly that there is no statistical difference in the number of cycles spent in vm_map_entry_splay().

Fri, Dec 29, 9:48 PM
alc accepted D13681: Do not lock vm map in swapout_procs()..
Fri, Dec 29, 7:40 PM
alc accepted D13678: Style fixes for vm_swapout.c extracted from D13671..
Index: vm/vm_swapout.c
===================================================================
--- vm/vm_swapout.c     (revision 327347)
+++ vm/vm_swapout.c     (working copy)
@@ -938,9 +938,10 @@ swapout(struct proc *p)
            P_INMEM, ("swapout: lost a swapout race?"));
Fri, Dec 29, 5:52 PM
alc committed rS327349: After r327168, the variable "vm_pageout_wanted" can be static..
After r327168, the variable "vm_pageout_wanted" can be static.
Fri, Dec 29, 5:02 PM
alc added inline comments to D13671: Collection of fixes for OOM handling of some corner cases..
Fri, Dec 29, 3:18 AM
alc added a comment to D13671: Collection of fixes for OOM handling of some corner cases..
In D13671#285730, @kib wrote:
In D13671#285719, @alc wrote:

Does swapout_procs() actually need an exclusive lock map either?

I think that swapout_procs() in fact does not need the map lock at all. It does not currently access the map entries nor it makes decisions based on the residency count or swap usage.

In D13671#285720, @alc wrote:

Shouldn't swapout() do a pmap_advise(DONTNEED) on every map entry? Otherwise, won't lingering reference bits from the pmap of the swapped out process potentially lead the reactivation of pages that were only accessed by the swapped out process?

I am not sure. swapout() occurs when the process was idle for long enough time. Mist likely, pagedaemon already processed the pages and there is no lingering reference bits in the pte. If pagedaemon did not scanned that pages yet, we are definitely not low on memory and reactivation maintains fairness.

If we start doing pmap_advise(), the map lock will be needed.

Fri, Dec 29, 3:07 AM

Thu, Dec 28

alc added inline comments to D13671: Collection of fixes for OOM handling of some corner cases..
Thu, Dec 28, 11:22 PM
alc added a comment to D13671: Collection of fixes for OOM handling of some corner cases..

Shouldn't swapout() do a pmap_advise(DONTNEED) on every map entry? Otherwise, won't lingering reference bits from the pmap of the swapped out process potentially lead the reactivation of pages that were only accessed by the swapped out process?

Thu, Dec 28, 11:11 PM
alc added a comment to D13671: Collection of fixes for OOM handling of some corner cases..

Does swapout_procs() actually need an exclusive lock map either?

Thu, Dec 28, 11:03 PM
alc added inline comments to D13671: Collection of fixes for OOM handling of some corner cases..
Thu, Dec 28, 10:17 PM
alc added a comment to D13644: Eliminate "pass" from vm_pageout_scan() and vm_pageout_worker().

I left this patch running overnight on a machine that was configured to perform a moderate amount of paging.

Thu, Dec 28, 6:31 PM
alc updated the diff for D13644: Eliminate "pass" from vm_pageout_scan() and vm_pageout_worker().

I have updated this patch according to yesterday's discussion. However, I have not yet updated any of the comments to reflect any of the changes in the code.

Thu, Dec 28, 5:51 PM
alc added a comment to D13398: Complete zig-zag splay, rather than zigging and looping again.

I see an 18.15% reduction in cycles spent in vm_map_entry_splay() for buildworld.

x before_world
+ after_world
+------------------------------------------------------------------------------+
|+                                                                            x|
|+                                                                            x|
|+                                                                            x|
|++                                                                          xx|
|A|                                                                          |A|
+------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   5 4.8476252e+10 4.8601982e+10 4.8593236e+10 4.8570385e+10      53214127
+   5 3.9698544e+10  3.983885e+10 3.9745925e+10 3.9755596e+10      51061053
Difference at 95.0% confidence
        -8.81479e+09 +/- 7.60559e+07
        -18.1485% +/- 0.156589%
        (Student's t, pooled s = 5.21487e+07)
Thu, Dec 28, 7:00 AM
alc added a comment to D13644: Eliminate "pass" from vm_pageout_scan() and vm_pageout_worker().
In D13644#285437, @alc wrote:

The following version would address the issue that I raised yesterday without eliminating "pass". That said, I am no longer incrementing it. Its value is either 0 or 1.

Hm, if we lift the computation of the target into vm_pageout_worker(), I think we could address this problem without keeping "pass"?

Thu, Dec 28, 3:08 AM

Wed, Dec 27

alc added a comment to D13644: Eliminate "pass" from vm_pageout_scan() and vm_pageout_worker().

The following version would address the issue that I raised yesterday without eliminating "pass". That said, I am no longer incrementing it. It's value is either 0 or 1.

while (TRUE) {
        mtx_lock(&vm_page_queue_free_mtx);
        if (vm_pages_needed && !vm_page_count_min()) {
                vm_pages_needed = false;
                wakeup(&vm_cnt.v_free_count);
        }
        if (!target_met) {
                mtx_unlock(&vm_page_queue_free_mtx);
                MPASS(pass == 1);
                pause("pwait", hz / VM_INACT_SCAN_RATE);
        } else if (vm_pages_needed) {
                mtx_unlock(&vm_page_queue_free_mtx);
                pass = 1;
        } else {
                vm_pageout_wanted = false;
                if (mtx_sleep(&vm_pageout_wanted,
                    &vm_page_queue_free_mtx, PDROP | PVM, "psleep",
                    hz) == 0) {
                        VM_CNT_INC(v_pdwakeups);
                        pass = 1;
                } else
                        pass = 0;
        }
        target_met = vm_pageout_scan(domain, pass);
}
Wed, Dec 27, 9:30 PM
alc added a comment to D13398: Complete zig-zag splay, rather than zigging and looping again.

That said, the increase in the variance is striking.

Wed, Dec 27, 8:52 PM
alc added a comment to D13398: Complete zig-zag splay, rather than zigging and looping again.

I've tested the patch with compiler.compiler from SPECjvm2008. I'm afraid that there is no discernible difference in the number of cycles spent in vm_map_entry_splay().

x before
+ after
+-------------------------------------------------------------------------------+
|                                                +                              |
|+                 + x   + x    +x      xx   x   +x x   x   +            +     +|
|                |___________|__________AM_A_____M__|________________|          |
+-------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x   9      20905598      23828774      22610639      22557854      994238.6
+   9      19257738      25797772      23263976      22777334     2164232.2
No difference proven at 95.0% confidence
Wed, Dec 27, 8:50 PM

Tue, Dec 26

alc added a comment to D13644: Eliminate "pass" from vm_pageout_scan() and vm_pageout_worker().

Regardless of whether we eliminate "pass", I believe that there is another issue, albeit a minor one, that ought to be addressed. When we perform a "pass == 0" scan, I believe that we always return true from vm_pageout_scan(). (Please check me on this claim!) Suppose that in the midst of that scan pagedaemon_wakeup() was called. Here is the issue. Consider the following snippet.

/*                                                               
 * Do not clear vm_pageout_wanted until we reach our free page   
 * target.  Otherwise, we may be awakened over and over again,   
 * wasting CPU time.                                             
 */
if (vm_pageout_wanted && target_met)
        vm_pageout_wanted = false;

This is going to reset vm_pageout_wanted to false, mtx_sleep() is going to be called, and the page daemon won't perform a "pass == 1" scan until another page allocation occurs and wakes it up. If we are extremely unlucky and another pagedaemon_wakeup() call occurs after vm_pageout_wanted is reset but before mtx_sleep(), then the page daemon might sleep until pagedaemon_wait() is called.

Tue, Dec 26, 9:52 PM
alc updated the diff for D13644: Eliminate "pass" from vm_pageout_scan() and vm_pageout_worker().

Don't free any pages unless the number of free pages is below the wakeup threshold for the page daemon.

Tue, Dec 26, 9:31 PM
alc added a comment to D13644: Eliminate "pass" from vm_pageout_scan() and vm_pageout_worker().
In D13644#285200, @kib wrote:

With the elimination of "pass", I want to point out that the page daemon will attempt to bring the number of free pages up to the target no matter how it was awakened. I do not think that that is a bad idea.

The difference vm_cnt.v_free_target - vm_pageout_wakeup_thresh scales linearly with the total amount of memory and becomes large on systems with lots of memory. With 16GB of RAM it's 240MB. With this change, we effectively lose the ability to cache that much more memory since the page daemon will be freeing pages more aggressively, so this would impact workloads with a working set in that region. It's arguably not a major problem, but I don't really see a corresponding benefit?

Perhaps this is more generic question. On a machine with 128Gb of RAM, I have free_target 2.7G. Is it reasonable at all ?

Tue, Dec 26, 9:08 PM
alc added a comment to D13644: Eliminate "pass" from vm_pageout_scan() and vm_pageout_worker().

With the elimination of "pass", I want to point out that the page daemon will attempt to bring the number of free pages up to the target no matter how it was awakened. I do not think that that is a bad idea.

The difference vm_cnt.v_free_target - vm_pageout_wakeup_thresh scales linearly with the total amount of memory and becomes large on systems with lots of memory. With 16GB of RAM it's 240MB. With this change, we effectively lose the ability to cache that much more memory since the page daemon will be freeing pages more aggressively, so this would impact workloads with a working set in that region. It's arguably not a major problem, but I don't really see a corresponding benefit?

Tue, Dec 26, 9:02 PM
alc committed rS327224: MFC r326982.
MFC r326982
Tue, Dec 26, 8:34 PM
alc added a comment to D13155: Do not loop infinitely in vm_map_find()..

Can we abandon this now?

Tue, Dec 26, 8:09 PM
alc added a comment to D12888: Remove object_collapses and object_bypasses counters.

Can we abandon this change now?

Tue, Dec 26, 8:07 PM
alc created D13644: Eliminate "pass" from vm_pageout_scan() and vm_pageout_worker().
Tue, Dec 26, 7:10 PM
alc committed rS327218: Refactor vm_map_find(), creating a separate function, vm_map_alignspace(),.
Refactor vm_map_find(), creating a separate function, vm_map_alignspace(),
Tue, Dec 26, 5:59 PM
alc closed D13346: Handle address wrap errors in vm_map_find().
Tue, Dec 26, 5:59 PM
alc accepted D13640: Ensure that pass > 0 when starting a scan with vm_pages_needed = 1..
Tue, Dec 26, 4:24 PM

Mon, Dec 25

alc added a comment to D13346: Handle address wrap errors in vm_map_find().

Are there any other comments or questions about this change?

Mon, Dec 25, 8:59 PM
alc committed rS327179: Make the vm object bypass and collapse counters per CPU..
Make the vm object bypass and collapse counters per CPU.
Mon, Dec 25, 7:36 PM
alc closed D13611: Optimize the vm object bypass and collapse counters.
Mon, Dec 25, 7:36 PM
alc added a comment to D13611: Optimize the vm object bypass and collapse counters.

As an aside, it seems to me that sys/counter.h and/or the man page ought to provide guidance on when counter_u64_alloc() becomes possible. It appears to me that it is earlier than SI_SUB_CPU, specifically, we need the following to have completed:

SYSINIT(pcpu_zones, SI_SUB_KMEM, SI_ORDER_ANY, pcpu_zones_startup, NULL);
Mon, Dec 25, 7:25 PM
alc added inline comments to D13611: Optimize the vm object bypass and collapse counters.
Mon, Dec 25, 6:39 PM

Sun, Dec 24

alc added inline comments to D13611: Optimize the vm object bypass and collapse counters.
Sun, Dec 24, 9:47 PM
alc added a reviewer for D13611: Optimize the vm object bypass and collapse counters: mjg.
Sun, Dec 24, 9:43 PM
alc added a comment to D12888: Remove object_collapses and object_bypasses counters.

See D13611.

Sun, Dec 24, 9:41 PM
alc created D13611: Optimize the vm object bypass and collapse counters.
Sun, Dec 24, 9:40 PM
alc accepted D13424: Fix two problems with the page daemon control loop..
Sun, Dec 24, 6:20 PM
alc accepted D13424: Fix two problems with the page daemon control loop..
Sun, Dec 24, 5:28 PM

Sat, Dec 23

alc added inline comments to D13289: Use separate vmems to segregate KVA by domain.
Sat, Dec 23, 8:05 PM
alc added a comment to D13424: Fix two problems with the page daemon control loop..

As an aside, I really don't see the point of the "pass" parameter to vm_pageout_scan() anymore. Once upon a time, it controlled laundering, but now it only serves to tell us that we entered vm_pageout_scan() on the 1 second timeout. We could just as well calculate the page shortage unconditionally and let that determine whether to invoke the lowmem handlers, uma_reclaim(), and vm_swapout_run_idle().

Sat, Dec 23, 7:44 PM
alc added inline comments to D13424: Fix two problems with the page daemon control loop..
Sat, Dec 23, 6:54 PM

Tue, Dec 19

alc added a comment to D13522: Various improvements to the atomics man page.

When did we introduces fences? A statement should probably be added to the HISTORY section.

Tue, Dec 19, 5:23 PM
alc committed rS326982: Document the semantics of atomic_thread_fence operations..
Document the semantics of atomic_thread_fence operations.
Tue, Dec 19, 5:08 PM
alc closed D13522: Various improvements to the atomics man page.
Tue, Dec 19, 5:08 PM

Mon, Dec 18

alc added inline comments to D13534: Introduce atomic_load() and atomic_store()..
Mon, Dec 18, 7:39 PM
alc accepted D13534: Introduce atomic_load() and atomic_store()..

I'm happy with the implementation. (I assume that the use cases will be committed separately.)

Mon, Dec 18, 6:29 PM
alc added inline comments to D13522: Various improvements to the atomics man page.
Mon, Dec 18, 6:02 PM
alc added inline comments to D13534: Introduce atomic_load() and atomic_store()..
Mon, Dec 18, 5:47 PM
alc added inline comments to D13522: Various improvements to the atomics man page.
Mon, Dec 18, 4:40 PM

Dec 17 2017

alc added inline comments to D13424: Fix two problems with the page daemon control loop..
Dec 17 2017, 8:26 PM
alc created D13522: Various improvements to the atomics man page.
Dec 17 2017, 6:46 PM

Dec 11 2017

alc accepted D13422: Use a dedicated counter for laundry thread wakeups..
Dec 11 2017, 3:33 AM

Dec 10 2017

alc added inline comments to D13422: Use a dedicated counter for laundry thread wakeups..
Dec 10 2017, 9:18 PM

Dec 9 2017

alc added inline comments to D13422: Use a dedicated counter for laundry thread wakeups..
Dec 9 2017, 5:01 PM
alc added a comment to D13422: Use a dedicated counter for laundry thread wakeups..
In D13422#280514, @alc wrote:

I'm looking at D13424, and it doesn't appear to me that the meaning v_pdwakeups has changed. That is, it's still the case that we increment v_pdwakeups only if we are explicitly awakened. Am I missing something?

That's correct, but the intent behind reading pdwakeups was to count the number of inactive queue scans. We might perform multiple scans after a single wakeup.

Dec 9 2017, 4:50 PM

Dec 8 2017

alc added inline comments to D13424: Fix two problems with the page daemon control loop..
Dec 8 2017, 11:29 PM
alc added a comment to D13422: Use a dedicated counter for laundry thread wakeups..

I'm looking at D13424, and it doesn't appear to me that the meaning v_pdwakeups has changed. That is, it's still the case that we increment v_pdwakeups only if we are explicitly awakened. Am I missing something?

Dec 8 2017, 6:06 PM
alc accepted D13423: Fix the act_scan_laundry_weight mechanism..
Dec 8 2017, 5:36 PM

Dec 7 2017

alc added inline comments to D13398: Complete zig-zag splay, rather than zigging and looping again.
Dec 7 2017, 5:25 PM
alc added a comment to D13346: Handle address wrap errors in vm_map_find().
In D13346#280055, @kib wrote:
In D13346#280028, @alc wrote:

Nevermind. I forgot that vm_map_findspace() handles this case. It updates the given address to be at least vm_map_min(). In general, other callers don't pass 0, but themselves initialize the given address to vm_map_min().

So do you prefer to change gem code to do the same ?

Dec 7 2017, 4:14 PM
alc added a comment to D13346: Handle address wrap errors in vm_map_find().
In D13346#280027, @alc wrote:

Kostik, I've been reviewing vm_map_find()'s callers, and I'm concerned about i915_gem_mmap_ioctl(). Consider this snippet:

        p = curproc;
        map = &p->p_vmspace->vm_map;
...
        addr = 0;
        vm_object_reference(obj->vm_obj);
        rv = vm_map_find(map, obj->vm_obj, args->offset, &addr, args->size, 0,

I believe that 0 is going to be out-of-range for a map on amd64, and so vm_map_find(), either with or without this revision, is going to return an error. Am I missing something here?

Dec 7 2017, 6:08 AM
alc added a comment to D13346: Handle address wrap errors in vm_map_find().

Kostik, I've been reviewing vm_map_find()'s callers, and I'm concerned about i915_gem_mmap_ioctl(). Consider this snippet:

        p = curproc;
        map = &p->p_vmspace->vm_map;
...
        addr = 0;
        vm_object_reference(obj->vm_obj);
        rv = vm_map_find(map, obj->vm_obj, args->offset, &addr, args->size, 0,

I believe that 0 is going to be out-of-range for a map on amd64, and so vm_map_find(), either with or without this revision, is going to return an error. Am I missing something here?

Dec 7 2017, 5:39 AM

Dec 6 2017

alc added a comment to D13346: Handle address wrap errors in vm_map_find().

Doug's suggested change reduces the number of iterations in vm_map_entry_splay() during a "buildworld" workload by 41.5%. To be clear, we're still touching the same map entries; we're just avoiding redundant comparisons.

Dec 6 2017, 6:16 PM