From: John Stultz <john.stultz@linaro.org> To: Michel Lespinasse <walken@google.com> Cc: LKML <linux-kernel@vger.kernel.org>, Andrew Morton <akpm@linux-foundation.org>, Android Kernel Team <kernel-team@android.com>, Robert Love <rlove@google.com>, Mel Gorman <mel@csn.ul.ie>, Hugh Dickins <hughd@google.com>, Dave Hansen <dave@linux.vnet.ibm.com>, Rik van Riel <riel@redhat.com>, Dmitry Adamushko <dmitry.adamushko@gmail.com>, Dave Chinner <david@fromorbit.com>, Neil Brown <neilb@suse.de>, Andrea Righi <andrea@betterlinux.com>, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>, Mike Hommey <mh@glandium.org>, Jan Kara <jack@suse.cz>, KOSAKI Motohiro <kosaki.motohiro@gmail.com>, Minchan Kim <minchan@kernel.org>, "linux-mm@kvack.org" <linux-mm@kvack.org> Subject: Re: [PATCH 1/5] [RFC] Add volatile range management code Date: Thu, 09 Aug 2012 12:11:51 -0700 [thread overview] Message-ID: <50240B77.2060204@linaro.org> (raw) In-Reply-To: <CANN689HWYO5DD_p7yY39ethcFu_JO9hudMcDHd=K8FUfhpHZOg@mail.gmail.com> On 08/09/2012 02:46 AM, Michel Lespinasse wrote: > On Fri, Jul 27, 2012 at 8:57 PM, John Stultz <john.stultz@linaro.org> wrote: >> v5: >> * Drop intervaltree for prio_tree usage per Michel & >> Dmitry's suggestions. > Actually, I believe the ranges you need to track are non-overlapping, correct ? Correct. Any overlapping range is coalesced. > If that is the case, a simple rbtree, sorted by start-of-range > address, would work best. > (I am trying to remove prio_tree users... :) Sigh. Sure. Although I've blown with the wind on a number of different approaches for storing the ranges. I'm not particularly passionate about it, but the continual conflicting suggestions are a slight frustration. :) >> + /* First, find any existing intervals that overlap */ >> + prio_tree_iter_init(&iter, root, start, end); > Note that prio tree iterations take intervals as [start; last] not [start; end[ > So if you want to stick with prio trees, you would have to use end-1 here. Thanks! I think I hit this off-by-one issue in my testing, but fixed it on the backend w/ : modify_range(&inode->i_data, start, end-1, &mark_nonvolatile_page); Clearly fixing it at the start instead of papering over it is better. >> + node = prio_tree_next(&iter); >> + while (node) { > I'm confused, I don't think you ever expect more than one range to > match, do you ??? So yea. If you already have two ranges (0-5),(10-15) and then add range (0-20) we need to coalesce the two existing ranges into the new one. > This is far from a complete code review, but I just wanted to point > out a couple details that jumped to me first. I am afraid I am missing > some of the background about how the feature is to be used to really > dig into the rest of the changes at this point :/ Well, I really appreciate any feedback here. thanks -john
WARNING: multiple messages have this Message-ID (diff)
From: John Stultz <john.stultz@linaro.org> To: Michel Lespinasse <walken@google.com> Cc: LKML <linux-kernel@vger.kernel.org>, Andrew Morton <akpm@linux-foundation.org>, Android Kernel Team <kernel-team@android.com>, Robert Love <rlove@google.com>, Mel Gorman <mel@csn.ul.ie>, Hugh Dickins <hughd@google.com>, Dave Hansen <dave@linux.vnet.ibm.com>, Rik van Riel <riel@redhat.com>, Dmitry Adamushko <dmitry.adamushko@gmail.com>, Dave Chinner <david@fromorbit.com>, Neil Brown <neilb@suse.de>, Andrea Righi <andrea@betterlinux.com>, "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>, Mike Hommey <mh@glandium.org>, Jan Kara <jack@suse.cz>, KOSAKI Motohiro <kosaki.motohiro@gmail.com>, Minchan Kim <minchan@kernel.org>, "linux-mm@kvack.org" <linux-mm@kvack.org> Subject: Re: [PATCH 1/5] [RFC] Add volatile range management code Date: Thu, 09 Aug 2012 12:11:51 -0700 [thread overview] Message-ID: <50240B77.2060204@linaro.org> (raw) In-Reply-To: <CANN689HWYO5DD_p7yY39ethcFu_JO9hudMcDHd=K8FUfhpHZOg@mail.gmail.com> On 08/09/2012 02:46 AM, Michel Lespinasse wrote: > On Fri, Jul 27, 2012 at 8:57 PM, John Stultz <john.stultz@linaro.org> wrote: >> v5: >> * Drop intervaltree for prio_tree usage per Michel & >> Dmitry's suggestions. > Actually, I believe the ranges you need to track are non-overlapping, correct ? Correct. Any overlapping range is coalesced. > If that is the case, a simple rbtree, sorted by start-of-range > address, would work best. > (I am trying to remove prio_tree users... :) Sigh. Sure. Although I've blown with the wind on a number of different approaches for storing the ranges. I'm not particularly passionate about it, but the continual conflicting suggestions are a slight frustration. :) >> + /* First, find any existing intervals that overlap */ >> + prio_tree_iter_init(&iter, root, start, end); > Note that prio tree iterations take intervals as [start; last] not [start; end[ > So if you want to stick with prio trees, you would have to use end-1 here. Thanks! I think I hit this off-by-one issue in my testing, but fixed it on the backend w/ : modify_range(&inode->i_data, start, end-1, &mark_nonvolatile_page); Clearly fixing it at the start instead of papering over it is better. >> + node = prio_tree_next(&iter); >> + while (node) { > I'm confused, I don't think you ever expect more than one range to > match, do you ??? So yea. If you already have two ranges (0-5),(10-15) and then add range (0-20) we need to coalesce the two existing ranges into the new one. > This is far from a complete code review, but I just wanted to point > out a couple details that jumped to me first. I am afraid I am missing > some of the background about how the feature is to be used to really > dig into the rest of the changes at this point :/ Well, I really appreciate any feedback here. thanks -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>
next prev parent reply other threads:[~2012-08-09 19:14 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-07-28 3:57 [PATCH 0/5][RFC] Fallocate Volatile Ranges v6 John Stultz 2012-07-28 3:57 ` John Stultz 2012-07-28 3:57 ` [PATCH 1/5] [RFC] Add volatile range management code John Stultz 2012-07-28 3:57 ` John Stultz 2012-08-09 9:46 ` Michel Lespinasse 2012-08-09 9:46 ` Michel Lespinasse 2012-08-09 13:35 ` Andrea Righi 2012-08-09 13:35 ` Andrea Righi 2012-08-09 19:33 ` John Stultz 2012-08-09 19:33 ` John Stultz 2012-08-09 19:39 ` Andrea Righi 2012-08-09 19:39 ` Andrea Righi 2012-08-09 19:11 ` John Stultz [this message] 2012-08-09 19:11 ` John Stultz 2012-07-28 3:57 ` [PATCH 2/5] [RFC] tmpfs: Add FALLOC_FL_MARK_VOLATILE/UNMARK_VOLATILE handlers John Stultz 2012-07-28 3:57 ` John Stultz 2012-07-28 3:57 ` [PATCH 3/5] [RFC] ashmem: Convert ashmem to use volatile ranges John Stultz 2012-07-28 3:57 ` John Stultz 2012-07-28 3:57 ` [PATCH 4/5] [RFC][HACK] Add LRU_VOLATILE support to the VM John Stultz 2012-07-28 3:57 ` John Stultz 2012-08-06 3:04 ` Minchan Kim 2012-08-06 3:04 ` Minchan Kim 2012-08-06 15:46 ` Dan Magenheimer 2012-08-06 15:46 ` Dan Magenheimer 2012-08-07 0:56 ` Minchan Kim 2012-08-07 0:56 ` Minchan Kim 2012-08-07 1:26 ` Dan Magenheimer 2012-08-07 1:26 ` Dan Magenheimer 2012-08-07 1:45 ` Minchan Kim 2012-08-07 1:45 ` Minchan Kim 2012-08-06 20:38 ` John Stultz 2012-08-06 20:38 ` John Stultz 2012-07-28 3:57 ` [PATCH 5/5] [RFC][HACK] Switch volatile/shmem over to LRU_VOLATILE John Stultz 2012-07-28 3:57 ` John Stultz 2012-08-09 9:28 ` [PATCH 0/5][RFC] Fallocate Volatile Ranges v6 Michel Lespinasse 2012-08-09 9:28 ` Michel Lespinasse 2012-08-09 18:45 ` John Stultz 2012-08-09 18:45 ` John Stultz [not found] <1343346546-53230-1-git-send-email-john.stultz@linaro.org> 2012-07-26 23:49 ` [PATCH 1/5] [RFC] Add volatile range management code John Stultz 2012-07-26 23:52 ` John Stultz -- strict thread matches above, loose matches on Subject: below -- 2012-06-27 4:17 [PATCH 0/5][RFC] Fallocate Volatile Ranges v5 John Stultz 2012-06-27 4:17 ` [PATCH 1/5] [RFC] Add volatile range management code John Stultz 2012-06-27 4:17 ` John Stultz
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=50240B77.2060204@linaro.org \ --to=john.stultz@linaro.org \ --cc=akpm@linux-foundation.org \ --cc=andrea@betterlinux.com \ --cc=aneesh.kumar@linux.vnet.ibm.com \ --cc=dave@linux.vnet.ibm.com \ --cc=david@fromorbit.com \ --cc=dmitry.adamushko@gmail.com \ --cc=hughd@google.com \ --cc=jack@suse.cz \ --cc=kernel-team@android.com \ --cc=kosaki.motohiro@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mel@csn.ul.ie \ --cc=mh@glandium.org \ --cc=minchan@kernel.org \ --cc=neilb@suse.de \ --cc=riel@redhat.com \ --cc=rlove@google.com \ --cc=walken@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.