From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH v2 5/8] tmem: Move TMEM_CONTROL subop of tmem hypercall to sysctl. Date: Thu, 27 Aug 2015 15:20:21 -0400 Message-ID: <20150827192021.GK5982@konrad-lan.dumpdata.com> References: <1440673323-6648-1-git-send-email-konrad.wilk@oracle.com> <1440673323-6648-6-git-send-email-konrad.wilk@oracle.com> <55DF0CC3.9070200@citrix.com> <20150827184731.GG5982@konrad-lan.dumpdata.com> <55DF5DDB.5000909@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZV2is-0005ua-Oz for xen-devel@lists.xenproject.org; Thu, 27 Aug 2015 19:20:30 +0000 Content-Disposition: inline In-Reply-To: <55DF5DDB.5000909@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper Cc: xen-devel@lists.xenproject.org, wei.liu2@citrix.com, jbeulich@suse.com List-Id: xen-devel@lists.xenproject.org > >>> +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. Right. > > 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 Yes. I shall post this patchset plus extra patches next week. > > >>> + 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. It will break the tmem hypervisor code as it has a check for TMEM_CONTROL (which per Jan suggestion should return -E.. something). > > A comment however will be useful in all cases. Aye.