xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Ian Campbell <ian.campbell@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Kevin Tian <kevin.tian@intel.com>, Wei Liu <wei.liu2@citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Jun Nakajima <jun.nakajima@intel.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Keir Fraser <keir@xen.org>
Subject: Re: [PATCH v3] x86/p2m: use large pages for MMIO mappings
Date: Fri, 15 Jan 2016 13:57:02 +0000	[thread overview]
Message-ID: <1452866222.6020.8.camel@citrix.com> (raw)
In-Reply-To: <5698DC6502000078000C72F9@prv-mh.provo.novell.com>

On Fri, 2016-01-15 at 03:47 -0700, Jan Beulich wrote:
> > > > On 15.01.16 at 11:09, <ian.campbell@citrix.com> wrote:
> > On Thu, 2016-01-14 at 03:04 -0700, Jan Beulich wrote:
> > > - ARM side unimplemented (and hence libxc for now made cope with both
> > >   models),
> > 
> > So, one model is the one described in the commit message:
> > 
> > > - zero (success, everything done)
> > > - positive (success, this many done, more to do: re-invoke)
> > > - negative (error)
> > 
> > What is the other one? I'd expect ARM to already implement a subset of
> > this
> > (i.e. 0 or negative, perhaps with a subset of the possible errno
> > values), 
> > which I'd then expect libxc to just cope with without it constituting a
> > second model.
> 
> Well, first of all ARM doesn't get switched away from the current
> model (yet), i.e. returning -E2BIG out of do_domctl().

Which AFAICT is a valid behaviour under the new model described in the
commit message specifically the "negative (error)" case.

I think the core of my objection/confusion here is describing this as two
different models when what is being introduced is a single API which can
fail either partially or entirely, with that being at the discretion of the
internals. In any case libxc needs to cope with the complete gamut of
behaviours of the interface.

IOW rather than describing a new API and referring obliquely to ARM only
supporting an old model I think this needs a complete description of the
interface covering the full possibilities of the API.

>  And then
> the difference between what the patch implements and what the
> non-commit message comment describes is how errors get handled:
> The patch makes a negative error value returned upon error, with
> the caller having no way to tell at what point the error occurred
> (and with a best effort undo in the case of "map"). The described
> alternative would return the number of succeeded entries unless
> an error occurred on the very first MFN, without any attempt to
> undo the part that was done successfully. I.e. it would leave it
> to the caller to decide what to do, and whether/when to roll back.

That's (probably, I don't quite follow all the details as written) fine,
but the interface should be described as a single API with the possible
failure cases each spelled out, i.e. not described as a split/contrast
between old vs. new or x86 vs. arm behaviour. The fact that x86 and arm
might currently exhibit different subsets of the possibilities offered by
the API is of at best secondary interest.

Ian.

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

  reply	other threads:[~2016-01-15 13:57 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-14 10:04 [PATCH v3] x86/p2m: use large pages for MMIO mappings Jan Beulich
2016-01-15 10:09 ` Ian Campbell
2016-01-15 10:47   ` Jan Beulich
2016-01-15 13:57     ` Ian Campbell [this message]
2016-01-15 14:39       ` Jan Beulich
2016-01-15 14:55         ` Ian Campbell
2016-01-18  8:11           ` Jan Beulich
2016-01-18 16:32             ` Ian Campbell
2016-01-18 16:51               ` Jan Beulich
2016-01-18 17:00                 ` Ian Campbell

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=1452866222.6020.8.camel@citrix.com \
    --to=ian.campbell@citrix.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=JBeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=jun.nakajima@intel.com \
    --cc=keir@xen.org \
    --cc=kevin.tian@intel.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=tim@xen.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).