From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: Re: open-coded page list manipulation in shadow code Date: Tue, 27 Jan 2015 11:50:26 +0100 Message-ID: <20150127105026.GA38811@deinos.phlegethon.org> References: <54C67AC802000078000598CE@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YG3j3-0003Ho-6t for xen-devel@lists.xenproject.org; Tue, 27 Jan 2015 10:50:29 +0000 Content-Disposition: inline In-Reply-To: <54C67AC802000078000598CE@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel List-Id: xen-devel@lists.xenproject.org At 16:35 +0000 on 26 Jan (1422286504), Jan Beulich wrote: > Tim, > > in the course of adding >16Tb support to the hypervisor, I ran into > issues with you having added several open-coded page list accesses > while breaking up non-order-0 allocations there. I looked at that > code for quite a while, but wasn't really able to figure how to > properly convert these. In particular I'm struggling with the list > terminations (would using NULL for PAGE_LIST_NULL be correct > when using ordinary struct list_head-s for struct page_info's list > member?) Yes, I think so. The fragmentary lists would terminate in NULL in both directions. > and the implications of this comment > > /* Page lists don't have pointers back to the head structure, so > * it's safe to use a head structure on the stack to link the pages > * together. */ Yep; we'd have to make sure that we explicitly NULL'ed out the first page's prev and the last page's next. This all seems a bit unsatisfactory though. Maybe it would be better to piggy-back these partial lists into the hash-table's linked list (i.e. the 'next_shadow' field) instead of the pinned list. That list is much more active, though. Let me have a think about it on Thursday (assuming I don't have too many patch series to review by then!) > in shadow_alloc(), but then again I also didn't look very closely at > the other places that also fail to compile. > > To help myself test the new code, I invented a shadow stub which > provides just enough functionality to build and not crash with all > the other shadow code compiled out. I wonder whether such a > build mode might actually be useful for other purposes, and hence > whether it might be worthwhile extracting that from the patch into > a standalone one. If it's not too much work, yes please. There are already use cases where people don't need shadow pagetable support, and once PVH is stable that will be more common. Having the option to compile out 7kloc of twisty MMU code seems good. :) Tim.