linux-kernel - Re: [PATCH v4 1/3] mm: introduce compaction and migration for virtio ballooned pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120718230706.GB2313@t510.redhat.com>
Date:	Wed, 18 Jul 2012 20:07:07 -0300
From:	Rafael Aquini <aquini@...hat.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	virtualization@...ts.linux-foundation.org,
	Rusty Russell <rusty@...tcorp.com.au>,
	"Michael S. Tsirkin" <mst@...hat.com>,
	Rik van Riel <riel@...hat.com>, Mel Gorman <mel@....ul.ie>,
	Andi Kleen <andi@...stfloor.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Minchan Kim <minchan@...nel.org>,
	Rafael Aquini <aquini@...ux.com>
Subject: Re: [PATCH v4 1/3] mm: introduce compaction and migration for virtio
 ballooned pages

Howdy Andrew,

Thanks for taking the time to go through this work and provide me with such good
feedback.

On Wed, Jul 18, 2012 at 03:46:05PM -0700, Andrew Morton wrote:
> On Tue, 17 Jul 2012 13:50:41 -0300
> Rafael Aquini <aquini@...hat.com> wrote:
> 
> > This patch introduces the helper functions as well as the necessary changes
> > to teach compaction and migration bits how to cope with pages which are
> > part of a guest memory balloon, in order to make them movable by memory
> > compaction procedures.
> > 
> > ...
> >
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -1629,5 +1629,20 @@ static inline unsigned int debug_guardpage_minorder(void) { return 0; }
> >  static inline bool page_is_guard(struct page *page) { return false; }
> >  #endif /* CONFIG_DEBUG_PAGEALLOC */
> >  
> > +#if (defined(CONFIG_VIRTIO_BALLOON) || \
> > +	defined(CONFIG_VIRTIO_BALLOON_MODULE)) && defined(CONFIG_COMPACTION)
> > +extern bool putback_balloon_page(struct page *);
> > +extern struct address_space *balloon_mapping;
> > +
> > +static inline bool is_balloon_page(struct page *page)
> > +{
> > +	return (page->mapping == balloon_mapping) ? true : false;
> 
> You can simply do
> 
> 	return page->mapping == balloon_mapping;

Yes, I will do 
   return (page->mapping && page->mapping == balloon_mapping);

actually. I just got a case of NULL pointer deref while running on bare-metal
with no balloon driver loaded.

> 
> > +}
> > +#else
> > +static inline bool is_balloon_page(struct page *page)       { return false; }
> > +static inline bool isolate_balloon_page(struct page *page)  { return false; }
> > +static inline bool putback_balloon_page(struct page *page)  { return false; }
> > +#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE) && COMPACTION */
> 
> This means that if CONFIG_VIRTIO_BALLOON=y and CONFIG_COMPACTION=n,
> is_balloon_page() will always return NULL.  IOW, no pages are balloon
> pages!  This is wrong.
> 
I believe it's right, actually, as we can see CONFIG_COMPACTION=n associated with
CONFIG_MIGRATION=y (and  CONFIG_VIRTIO_BALLOON=y).
For such config case we cannot perform the is_balloon_page() test branches
placed on mm/migration.c


> I'm not sure what to do about this, apart from renaming the function to
> is_compactible_balloon_page() or something similarly aawkward.
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/