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

Re: [Xen-devel] [PATCH v4 3/7] libxl: introduce a new structure to represent static shared memory regions

On 02/06/2018 04:06 PM, Zhongze Liu wrote:


2018-02-06 23:46 GMT+08:00 Julien Grall <julien.grall@xxxxxxx>:

On 02/06/2018 03:41 PM, Zhongze Liu wrote:

Thanks for reviewing.

2018-02-06 19:27 GMT+08:00 Julien Grall <julien.grall@xxxxxxx>:


On 01/30/2018 05:50 PM, Zhongze Liu wrote:

Add a new structure to the IDL familiy to represent static shared memory


+libxl_static_shm = Struct("static_shm", [
+    ("id", string),
+    ("offset", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}),
+    ("begin", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}),
+    ("end", uint64, {'init_val': 'LIBXL_SSHM_RANGE_UNKNOWN'}),

We might want to store the size rather than the end. This would allow us
cover region up to the address 2^64-1.

Also, this would make clearer whether end is included in the region or

I think making the range inclusive and documenting it would have the
same effect.
But I'm not sure which syntax is more friendly to the users. What do you

You would still run into some problem. Indeed LIBX_SSHM_RANGE_UNKNOWN is
defined as UINT64_MAX. So how would you differentiate them?

But saying inclusive, I was actually trying to say "including the page
that begins at @end", so

Which is quite confusing. Usually when I see a range, I assume that the end will be the actual end. Not PAGE_ALIGN(end).

the only possibility when  LIBXL_SSHM_RANGE_UNKNOWN would be a valid value for
@end is when the page granularity is 1byte, which, I think, is not
very likely to happen.

But soon I find this might lead to more confusion. Now I agree with
you that we should use
the begin/size syntax instead of the current one.
Yes, begin/size is straightforward. There are no way to lead to map one less (or extra) page by confusion.


Julien Grall

Xen-devel mailing list



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