899493 - Manually swap out anonymous memory of b2g

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: General, defect, P3)

Description

[Ting-Yuan Huang]

Attachment: BionicGlue.cpp.diff

In the thread
https://groups.google.com/forum/#!topic/mozilla.dev.b2g/QNLuHylCOlU
we observed that a non-trivial portion of anonymous memory used by the b2g process is non-varying.

Attached is a WIP draft that tracks the calls to the mmap() family. It waits for SIGUSR2 to writes anonymous memory to files and maps them back so that the non-varying parts can be evicted and those rarely used data are swapped to flash memory.

In the test[1], it saved about 17MB out of 46MB anonymous memory on average without manually calling GC , or 10MB out of 38MB right after GC. The size of the varying parts (measured by summing Private_Dirty pages) is quite stable and is around 22MB.

In some extreme cases, such as running SunSpider 1.0, the Rss(committed physical memory) of browser went to ~130MB and ~100MB before killed with and without this patch, respectively. Namely, it gets extra 30MB memory at the expense of performance.

The downside of this patch is that it cost ~5 sec to write out all anonymous memory that is tracked. This would incur a non-acceptable freeze of the device, including UI. A possible way to ease the lag is write out the memory incrementally; It will be implemented in the next WIP patch.

[1] open twitter, facebook and look around.

Comment 1

[Ting-Yuan Huang]

Attachment: about-memorys w/w.o gc and w/w.o this patch.

Comment 2

[Ting-Yuan Huang]

Attachment: BionicGlue.cpp.diff

Comment 3

[Andreas Gal :gal]

Can we write this out async and then map it once it was written? And will this destroy the flash we are using?

Comment 4

[Ting-Yuan Huang]

(In reply to Andreas Gal :gal from comment #3)
> Can we write this out async and then map it once it was written?

Sure, I'll try this.

> And will this destroy the flash we are using?

Typically all committed anonymous pages, except for the fast-varying ones, will be written. It's roughly 30MB ~ 50MB. That will be written once after booting to a stable state. If this is not acceptable, we may consider writing them to an in-memory compressed filesystem. The compression ratio is roughly 25% - 50%, depending on the compression methods, so the memory saved will be discounted by 25% - 50%. We can also write to a compressed filesystem on flash so that the data written to it is 7.5MB ~ 25MB for each boot.

By the way, I'll also try to identify the creation sites of those stable data. If we are lucky enough, they'll be eliminated much more efficiently from the higher level.

Comment 5

[Justin Lebar (not reading bugmail)]

> Can we write this out async and then map it once it was written?

Only if you can guarantee that the memory we're writing out doesn't change in the meantime...

I'm not convinced this is something we want to do.  In the best case, suppose we write an entire app process out to disk.  This is great, because now we can have one more app process in memory.

But then when the user goes to run this app (or when the app wakes it self up), we have to page in much of the process, and we have to page out an equal amount of memory.  It sounds like paging out a chunk memory will take at least a few seconds.  I'd expect paging in memory will take roughly that long.

Given the choice between re-launching the app, which takes 2-3s, and paging the app back in, which takes at least 2-3s, using a pagefile seems not very appealing to me.

Comment 6

[Ting-Yuan Huang]

(In reply to Justin Lebar [:jlebar] from comment #5)
> > Can we write this out async and then map it once it was written?
> 
> Only if you can guarantee that the memory we're writing out doesn't change
> in the meantime...

Yes. So I stopped all threads when writing. Now I'm planning to mark those pages read-only when writing in a separate thread. If SIGSEGV was caught, the scheduled, corresponding write would be cancelled.

> I'm not convinced this is something we want to do.  In the best case,
> suppose we write an entire app process out to disk.  This is great, because
> now we can have one more app process in memory.

This patch tries to exploit the stable and one-shot data in the main process (b2g). The numbers reported are b2g only. Content processes seem to change more quickly and are harder to deal with.

Comment 7

[Mike Hommey [:glandium]]

Probably a stupid question, but why can't you just enable swap?

Comment 8

[Ting-Yuan Huang]

(In reply to Mike Hommey [:glandium] from comment #7)
> Probably a stupid question, but why can't you just enable swap?

I think it's not possible to enable swap only for certain processes without modifying kernel (not 100% sure). It also seems to be hard to swap out only once after boot. Maybe we should try some approaches in kernel, although that would be Firefox OS only.

To emulate the behavior of swap but remaining portable, I tried to intercept all anonymous maps, redirect them into file based maps and tuned write back parameters in /proc/sys/vm/dirty_* so that dirty pages are written back as lazy as possible. Unfortunately I still observed a large amount of IO and delay and I'm not sure if this would wear the flash too quickly.

Nevertheless, I'll try to enable swap and measure the IO later. Let's see if it is feasible.

Comment 9

[Mike Hommey [:glandium]]

Why limit to some processes? The kernel would swap out unused pages, whatever process they are in. There's possibly some win to get from that happening for (useless) system processes, and background b2g processes.
And swap doesn't have to be real swap. It can be compressed page cache. See http://code.google.com/p/compcache/ (which is now in mainline kernel)

Comment 10

[Mike Hommey [:glandium]]

There are actually more in-kernel compression tricks than compcache/zram: http://lwn.net/Articles/545244/

Comment 11

[Hugh Nougher [:Hughman]]

Wouldn't this also have negative effects on the work from bug 771765?

Both have an aim of reducing the unused/rarely-used memory in the main process but bug 771765 also assists in reducing content processes memory by reusing the main processes pages, which with this will be paged out. Unless I'm misunderstanding something?

Comment 12

[Justin Lebar (not reading bugmail)]

Bug 771765 affects only content processes; it has no effect on the main process's memory usage.

This bug as currently being investigated affects only the main process.  But I don't think that paging out content-process memory mixes badly with Nuwa, so long as whatever paging mechanism we use (the kernel, or our own manual one) is aware of shared pages.

Comment 13

[Ting-Yuan Huang]

(In reply to Mike Hommey [:glandium] from comment #9)
> Why limit to some processes? The kernel would swap out unused pages,
> whatever process they are in. There's possibly some win to get from that
> happening for (useless) system processes, and background b2g processes.

Because the flash memory is fragile and slow, we want to exploit the fact that some data in the b2g process are stable or unused to avoid large amount of IO that come with swap.

> And swap doesn't have to be real swap. It can be compressed page cache. See
> http://code.google.com/p/compcache/ (which is now in mainline kernel)

Cool! My original plan is to construct a solution that can also be applied on Fennec. I'll try this to see if it is significantly better then the userland solution.

Comment 14

[Ting-Yuan Huang]

Attachment: anon_cache.diff

Allow threads to run concurrently and break large chunks into small ones so that IO won't block some critical regions too long.

The longest pause observed so far is less than 0.02s.

Comment 15

[Michael Wu [:mwu]]

In general, this doesn't seem like a good idea. If we don't need certain pieces of memory, we should drop it altogether. If we really want swap, we have an actual swap mechanism provided by the kernel which has better global information than we do.

I think we should spend more time on things like bug 748598 and bug 883253 which lets us more intelligently drop anonymous pages that aren't needed.

Comment 16

[Thinker Li [:sinker]]

This bug base on the heuristic of a lot of data, on heap, are never changed/touched after initialized.  With this heuristic, it is far more efficient than general swap algorithm implemented kernel with less I/O operations.  It dumps the memory only one time per-startup.

Bug 748598 & bug 883253 can save a lot of memory, but it is not easy, I guess, to apply the mechanism of bug 883253 to all potential static content in the heap.  Maybe we need more experiments to understand how much memory saved by bug 883253 and this one are overlaid, then we can say one of bugs can be dropped or to keep both of them.

Comment 17

[Mike Hommey [:glandium]]

If we're filling static data in heap, and then never use it, we'd better find out what it is and *not* do it at all, than try to circumvent using dirty low-level tricks.

Comment 18

[Thinker Li [:sinker]]

I mean never change it, not never use it.  It is very common for most programs, and as what the experiments showed Gecko has also much data of this sort.  It seems very labor to figure out these data of large number and scattered piece by piece.  I am not sure how long it shall be to figure out most of them.

By the way, I think Ting-Yuan will upload a new patch in next few days.

Comment 19

[Mike Hommey [:glandium]]

(In reply to Thinker Li [:sinker] from comment #18)
> I mean never change it, not never use it.

If they are used, how is swapping them beneficial?

Comment 20

[Thinker Li [:sinker]]

For memory pressure, kernel will drop these pages without paging out.

Comment 21

[Mike Hommey [:glandium]]

(In reply to Thinker Li [:sinker] from comment #20)
> For memory pressure, kernel will drop these pages without paging out.

I don't get it. Either it's used and you don't want those pages to be dropped, or it's not in which case, back to comment 17.

Comment 22

[Thinker Li [:sinker]]

The content of these pages are never changed, but sometime we need them, and most time they can be dropped.  So, if they are written out to the storage at some point, kernel can reclaim these pages while others really need more memory.  If some of these content are accessed again, kernel know where to load it back.  It works just like swap devices implemented by the kernel, except to write pages out only one time, does not write out dirty pages after.

Comment 23

[Mike Hommey [:glandium]]

Then it would be good to know what those are, why we're most of the time not using them, and if we can't just not have them in the heap when they aren't needed.

Comment 24

[Ting-Yuan Huang]

Attachment: WIP -- anon_cache.diff

Although we are still discussing the validity, a working implementation is already done :-)

https://tbpl.mozilla.org/?tree=Try&rev=b13bec816b01

It works as following:

1. Intercept mmap() family so as to record all memory ranges mapped.
2. After booting into a stable state (30secs after showing the boot logo), it writes the recorded anonymous pages out and map them back as private.
2.1 before writing them out, a page is marked as read-only to detect accidental changes by other threads. If a pages is changed during writing, it will not be mapped back.
2.2 pthread_create() is also intercepted so as to make sure the stacks are not written out; The SIGSEGV handler also resides on those stacks.
2.3 If there are any threads get forked during the write process, the page permissions and SIGSEGV handler will be restored for the newly forked, child process.
2.4 There's a signal dispatching bug that prevents thumb-2 signal handlers from working correctly. After bug 919901 is resolved, the workaround in acache_help.s will be removed.

Comment 25

[Thinker Li [:sinker]]

(In reply to Mike Hommey [:glandium] from comment #23)
> Then it would be good to know what those are, why we're most of the time not
> using them, and if we can't just not have them in the heap when they aren't
> needed.

I am agree!  Maybe you, mwu, me or some one would like to do it, and at some point, we had fixed most of them, we don't need this any more.  For the short term, I believe this is an acceptable, less changes, and workable approach. If you have any better idea to fix the problem quickly, I would like to move to it.

Comment 26

[Mike Hommey [:glandium]]

Possibly dangerous and not widely tested hacks don't really qualify as a quick, acceptable and workable fix imho. Rushing things out is always a recipe for disaster. Sure B2G is kind of more controlled than Android, but i've had too many problems with segfault handlers on android devices to rush into using one for the entire heap on B2G.

For reference, there are kernels in the wild that are slow at dispatching segfaults (up to several hundreds of milliseconds per segfault on cyanogenmod, few milliseconds on "stock" kernels), because someone felt it would be nice to set user_debug to 31 on the kernel command line by default for some qualcomm targets. Which means manufacturers of b2g devices may be *very* likely to have that.

There are also other kernels that don't give out the right crash addresses to segfault handlers, which likely comes from manufacturer patches (so far, we know some HTC devices are affected ; we're looking in crash stats to find if there are more).

Comment 27

[Thinker Li [:sinker]]

I am certainly agree!  That is why we still need some ones to port b2g to various platforms for various driver, libraries, kernel and other platform issues.  I think it is lucky that this bug is very independent from the reset, so we can disable it if necessary without any run time overhead.  So, I think we should do some run time condition checking, to make sure it is failed ASAP and in a clear way.

Comment 28

[Thinker Li [:sinker]]

For performance of segfault, this bug causes segfault while it is writing out memory.  That means the segfault performance means nothing once finish the data writing.  So, I don't think segfault performance is a valid argument.

Comment 29

[Ting-Yuan Huang]

I can't find any specification defining or guaranteeing the behavior of a segfault handler. Maybe we should do as many error checks as possible. At least we can tell quickly and precisely if some problems are related to this hack. For B2G, it's up to manufacturers that if it is worth or not to trade debugging efforts for 10MB ~ 20MB system memory.

I can't agree anymore that it looks very dangerous and resulting problems (if any) are very hard to debug but can this be eased by careful reviewing and thorough testing?

By the way, a page is only protected when it is being swapped out. On unagi, the number of page faults caused by this hack is around 30 ~ 40. I'm not sure if this is a problem or not on some other devices/kernels but, just for reference, I can't tell the difference w/w.o. this patch on unagi in terms of UI responsiveness.

Comment 30

[Ting-Yuan Huang]

> I can't agree anymore that it looks very dangerous and resulting problems
> (if any) are very hard to debug but can this be eased by careful reviewing
> and thorough testing?

I mean "I can't agree any more", i.e., I agree that it's dangerous.

Comment 31

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 809708 [details] [diff] [review]
WIP -- anon_cache.diff

Review of attachment 809708 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for taking so long.  This was a complicated patch though ;-).  I think we should keep experimenting with this, but I have a lot of questions.

Does the pthreads implementation on b2g allow a writer to be starved?  That is, does pthread_rwlock_rdlock block if some other thread is blocked in pthread_rwlock_wdlock?  The pthreads specification leaves this behavior up to the implementation.  http://stackoverflow.com/questions/2190090/how-to-prevent-writer-starvation-in-a-read-write-lock-in-pthreads discusses how glibc behaves on Linux, but I don't know how the bionic pthreads implementation behaves.  Alternatively, do we prevent writer starvation via some other mechanism?  I didn't see anything but I could have missed it.  Your handwritten rwlock implementation definitely allows the writer to be starved.

You also don't ever use the read side of the rwlocks in svma, or vmas_rwlock, so those should just be plain mutexes.

I don't know if all the uses of volatile are correct without pulling out a C reference book, so I skipped those for now.  I also skipped most of the binary tree implementation.

This is only going to work in the main process, right?  Does our seccomp sandbox allow us to write to the file system in a child process?

::: b2g/app/nsBrowserApp.cpp
@@ +50,5 @@
> +#include <sys/types.h>
> +#endif
> +
> +#ifdef MOZ_WIDGET_GONK
> +static void _pthread_sleep(int sec)

You should add a comment about why sleep(3) does not work here.

@@ +69,5 @@
> +  pthread_mutex_destroy(&m);
> +  pthread_cond_destroy(&c);
> +}
> +
> +extern "C" void acache_trigger_write()__attribute__((weak));

surely there should be a space before __attribute__.

::: mozglue/build/BionicGlue.cpp
@@ +285,5 @@
> +}
> +
> +extern "C" NS_EXPORT int
> +pthread_create(pthread_t *thread_out, pthread_attr_t const * attr,
> +        void *(*start_routine)(void *), void * arg)

Does the interact correctly with Nuwa?  It also does stuff with pthread_create.

::: mozglue/build/acache.c
@@ +1,1 @@
> +#ifdef MOZ_WIDGET_GONK

Needs a license header, and we should just build this file conditionally (and not #ifdef all the contents).

@@ +17,5 @@
> +
> +#define USE_PTHREAD_LOCK
> +
> +#define NVMAS 1024
> +#define NSVMAS 4096

Magic constants need comments.

@@ +21,5 @@
> +#define NSVMAS 4096
> +
> +#ifndef MMAP2_SHIFT
> +#define  MMAP2_SHIFT  12
> +#endif

Here too.

@@ +52,5 @@
> +/********************
> + * read-write locks *
> + ********************/
> +
> +#ifndef USE_PTHREAD_LOCK

You defined USE_PTHREAD_LOCK about, so why do we need any of this?  I'm not thrilled about reimplementing pthread_rwlock.

@@ +61,5 @@
> +
> +static void _yield()
> +{
> +    //sched_yield();
> +}

...

@@ +75,5 @@
> +        _yield();
> +    }
> +}
> +
> +static void _write_unlock(struct rwlock *lock)

Assuming you don't remove all this code, this should assert that lock->mutex is 1.

@@ +93,5 @@
> +static void _read_unlock(struct rwlock *lock)
> +{
> +    while (!__sync_bool_compare_and_swap(&lock->mutex, 0, 1))
> +        _yield();
> +    lock->count--;

And this should assert that count > 0.

@@ +165,5 @@
> +    unsigned int end;
> +    unsigned int prot;
> +    volatile unsigned int skip;
> +    struct rwlock lock0;
> +    struct rwlock lock1;

You absolutely need comments about what each lock protects, and what the lock hierarchy is (if there is one).

Looks like lock0 protects skip.

@@ +176,5 @@
> +static struct rwlock vmas_rwlock;
> +
> +// Linear sorted array, used for writing anonymous pages out.
> +static struct svma svmas[NSVMAS];
> +static unsigned int nsvmas;

Please name this something else.  There's already NSVMAS which is a macro.

@@ +255,5 @@
> +
> +static struct vma *vma_successor(struct vma *node)
> +{
> +    if (!node)
> +        return NULL;

How come this one handles null but vma_predecessor doesn't?

@@ +537,5 @@
> +    int i, s;
> +
> +    s = svma_bsearch(end);
> +
> +    for (i = s; i >= 0 && svmas[i].end > start; i--) {

Is it intentional that if start == svmas[i].end we do not block it?

@@ +555,5 @@
> +    }
> +    nsvmas = 0;
> +}
> +
> +#define WRITE_SIZE (PAGESIZE * 16)

Some comments about how/why this was chosen would be nice.

@@ +608,5 @@
> +static struct sigaction act_sigsegv;
> +static struct sigaction oact_sigsegv;
> +
> +unsigned int totallycopied;
> +// dst, src and size must be divisable by PAGESIZE.

assert that?

@@ +680,5 @@
> +            incorecpy(addr, start, size);
> +            //memcpy(addr, start, size);
> +
> +            // This is a workaround to distribute IO evenly.
> +            msync(addr, size, MS_SYNC);

Why does this "distribute IO evenly"?  Because it does IO in small chunks instead of at the end of this function when you munmaq(p)?  Why does that matter?

@@ +683,5 @@
> +            // This is a workaround to distribute IO evenly.
> +            msync(addr, size, MS_SYNC);
> +        }
> +        _write_lock(&svmas[i].lock1);
> +        if (svmas[i].skip == 0 && (prot & PROT_READ)) {

Why do we need to recheck these conditions?  It looks like skip is protected by lock0, so it can't have changed.  And I don't see any locks protecting prot.

Comment 32

[Mike Lee [:mlee]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #31)
> ...
> 
> This is only going to work in the main process, right?  Does our seccomp
> sandbox allow us to write to the file system in a child process?
> 
> ...

Paul, any input here re Kyle's question about the seccomp sandbox?

Comment 33

[Thinker Li [:sinker]]

> This is only going to work in the main process, right?  Does our seccomp
> sandbox allow us to write to the file system in a child process?

Yes! This patch is only work in the parent process.  For content process, we have already Nuwa.

Comment 34

[Ting-Yuan Huang]

Attachment: WIP -- anon_cache.patch

Thanks! I refined the patch according to your comments. Would you please give me some feedback one more time before a formal review?

> Does the pthreads implementation on b2g allow a writer to be starved?  That
> is, does pthread_rwlock_rdlock block if some other thread is blocked in
> pthread_rwlock_wdlock?  The pthreads specification leaves this behavior up
> to the implementation. 
> http://stackoverflow.com/questions/2190090/how-to-prevent-writer-starvation-
> in-a-read-write-lock-in-pthreads discusses how glibc behaves on Linux, but I
> don't know how the bionic pthreads implementation behaves.  Alternatively,
> do we prevent writer starvation via some other mechanism?  I didn't see
> anything but I could have missed it.  Your handwritten rwlock implementation
> definitely allows the writer to be starved.
> 
> You also don't ever use the read side of the rwlocks in svma, or
> vmas_rwlock, so those should just be plain mutexes.

The read-write lock is intended to reduce the time of interlocking and improve UI responsiveness. The write lock should only wait for writing a few pages out, which is a bounded, short time.

Anyway, after reviewing and refactoring carefully, I found it is possible to get rid of read-write lock, that is, to use a normal mutex to achieve the same effect.

> I don't know if all the uses of volatile are correct without pulling out a C
> reference book, so I skipped those for now.  I also skipped most of the
> binary tree implementation.

After refactoring, there are two volatile: skip and lock, which are either protected by mutexes or memory barriers (before read and after write).

> This is only going to work in the main process, right?  Does our seccomp
> sandbox allow us to write to the file system in a child process?

Yes. 

> ::: b2g/app/nsBrowserApp.cpp
> @@ +50,5 @@
> > +#include <sys/types.h>
> > +#endif
> > +
> > +#ifdef MOZ_WIDGET_GONK
> > +static void _pthread_sleep(int sec)
> 
> You should add a comment about why sleep(3) does not work here.

Done. Experiments shows that 30 secs should be enough for the system to boot into a steady state.

> @@ +69,5 @@
> > +  pthread_mutex_destroy(&m);
> > +  pthread_cond_destroy(&c);
> > +}
> > +
> > +extern "C" void acache_trigger_write()__attribute__((weak));
> 
> surely there should be a space before __attribute__.

Done.

> ::: mozglue/build/BionicGlue.cpp
> @@ +285,5 @@
> > +}
> > +
> > +extern "C" NS_EXPORT int
> > +pthread_create(pthread_t *thread_out, pthread_attr_t const * attr,
> > +        void *(*start_routine)(void *), void * arg)
> 
> Does the interact correctly with Nuwa?  It also does stuff with
> pthread_create.

The wrapper makes sure that, when creating new threads, their stacks are not registered to be written out. I think this is compatible with NUWA, although the integration must be done manually. I'll discuss with Thinker and Cervantes and provide the solution in the next patch. 

> ::: mozglue/build/acache.c
> @@ +1,1 @@
> > +#ifdef MOZ_WIDGET_GONK
> 
> Needs a license header, and we should just build this file conditionally
> (and not #ifdef all the contents).

Done.

> @@ +17,5 @@
> > +
> > +#define USE_PTHREAD_LOCK
> > +
> > +#define NVMAS 1024
> > +#define NSVMAS 4096
> 
> Magic constants need comments.

They are removed by dynamically expanding the data structure.
 
> @@ +21,5 @@
> > +#define NSVMAS 4096
> > +
> > +#ifndef MMAP2_SHIFT
> > +#define  MMAP2_SHIFT  12
> > +#endif
> 
> Here too.

Done. The offset in MMAP2 in Linux is always multiple of 4096, hence shift 12.

> @@ +52,5 @@
> > +/********************
> > + * read-write locks *
> > + ********************/
> > +
> > +#ifndef USE_PTHREAD_LOCK
> 
> You defined USE_PTHREAD_LOCK about, so why do we need any of this?  I'm not
> thrilled about reimplementing pthread_rwlock.
> 
> @@ +61,5 @@
> > +
> > +static void _yield()
> > +{
> > +    //sched_yield();
> > +}
> 
> ...
> 
> @@ +75,5 @@
> > +        _yield();
> > +    }
> > +}
> > +
> > +static void _write_unlock(struct rwlock *lock)
> 
> Assuming you don't remove all this code, this should assert that lock->mutex
> is 1.
> 
> @@ +93,5 @@
> > +static void _read_unlock(struct rwlock *lock)
> > +{
> > +    while (!__sync_bool_compare_and_swap(&lock->mutex, 0, 1))
> > +        _yield();
> > +    lock->count--;
> 
> And this should assert that count > 0.

pthread_* is not safe in signal handlers, so I choose to implement a spinlock in userspace. Now spinlock and pthread_mutex_* are used. Spinlock is used only when necessary; It is used to protect two critical sections in very short times.

> @@ +165,5 @@
> > +    unsigned int end;
> > +    unsigned int prot;
> > +    volatile unsigned int skip;
> > +    struct rwlock lock0;
> > +    struct rwlock lock1;
> 
> You absolutely need comments about what each lock protects, and what the
> lock hierarchy is (if there is one).
> 
> Looks like lock0 protects skip.

Done.

> @@ +176,5 @@
> > +static struct rwlock vmas_rwlock;
> > +
> > +// Linear sorted array, used for writing anonymous pages out.
> > +static struct svma svmas[NSVMAS];
> > +static unsigned int nsvmas;
> 
> Please name this something else.  There's already NSVMAS which is a macro.

Done. They are eliminated after refactoring.

> @@ +255,5 @@
> > +
> > +static struct vma *vma_successor(struct vma *node)
> > +{
> > +    if (!node)
> > +        return NULL;
> 
> How come this one handles null but vma_predecessor doesn't?

Done.

> @@ +537,5 @@
> > +    int i, s;
> > +
> > +    s = svma_bsearch(end);
> > +
> > +    for (i = s; i >= 0 && svmas[i].end > start; i--) {
> 
> Is it intentional that if start == svmas[i].end we do not block it?

Yes. The intervals are all half open, [).
 
> @@ +555,5 @@
> > +    }
> > +    nsvmas = 0;
> > +}
> > +
> > +#define WRITE_SIZE (PAGESIZE * 16)
> 
> Some comments about how/why this was chosen would be nice.

Done.

> @@ +608,5 @@
> > +static struct sigaction act_sigsegv;
> > +static struct sigaction oact_sigsegv;
> > +
> > +unsigned int totallycopied;
> > +// dst, src and size must be divisable by PAGESIZE.
> 
> assert that?

Done. 

> @@ +680,5 @@
> > +            incorecpy(addr, start, size);
> > +            //memcpy(addr, start, size);
> > +
> > +            // This is a workaround to distribute IO evenly.
> > +            msync(addr, size, MS_SYNC);
> 
> Why does this "distribute IO evenly"?  Because it does IO in small chunks
> instead of at the end of this function when you munmaq(p)?  Why does that
> matter?

Yes, it is used to prevent large IO at the end of the function. I'm not clear why it is the case, but the IO did block UI seriously. The same behavior can be observed on desktop firefox: when doing IO intensive works, such as repo sync, firefox doesn't respond.

> @@ +683,5 @@
> > +            // This is a workaround to distribute IO evenly.
> > +            msync(addr, size, MS_SYNC);
> > +        }
> > +        _write_lock(&svmas[i].lock1);
> > +        if (svmas[i].skip == 0 && (prot & PROT_READ)) {
> 
> Why do we need to recheck these conditions?  It looks like skip is protected
> by lock0, so it can't have changed.  And I don't see any locks protecting
> prot.

You are right. prot needn't to be checked again.

skip is checked intentionally. It is protected by the lock0 and lock1.

Comment 35

[Mike Hommey [:glandium]]

(In reply to Ting-Yuan Huang from comment #34)
> Done. The offset in MMAP2 in Linux is always multiple of 4096, hence shift
> 12.

The offset in MMAP2 in Linux is always multiple of 4096 *on some architectures*. It is always multiple of the page size. The page size is not necessarily 4096. But it is 4096 on arm.</pedantic>

Comment 36

[Ting-Yuan Huang]

Attachment: WIP -- anon_cache.patch

Two updates:

1. It is possible to break system calls when settings memory regions read-only. Those system calls will give EFAULT, instead of synchronously signaling the faulting thread and then restart. The proposed solution is to add an additional check in HANDLE_EINTR() and retry. It is possible that the EFAULT doesn't come from the anonymous page writer but that should be fine since the writer only takes a limited amount of time and eventually the flow goes on (i.e. stop retrying for EFAULT).

2. Removed MMAP2_SHIFT; Use PAGE_SHIFT instead.

Comment 37

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Just fyi I hope to get to this this week.

Comment 38

[Mike Lee [:mlee]]

Ting, assigning this to you since you've submitted a patch.

Comment 39

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

The patch is from Ting-Yuan, not Ting-Yu.

Comment 40

[Joe Cheng [:jcheng] (please needinfo)]

triage: since tarako has zram enabled, this is currently not needed for tarako at this moment. remove [tarako]

Comment 41

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

So is this still something we want to do at all?  Or has zram obsoleted this?

Comment 42

[Ting-Yuan Huang]

I believe the gain here largely overlap zram.