[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 01/11] memory: batch processing in acquire_resource()


On 15/07/2020 13:28, Jan Beulich wrote:
May I ask that you drop the if() around this block of code?

Also, looking at this, I wonder whether it's a good idea to use the
"start extent" model here anyway: You could as well update the
struct (saying that it may be clobbered in the public header) and
copy the whole thing back to the original guest struct. This would
then remove the pretty arbitrary "UINT_MAX >> MEMOP_EXTENT_SHIFT"
limit you currently need to enforce. The main question is whether
we'd consider such an adjustment to an existing interface
acceptable; there's an at least theoretical risk that it may break
existing callers. Then again no existing caller can sensibly have
specified a count above 32, and when the copying back would be
limited to just the continuation case, no such caller would be
affected in any way afaict.

I can see two risks with this approach:
1) There is no requirement for the 'arg' buffer to be in read-write memory. 2) A guest may expect to re-use the value within the structure after the hypercalls.

So I think the "start extent" model is better. This is already used in other memory sub-hypercalls, so the risk to break existing users is more limited.


Julien Grall



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.