Re: [RFC v2] Support volatile range for anon vma
From: Minchan Kim <minchan@kernel.org>
Date: 2012-12-04 07:22:10
Also in:
lkml
On Mon, Dec 03, 2012 at 04:57:20PM -0800, John Stultz wrote:
On 12/03/2012 04:00 PM, Minchan Kim wrote:quoted
On Wed, Nov 28, 2012 at 08:18:01PM -0800, John Stultz wrote:quoted
On 11/21/2012 04:36 PM, John Stultz wrote:quoted
2) Being able to use this with tmpfs files. I'm currently trying to better understand the rmap code, looking to see if there's a way to have try_to_unmap_file() work similarly to try_to_unmap_anon(), to allow allow users to madvise() on mmapped tmpfs files. This would provide a very similar interface as to what I've been proposing with fadvise/fallocate, but just using process virtual addresses instead of (fd, offset) pairs. The benefit with (fd,offset) pairs for Android is that its easier to manage shared volatile ranges between two processes that are sharing data via an mmapped tmpfs file (although this actual use case may be fairly rare). I believe we should still be able to rework the ashmem internals to use madvise (which would provide legacy support for existing android apps), so then its just a question of if we could then eventually convince Android apps to use the madvise interface directly, rather then the ashmem unpin ioctl.Hey Minchan, I've been playing around with your patch trying to better understand your approach and to extend it to support tmpfs files. In doing so I've found a few bugs, and have some rough fixes I wanted to share. There's still a few edge cases I need to deal with (the vma-purged flag isn't being properly handled through vma merge/split operations), but its starting to come along.Hmm, my patch doesn't allow to merge volatile with another one by inserting VM_VOLATILE into VM_SPECIAL so I guess merge isn't problem. In case of split, __split_vma copy old vma to new vma like this *new = *vma; So the problem shouldn't happen, I guess. Did you see the real problem about that?Yes, depending on the pattern that MADV_VOLATILE and MADV_NOVOLATILE is applied, we can get a result where data is purged, but we aren't notified of it. Also, since madvise returns early if it encounters an error, in the case where you have checkerboard volatile regions (say every other page is volatile), which you mark non-volatile with one large MADV_NOVOLATILE call, the first volatile vma will be marked non-volatile, but since it returns purged, the madvise loop will stop and the following volatile regions will be left volatile. The patches in the git tree below which handle the perged state better seem to work for my tests, as far as resolving any overlapping calls. Of course there may yet still be problems I've not found.quoted
quoted
Anyway, take a look at the tree here and let me know what you think. http://git.linaro.org/gitweb?p=people/jstultz/android-dev.git;a=shortlog;h=refs/heads/dev/minchan-anonvolEager to hear what you think!
Below two patches look good to me. [rmap: Simplify volatility checking by moving it out of try_to_unmap_one] [rmap: ClearPageDirty() when returning SWAP_DISCARD] [madvise: Fix NOVOLATILE bug] I can't understand description of the patch. Could you elaborate it with example? [madvise: Fixup vma->purged handling] I included VM_VOLATILE into VM_SPECIAL intentionally. If comment of VM_SPECIAL is right, merge with volatile vmas shouldn't happen. So I guess you see other problem. When I see my source code today, locking scheme/purge handling is totally broken. I will look at it. Maybe you are seeing bug related that. Part of patch is needed. It could be separate patch. I will merge it. ff --git a/mm/madvise.c b/mm/madvise.c index 65af0b5..5469d76 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c@@ -139,8 +139,10 @@ success: if (behavior == MADV_NOVOLATILE || behavior == MADV_VOLATILE) volatile_lock(vma); vma->vm_flags = new_flags; - if (behavior == MADV_NOVOLATILE) + if (behavior == MADV_NOVOLATILE) { error = vma->purged; + vma->purged = 0; + } if (behavior == MADV_NOVOLATILE || behavior == MADV_VOLATILE) volatile_unlock(vma); out:
First of all, after I resolve above issue, let's talk about tmpfs volatile. Thanks for the fix, John!
Thanks again! -john -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
-- Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>