|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 5/8] tmem: Move TMEM_CONTROL subop of tmem hypercall to sysctl.
On 27/08/15 19:47, Konrad Rzeszutek Wilk wrote:
>
>>> --- a/xen/include/public/sysctl.h
>>> +++ b/xen/include/public/sysctl.h
>>> @@ -710,6 +710,48 @@ struct xen_sysctl_psr_cat_op {
>>> typedef struct xen_sysctl_psr_cat_op xen_sysctl_psr_cat_op_t;
>>> DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
>>>
>>> +#define XEN_SYSCTL_TMEM_OP_ALL_CLIENTS 0xFFFFU
>>> +
>>> +#define XEN_SYSCTL_TMEM_OP_THAW 0
>>> +#define XEN_SYSCTL_TMEM_OP_FREEZE 1
>>> +#define XEN_SYSCTL_TMEM_OP_FLUSH 2
>>> +#define XEN_SYSCTL_TMEM_OP_DESTROY 3
>>> +#define XEN_SYSCTL_TMEM_OP_LIST 4
>>> +#define XEN_SYSCTL_TMEM_OP_SET_WEIGHT 5
>>> +#define XEN_SYSCTL_TMEM_OP_SET_CAP 6
>>> +#define XEN_SYSCTL_TMEM_OP_SET_COMPRESS 7
>>> +#define XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB 8
>>> +#define XEN_SYSCTL_TMEM_OP_SAVE_BEGIN 10
>>> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_VERSION 11
>>> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_MAXPOOLS 12
>>> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_WEIGHT 13
>>> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_CAP 14
>>> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_CLIENT_FLAGS 15
>>> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_FLAGS 16
>>> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_NPAGES 17
>>> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_POOL_UUID 18
>>> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_PAGE 19
>>> +#define XEN_SYSCTL_TMEM_OP_SAVE_GET_NEXT_INV 20
>>> +#define XEN_SYSCTL_TMEM_OP_SAVE_END 21
>>> +#define XEN_SYSCTL_TMEM_OP_RESTORE_BEGIN 30
>>> +#define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE 32
>>> +#define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE 33
>>> +
>>> +struct xen_sysctl_tmem_op {
>>> + uint32_t cmd; /* IN: XEN_SYSCTL_TMEM_OP_* . */
>>> + int32_t pool_id; /* IN: 0 by default unless _SAVE_*, RESTORE_* .*/
>>> + uint32_t cli_id; /* IN: client id, 0 for
>>> XEN_SYSCTL_TMEM_QUERY_FREEABLE_MB
>>> + for all others can be the domain id or
>>> + XEN_SYSCTL_TMEM_OP_ALL_CLIENTS for all. */
>>> + uint32_t arg1; /* IN: If not applicable to command use 0. */
>>> + uint32_t arg2; /* IN: If not applicable to command use 0. */
>> Please can this interface be fixed as part of the move, even if it is in
>> subsequent patches for clarity.
> I will gladly fix this interface in further patches. By all means!
What I wish to avoid is 4.6 releasing with the API in this state, as
adjusting valgrind and strace to compensate will be miserable.
The best solution would be to have this patch and the fixups adjacent in
the series, at which point the valgrind/strace adjustments can start
with the clean API for 4.6
>>> + uint64_t oid[3]; /* IN: If not applicable to command use 0. */
>>> + XEN_GUEST_HANDLE_64(char) buf; /* IN/OUT: Buffer to save and restore
>>> ops. */
>>> +};
>>> +typedef struct xen_sysctl_tmem_op xen_sysctl_tmem_op_t;
>>> +DEFINE_XEN_GUEST_HANDLE(xen_sysctl_tmem_op_t);
>>> +
>>> struct xen_sysctl {
>>> uint32_t cmd;
>>> #define XEN_SYSCTL_readconsole 1
>>> @@ -734,6 +776,7 @@ struct xen_sysctl {
>>> #define XEN_SYSCTL_psr_cmt_op 21
>>> #define XEN_SYSCTL_pcitopoinfo 22
>>> #define XEN_SYSCTL_psr_cat_op 23
>>> +#define XEN_SYSCTL_tmem_op 24
>>> uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
>>> union {
>>> struct xen_sysctl_readconsole readconsole;
>>> @@ -758,6 +801,7 @@ struct xen_sysctl {
>>> struct xen_sysctl_coverage_op coverage_op;
>>> struct xen_sysctl_psr_cmt_op psr_cmt_op;
>>> struct xen_sysctl_psr_cat_op psr_cat_op;
>>> + struct xen_sysctl_tmem_op tmem_op;
>>> uint8_t pad[128];
>>> } u;
>>> };
>>> diff --git a/xen/include/public/tmem.h b/xen/include/public/tmem.h
>>> index 4fd2fc6..208f5a6 100644
>>> --- a/xen/include/public/tmem.h
>>> +++ b/xen/include/public/tmem.h
>>> @@ -33,7 +33,7 @@
>>> #define TMEM_SPEC_VERSION 1
>>>
>>> /* Commands to HYPERVISOR_tmem_op() */
>>> -#define TMEM_CONTROL 0
>>> +#define TMEM_CONTROL_MOVED 0
>> If anything, this should be deleted as opposed to being renamed. The
>> _MOVED suffix is confusing, as TMEM_CONTROL_MOVED sounds like it could
>> be a plausible TMEM operation.
> Ah, will do what Jan asked and just put a big comment. And keep the old
> name (or maybe add 'DEPRECATED'?)
If you are going to the effort of renaming, then just delete. Either
way will break the build of unsuspecting code, and the latter leaves us
with less junk.
A comment however will be useful in all cases.
>>> #define TMEM_NEW_POOL 1
>>> #define TMEM_DESTROY_POOL 2
>>> #define TMEM_PUT_PAGE 4
>>> @@ -48,35 +48,9 @@
>>> #endif
>>>
>>> /* Privileged commands to HYPERVISOR_tmem_op() */
>>> -#define TMEM_AUTH 101
>>> +#define TMEM_AUTH 101
>>> #define TMEM_RESTORE_NEW 102
>>>
>>> -/* Subops for HYPERVISOR_tmem_op(TMEM_CONTROL) */
>>> -#define TMEMC_THAW 0
>>> -#define TMEMC_FREEZE 1
>>> -#define TMEMC_FLUSH 2
>>> -#define TMEMC_DESTROY 3
>>> -#define TMEMC_LIST 4
>>> -#define TMEMC_SET_WEIGHT 5
>>> -#define TMEMC_SET_CAP 6
>>> -#define TMEMC_SET_COMPRESS 7
>>> -#define TMEMC_QUERY_FREEABLE_MB 8
>>> -#define TMEMC_SAVE_BEGIN 10
>>> -#define TMEMC_SAVE_GET_VERSION 11
>>> -#define TMEMC_SAVE_GET_MAXPOOLS 12
>>> -#define TMEMC_SAVE_GET_CLIENT_WEIGHT 13
>>> -#define TMEMC_SAVE_GET_CLIENT_CAP 14
>>> -#define TMEMC_SAVE_GET_CLIENT_FLAGS 15
>>> -#define TMEMC_SAVE_GET_POOL_FLAGS 16
>>> -#define TMEMC_SAVE_GET_POOL_NPAGES 17
>>> -#define TMEMC_SAVE_GET_POOL_UUID 18
>>> -#define TMEMC_SAVE_GET_NEXT_PAGE 19
>>> -#define TMEMC_SAVE_GET_NEXT_INV 20
>>> -#define TMEMC_SAVE_END 21
>>> -#define TMEMC_RESTORE_BEGIN 30
>>> -#define TMEMC_RESTORE_PUT_PAGE 32
>>> -#define TMEMC_RESTORE_FLUSH_PAGE 33
>>> -
>>> /* Bits for HYPERVISOR_tmem_op(TMEM_NEW_POOL) */
>>> #define TMEM_POOL_PERSIST 1
>>> #define TMEM_POOL_SHARED 2
>>> @@ -110,16 +84,7 @@ struct tmem_op {
>>> uint32_t flags;
>>> uint32_t arg1;
>>> } creat; /* for cmd == TMEM_NEW_POOL, TMEM_AUTH, TMEM_RESTORE_NEW
>>> */
>>> - struct {
>>> - uint32_t subop;
>>> - uint32_t cli_id;
>>> - uint32_t arg1;
>>> - uint32_t arg2;
>>> - uint64_t oid[3];
>>> - tmem_cli_va_t buf;
>>> - } ctrl; /* for cmd == TMEM_CONTROL */
>>> struct {
>>> -
>>> uint64_t oid[3];
>>> uint32_t index;
>>> uint32_t tmem_offset;
>>> diff --git a/xen/include/xen/tmem.h b/xen/include/xen/tmem.h
>>> index 5dbf9d5..32a542a 100644
>>> --- a/xen/include/xen/tmem.h
>>> +++ b/xen/include/xen/tmem.h
>>> @@ -9,6 +9,9 @@
>>> #ifndef __XEN_TMEM_H__
>>> #define __XEN_TMEM_H__
>>>
>>> +struct xen_sysctl_tmem_op;
>> #include<public/sysctl.h> please.
> I tried that and it blew up with tons of other dependencies. This makes
> it much simpler without adding extra #include.
Fair enough.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |