On Mon, Dec 03, 2012 at 04:57:20PM -0800, John Stultz wrote: > On 12/03/2012 04:00 PM, Minchan Kim wrote: > >On Wed, Nov 28, 2012 at 08:18:01PM -0800, John Stultz wrote: > >>On 11/21/2012 04:36 PM, John Stultz wrote: > >>>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. > > >>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-anonvol > > Eager 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@xxxxxxxxx. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a> -- Kind regards, Minchan Kim -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>