From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx118.postini.com [74.125.245.118]) by kanga.kvack.org (Postfix) with SMTP id 5A2806B002B for ; Tue, 14 Aug 2012 13:44:27 -0400 (EDT) Date: Tue, 14 Aug 2012 14:44:05 -0300 From: Rafael Aquini Subject: Re: [PATCH v7 1/4] mm: introduce compaction and migration for virtio ballooned pages Message-ID: <20120814174404.GA13338@t510.redhat.com> References: <292b1b52e863a05b299f94bda69a61371011ac19.1344619987.git.aquini@redhat.com> <20120813082619.GE14081@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120813082619.GE14081@redhat.com> Sender: owner-linux-mm@kvack.org List-ID: To: "Michael S. Tsirkin" Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, Rusty Russell , Rik van Riel , Mel Gorman , Andi Kleen , Andrew Morton , Konrad Rzeszutek Wilk , Minchan Kim On Mon, Aug 13, 2012 at 11:26:19AM +0300, Michael S. Tsirkin wrote: > > +static inline bool movable_balloon_page(struct page *page) > > +{ > > + return (page->mapping && page->mapping == balloon_mapping); > > I am guessing this needs smp_read_barrier_depends, and maybe > ACCESS_ONCE ... > I'm curious about your guessing here. Could you ellaborate it further, please? > > +#else > > +static inline bool isolate_balloon_page(struct page *page) { return false; } > > +static inline void putback_balloon_page(struct page *page) { return false; } > > +static inline bool movable_balloon_page(struct page *page) { return false; } > > +#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE) && CONFIG_COMPACTION */ > > + > > This does mean that only one type of balloon is useable at a time. > I wonder whether using a flag in address_space structure instead > is possible ... This means we are only introducing this feature for virtio_balloon by now. Despite the flagging address_space stuff is something we surely can look in the future, I quite didn't get how we could be using two different types of balloon devices at the same time for the same system. Could you ellaborate it a little more, please? > > +/* __isolate_lru_page() counterpart for a ballooned page */ > > +bool isolate_balloon_page(struct page *page) > > +{ > > + if (WARN_ON(!movable_balloon_page(page))) > > Looks like this actually can happen if the page is leaked > between previous movable_balloon_page and here. > > > + return false; Yes, it surely can happen, and it does not harm to catch it here, print a warn and return. While testing it, I wasn't lucky to see this small window opening, though. -- 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: email@kvack.org