All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>,
	Julien Grall <julien.grall@arm.com>,
	Stefano Stabellini <stefano.stabellini@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>,
	Tim Deegan <tim@xen.org>, Xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH v3] xen/mm: Fix page_list_* helpers to evaluate all their arguments
Date: Tue, 8 Mar 2016 11:05:00 +0000	[thread overview]
Message-ID: <56DEB1DC.2070006@citrix.com> (raw)
In-Reply-To: <56DEB01A02000078000DA47E@prv-mh.provo.novell.com>

On 08/03/16 09:57, Jan Beulich wrote:
>>>> On 07.03.16 at 19:12, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/include/asm-arm/domain.h
>> +++ b/xen/include/asm-arm/domain.h
>> @@ -65,6 +65,7 @@ struct arch_domain
>>          RELMEM_mapping,
>>          RELMEM_done,
>>      } relmem;
>> +    struct page_list_head relmem_list;
> Well, if I was ARM maintainer I would say no to this otherwise pointless
> addition (even more so that this list doesn't get initialized anywhere).
> The expectation I had for how the build issue would be fixed was to
> simply not convert (at least) page_list_del2() to an inline function.

No.  Discarding parameters is what got us into the first mess.  I will
not propagate the problem.

It is a bug that ARM relied on the discarded parameters to compile.

Ultimately, the bug is that common/page_alloc.c references d->arch. 
More generally, the problem is that common/page_alloc.c has x86
specifics in it.

The two options are to make relmem_list common, or to remove x86
specifics from common by introduce arch_free_domheap_page() helpers
which maintain relmem_list on x86.  On second thoughts, this latter
option seems to be better, as it would also allow the removal of
page_list_del2, although it is not clear if this is safe to do.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

  reply	other threads:[~2016-03-08 11:05 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-05 16:52 [PATCH] xen/mm: Fix page_list_* helpers to evaluate all their arguments Andrew Cooper
2016-03-05 23:42 ` Doug Goldstein
2016-03-07 13:15 ` Jan Beulich
2016-03-07 15:01   ` [PATCH v2] " Andrew Cooper
2016-03-07 16:53     ` Jan Beulich
2016-03-07 18:12       ` [PATCH v3] " Andrew Cooper
2016-03-08  9:57         ` Jan Beulich
2016-03-08 11:05           ` Andrew Cooper [this message]
2016-03-08 11:25             ` Jan Beulich
2016-03-09 12:12         ` George Dunlap

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=56DEB1DC.2070006@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=george.dunlap@eu.citrix.com \
    --cc=julien.grall@arm.com \
    --cc=stefano.stabellini@citrix.com \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xen.org \
    /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: link
Be 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.