Re: [PATCH v2 6/6] mm/balloon_compaction: general cleanup
On Sat, Sep 13, 2014 at 4:06 AM, Andrew Morton wrote: > On Fri, 12 Sep 2014 17:04:04 -0700 Andrew Morton > wrote: > >> On Sat, 30 Aug 2014 20:41:27 +0400 Konstantin Khlebnikov >> wrote: >> >> > * move special branch for balloon migraion into migrate_pages >> > * remove special mapping for balloon and its flag AS_BALLOON_MAP >> > * embed struct balloon_dev_info into struct virtio_balloon >> > * cleanup balloon_page_dequeue, kill balloon_page_free >> >> Not sure what's going on here - your include/linux/balloon_compaction.h >> seems significantly different from mine. > > OK, I worked it out. > >> I think I'll just drop this patch - it's quite inconvenient to have a >> large "general cleanup" coming after a stack of significant functional >> changes. It makes review, debug, fix, merge and reversion harder. >> Let's worry about it later. > > But I'm still thinking we should defer this one? It seems in this case massive cleanup before fixies leads to much bigger mess. I've separated fixes specially for merging into stable kernels. The rest patches mostly rewrites this code using new approatch. I don't know how to make it more reviewable, it's easier to write a new version from the scratch. Probably rework should be untied form fixes and sent as separate patchset. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 6/6] mm/balloon_compaction: general cleanup
On Fri, 12 Sep 2014 17:04:04 -0700 Andrew Morton wrote: > On Sat, 30 Aug 2014 20:41:27 +0400 Konstantin Khlebnikov > wrote: > > > * move special branch for balloon migraion into migrate_pages > > * remove special mapping for balloon and its flag AS_BALLOON_MAP > > * embed struct balloon_dev_info into struct virtio_balloon > > * cleanup balloon_page_dequeue, kill balloon_page_free > > Not sure what's going on here - your include/linux/balloon_compaction.h > seems significantly different from mine. OK, I worked it out. > I think I'll just drop this patch - it's quite inconvenient to have a > large "general cleanup" coming after a stack of significant functional > changes. It makes review, debug, fix, merge and reversion harder. > Let's worry about it later. But I'm still thinking we should defer this one? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 6/6] mm/balloon_compaction: general cleanup
On Sat, 30 Aug 2014 20:41:27 +0400 Konstantin Khlebnikov wrote: > * move special branch for balloon migraion into migrate_pages > * remove special mapping for balloon and its flag AS_BALLOON_MAP > * embed struct balloon_dev_info into struct virtio_balloon > * cleanup balloon_page_dequeue, kill balloon_page_free Not sure what's going on here - your include/linux/balloon_compaction.h seems significantly different from mine. I think I'll just drop this patch - it's quite inconvenient to have a large "general cleanup" coming after a stack of significant functional changes. It makes review, debug, fix, merge and reversion harder. Let's worry about it later. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 6/6] mm/balloon_compaction: general cleanup
On Sat, 30 Aug 2014 20:41:27 +0400 Konstantin Khlebnikov koc...@gmail.com wrote: * move special branch for balloon migraion into migrate_pages * remove special mapping for balloon and its flag AS_BALLOON_MAP * embed struct balloon_dev_info into struct virtio_balloon * cleanup balloon_page_dequeue, kill balloon_page_free Not sure what's going on here - your include/linux/balloon_compaction.h seems significantly different from mine. I think I'll just drop this patch - it's quite inconvenient to have a large general cleanup coming after a stack of significant functional changes. It makes review, debug, fix, merge and reversion harder. Let's worry about it later. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 6/6] mm/balloon_compaction: general cleanup
On Fri, 12 Sep 2014 17:04:04 -0700 Andrew Morton a...@linux-foundation.org wrote: On Sat, 30 Aug 2014 20:41:27 +0400 Konstantin Khlebnikov koc...@gmail.com wrote: * move special branch for balloon migraion into migrate_pages * remove special mapping for balloon and its flag AS_BALLOON_MAP * embed struct balloon_dev_info into struct virtio_balloon * cleanup balloon_page_dequeue, kill balloon_page_free Not sure what's going on here - your include/linux/balloon_compaction.h seems significantly different from mine. OK, I worked it out. I think I'll just drop this patch - it's quite inconvenient to have a large general cleanup coming after a stack of significant functional changes. It makes review, debug, fix, merge and reversion harder. Let's worry about it later. But I'm still thinking we should defer this one? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 6/6] mm/balloon_compaction: general cleanup
On Sat, Sep 13, 2014 at 4:06 AM, Andrew Morton a...@linux-foundation.org wrote: On Fri, 12 Sep 2014 17:04:04 -0700 Andrew Morton a...@linux-foundation.org wrote: On Sat, 30 Aug 2014 20:41:27 +0400 Konstantin Khlebnikov koc...@gmail.com wrote: * move special branch for balloon migraion into migrate_pages * remove special mapping for balloon and its flag AS_BALLOON_MAP * embed struct balloon_dev_info into struct virtio_balloon * cleanup balloon_page_dequeue, kill balloon_page_free Not sure what's going on here - your include/linux/balloon_compaction.h seems significantly different from mine. OK, I worked it out. I think I'll just drop this patch - it's quite inconvenient to have a large general cleanup coming after a stack of significant functional changes. It makes review, debug, fix, merge and reversion harder. Let's worry about it later. But I'm still thinking we should defer this one? It seems in this case massive cleanup before fixies leads to much bigger mess. I've separated fixes specially for merging into stable kernels. The rest patches mostly rewrites this code using new approatch. I don't know how to make it more reviewable, it's easier to write a new version from the scratch. Probably rework should be untied form fixes and sent as separate patchset. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 6/6] mm/balloon_compaction: general cleanup
On Sat, Aug 30, 2014 at 08:41:27PM +0400, Konstantin Khlebnikov wrote:
> From: Konstantin Khlebnikov
>
> * move special branch for balloon migraion into migrate_pages
> * remove special mapping for balloon and its flag AS_BALLOON_MAP
> * embed struct balloon_dev_info into struct virtio_balloon
> * cleanup balloon_page_dequeue, kill balloon_page_free
>
> Signed-off-by: Konstantin Khlebnikov
> ---
> drivers/virtio/virtio_balloon.c| 77 -
> include/linux/balloon_compaction.h | 127 ++---
> include/linux/migrate.h| 11 --
> include/linux/pagemap.h| 18 ---
> mm/balloon_compaction.c| 214
>
> mm/migrate.c | 29 +
> 6 files changed, 134 insertions(+), 342 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 25ebe8e..c84d6a8 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -59,7 +59,7 @@ struct virtio_balloon
>* Each page on this list adds VIRTIO_BALLOON_PAGES_PER_PAGE
>* to num_pages above.
>*/
> - struct balloon_dev_info *vb_dev_info;
> + struct balloon_dev_info vb_dev_info;
>
> /* Synchronize access/update to this struct virtio_balloon elements */
> struct mutex balloon_lock;
> @@ -127,7 +127,7 @@ static void set_page_pfns(u32 pfns[], struct page *page)
>
> static void fill_balloon(struct virtio_balloon *vb, size_t num)
> {
> - struct balloon_dev_info *vb_dev_info = vb->vb_dev_info;
> + struct balloon_dev_info *vb_dev_info = >vb_dev_info;
>
> /* We can only do one array worth at a time. */
> num = min(num, ARRAY_SIZE(vb->pfns));
> @@ -163,15 +163,15 @@ static void release_pages_by_pfn(const u32 pfns[],
> unsigned int num)
> /* Find pfns pointing at start of each page, get pages and free them. */
> for (i = 0; i < num; i += VIRTIO_BALLOON_PAGES_PER_PAGE) {
> struct page *page = balloon_pfn_to_page(pfns[i]);
> - balloon_page_free(page);
> adjust_managed_page_count(page, 1);
> + put_page(page);
> }
> }
>
> static void leak_balloon(struct virtio_balloon *vb, size_t num)
> {
> struct page *page;
> - struct balloon_dev_info *vb_dev_info = vb->vb_dev_info;
> + struct balloon_dev_info *vb_dev_info = >vb_dev_info;
>
> /* We can only do one array worth at a time. */
> num = min(num, ARRAY_SIZE(vb->pfns));
> @@ -353,12 +353,11 @@ static int init_vqs(struct virtio_balloon *vb)
> return 0;
> }
>
> -static const struct address_space_operations virtio_balloon_aops;
> #ifdef CONFIG_BALLOON_COMPACTION
> /*
> * virtballoon_migratepage - perform the balloon page migration on behalf of
> *a compation thread. (called under page lock)
> - * @mapping: the page->mapping which will be assigned to the new migrated
> page.
> + * @vb_dev_info: the balloon device
> * @newpage: page that will replace the isolated page after migration
> finishes.
> * @page : the isolated (old) page that is about to be migrated to newpage.
> * @mode : compaction mode -- not used for balloon page migration.
> @@ -373,17 +372,13 @@ static const struct address_space_operations
> virtio_balloon_aops;
> * This function preforms the balloon page migration task.
> * Called through balloon_mapping->a_ops->migratepage
> */
> -static int virtballoon_migratepage(struct address_space *mapping,
> +static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
> struct page *newpage, struct page *page, enum migrate_mode mode)
> {
> - struct balloon_dev_info *vb_dev_info = balloon_page_device(page);
> - struct virtio_balloon *vb;
> + struct virtio_balloon *vb = container_of(vb_dev_info,
> + struct virtio_balloon, vb_dev_info);
> unsigned long flags;
>
> - BUG_ON(!vb_dev_info);
> -
> - vb = vb_dev_info->balloon_device;
> -
> /*
>* In order to avoid lock contention while migrating pages concurrently
>* to leak_balloon() or fill_balloon() we just give up the balloon_lock
> @@ -395,42 +390,34 @@ static int virtballoon_migratepage(struct address_space
> *mapping,
> if (!mutex_trylock(>balloon_lock))
> return -EAGAIN;
>
> + get_page(newpage); /* balloon reference */
> +
> /* balloon's page migration 1st step -- inflate "newpage" */
> spin_lock_irqsave(_dev_info->pages_lock, flags);
> - balloon_page_insert(newpage, mapping, _dev_info->pages);
> + balloon_page_insert(vb_dev_info, newpage);
> vb_dev_info->isolated_pages--;
> spin_unlock_irqrestore(_dev_info->pages_lock, flags);
> vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
> set_page_pfns(vb->pfns, newpage);
> tell_host(vb, vb->inflate_vq);
>
> - /*
> - * balloon's page
Re: [PATCH v2 6/6] mm/balloon_compaction: general cleanup
On Sat, Aug 30, 2014 at 08:41:27PM +0400, Konstantin Khlebnikov wrote:
From: Konstantin Khlebnikov k.khlebni...@samsung.com
* move special branch for balloon migraion into migrate_pages
* remove special mapping for balloon and its flag AS_BALLOON_MAP
* embed struct balloon_dev_info into struct virtio_balloon
* cleanup balloon_page_dequeue, kill balloon_page_free
Signed-off-by: Konstantin Khlebnikov k.khlebni...@samsung.com
---
drivers/virtio/virtio_balloon.c| 77 -
include/linux/balloon_compaction.h | 127 ++---
include/linux/migrate.h| 11 --
include/linux/pagemap.h| 18 ---
mm/balloon_compaction.c| 214
mm/migrate.c | 29 +
6 files changed, 134 insertions(+), 342 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 25ebe8e..c84d6a8 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -59,7 +59,7 @@ struct virtio_balloon
* Each page on this list adds VIRTIO_BALLOON_PAGES_PER_PAGE
* to num_pages above.
*/
- struct balloon_dev_info *vb_dev_info;
+ struct balloon_dev_info vb_dev_info;
/* Synchronize access/update to this struct virtio_balloon elements */
struct mutex balloon_lock;
@@ -127,7 +127,7 @@ static void set_page_pfns(u32 pfns[], struct page *page)
static void fill_balloon(struct virtio_balloon *vb, size_t num)
{
- struct balloon_dev_info *vb_dev_info = vb-vb_dev_info;
+ struct balloon_dev_info *vb_dev_info = vb-vb_dev_info;
/* We can only do one array worth at a time. */
num = min(num, ARRAY_SIZE(vb-pfns));
@@ -163,15 +163,15 @@ static void release_pages_by_pfn(const u32 pfns[],
unsigned int num)
/* Find pfns pointing at start of each page, get pages and free them. */
for (i = 0; i num; i += VIRTIO_BALLOON_PAGES_PER_PAGE) {
struct page *page = balloon_pfn_to_page(pfns[i]);
- balloon_page_free(page);
adjust_managed_page_count(page, 1);
+ put_page(page);
}
}
static void leak_balloon(struct virtio_balloon *vb, size_t num)
{
struct page *page;
- struct balloon_dev_info *vb_dev_info = vb-vb_dev_info;
+ struct balloon_dev_info *vb_dev_info = vb-vb_dev_info;
/* We can only do one array worth at a time. */
num = min(num, ARRAY_SIZE(vb-pfns));
@@ -353,12 +353,11 @@ static int init_vqs(struct virtio_balloon *vb)
return 0;
}
-static const struct address_space_operations virtio_balloon_aops;
#ifdef CONFIG_BALLOON_COMPACTION
/*
* virtballoon_migratepage - perform the balloon page migration on behalf of
*a compation thread. (called under page lock)
- * @mapping: the page-mapping which will be assigned to the new migrated
page.
+ * @vb_dev_info: the balloon device
* @newpage: page that will replace the isolated page after migration
finishes.
* @page : the isolated (old) page that is about to be migrated to newpage.
* @mode : compaction mode -- not used for balloon page migration.
@@ -373,17 +372,13 @@ static const struct address_space_operations
virtio_balloon_aops;
* This function preforms the balloon page migration task.
* Called through balloon_mapping-a_ops-migratepage
*/
-static int virtballoon_migratepage(struct address_space *mapping,
+static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
struct page *newpage, struct page *page, enum migrate_mode mode)
{
- struct balloon_dev_info *vb_dev_info = balloon_page_device(page);
- struct virtio_balloon *vb;
+ struct virtio_balloon *vb = container_of(vb_dev_info,
+ struct virtio_balloon, vb_dev_info);
unsigned long flags;
- BUG_ON(!vb_dev_info);
-
- vb = vb_dev_info-balloon_device;
-
/*
* In order to avoid lock contention while migrating pages concurrently
* to leak_balloon() or fill_balloon() we just give up the balloon_lock
@@ -395,42 +390,34 @@ static int virtballoon_migratepage(struct address_space
*mapping,
if (!mutex_trylock(vb-balloon_lock))
return -EAGAIN;
+ get_page(newpage); /* balloon reference */
+
/* balloon's page migration 1st step -- inflate newpage */
spin_lock_irqsave(vb_dev_info-pages_lock, flags);
- balloon_page_insert(newpage, mapping, vb_dev_info-pages);
+ balloon_page_insert(vb_dev_info, newpage);
vb_dev_info-isolated_pages--;
spin_unlock_irqrestore(vb_dev_info-pages_lock, flags);
vb-num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
set_page_pfns(vb-pfns, newpage);
tell_host(vb, vb-inflate_vq);
- /*
- * balloon's page migration 2nd step -- deflate page
- *
- * It's safe to delete
