| Message ID | 20190725051125.10605-1-himadri18.07@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | hv_sock: use HV_HYP_PAGE_SIZE instead of PAGE_SIZE_4K | expand |
From: Himadri Pandya <himadrispandya@gmail.com> Date: Thu, 25 Jul 2019 05:11:25 +0000 > Older windows hosts require the hv_sock ring buffer to be defined > using 4K pages. This was achieved by using the symbol PAGE_SIZE_4K > defined specifically for this purpose. But now we have a new symbol > HV_HYP_PAGE_SIZE defined in hyperv-tlfs which can be used for this. > > This patch removes the definition of symbol PAGE_SIZE_4K and replaces > its usage with the symbol HV_HYP_PAGE_SIZE. This patch also aligns > sndbuf and rcvbuf to hyper-v specific page size using HV_HYP_PAGE_SIZE > instead of the guest page size(PAGE_SIZE) as hyper-v expects the page > size to be 4K and it might not be the case on ARM64 architecture. > > Signed-off-by: Himadri Pandya <himadri18.07@gmail.com> This doesn't compile: CC [M] net/vmw_vsock/hyperv_transport.o net/vmw_vsock/hyperv_transport.c:58:28: error: ‘HV_HYP_PAGE_SIZE’ undeclared here (not in a function); did you mean ‘HV_MESSAGE_SIZE’? #define HVS_SEND_BUF_SIZE (HV_HYP_PAGE_SIZE - sizeof(struct vmpipe_proto_header)) ^~~~~~~~~~~~~~~~
On 7/27/2019 10:50 AM, kbuild test robot wrote: > Hi Himadri, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on linus/master] > [cannot apply to v5.3-rc1 next-20190726] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] This patch should be applied to linux-next git tree. Thank you. - Himadri > > url: https://github.com/0day-ci/linux/commits/Himadri-Pandya/hv_sock-use-HV_HYP_PAGE_SIZE-instead-of-PAGE_SIZE_4K/20190726-085229 > config: x86_64-allyesconfig (attached as .config) > compiler: gcc-7 (Debian 7.4.0-10) 7.4.0 > reproduce: > # save the attached .config to linux build tree > make ARCH=x86_64 > > If you fix the issue, kindly add following tag > Reported-by: kbuild test robot <lkp@intel.com> > > All error/warnings (new ones prefixed by >>): > >>> net/vmw_vsock/hyperv_transport.c:58:28: error: 'HV_HYP_PAGE_SIZE' undeclared here (not in a function); did you mean 'HV_MESSAGE_SIZE'? > #define HVS_SEND_BUF_SIZE (HV_HYP_PAGE_SIZE - sizeof(struct vmpipe_proto_header)) > ^ >>> net/vmw_vsock/hyperv_transport.c:65:10: note: in expansion of macro 'HVS_SEND_BUF_SIZE' > u8 data[HVS_SEND_BUF_SIZE]; > ^~~~~~~~~~~~~~~~~ > In file included from include/linux/list.h:9:0, > from include/linux/module.h:9, > from net/vmw_vsock/hyperv_transport.c:11: > net/vmw_vsock/hyperv_transport.c: In function 'hvs_open_connection': >>> include/linux/kernel.h:845:2: error: first argument to '__builtin_choose_expr' not a constant > __builtin_choose_expr(__safe_cmp(x, y), \ > ^ > include/linux/kernel.h:921:27: note: in expansion of macro '__careful_cmp' > #define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >) > ^~~~~~~~~~~~~ >>> net/vmw_vsock/hyperv_transport.c:390:12: note: in expansion of macro 'max_t' > sndbuf = max_t(int, sk->sk_sndbuf, RINGBUFFER_HVS_SND_SIZE); > ^~~~~ >>> include/linux/kernel.h:845:2: error: first argument to '__builtin_choose_expr' not a constant > __builtin_choose_expr(__safe_cmp(x, y), \ > ^ > include/linux/kernel.h:913:27: note: in expansion of macro '__careful_cmp' > #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <) > ^~~~~~~~~~~~~ >>> net/vmw_vsock/hyperv_transport.c:391:12: note: in expansion of macro 'min_t' > sndbuf = min_t(int, sndbuf, RINGBUFFER_HVS_MAX_SIZE); > ^~~~~ >>> include/linux/kernel.h:845:2: error: first argument to '__builtin_choose_expr' not a constant > __builtin_choose_expr(__safe_cmp(x, y), \ > ^ > include/linux/kernel.h:921:27: note: in expansion of macro '__careful_cmp' > #define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >) > ^~~~~~~~~~~~~ > net/vmw_vsock/hyperv_transport.c:393:12: note: in expansion of macro 'max_t' > rcvbuf = max_t(int, sk->sk_rcvbuf, RINGBUFFER_HVS_RCV_SIZE); > ^~~~~ >>> include/linux/kernel.h:845:2: error: first argument to '__builtin_choose_expr' not a constant > __builtin_choose_expr(__safe_cmp(x, y), \ > ^ > include/linux/kernel.h:913:27: note: in expansion of macro '__careful_cmp' > #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <) > ^~~~~~~~~~~~~ > net/vmw_vsock/hyperv_transport.c:394:12: note: in expansion of macro 'min_t' > rcvbuf = min_t(int, rcvbuf, RINGBUFFER_HVS_MAX_SIZE); > ^~~~~ > net/vmw_vsock/hyperv_transport.c: In function 'hvs_stream_enqueue': >>> include/linux/kernel.h:845:2: error: first argument to '__builtin_choose_expr' not a constant > __builtin_choose_expr(__safe_cmp(x, y), \ > ^ > include/linux/kernel.h:913:27: note: in expansion of macro '__careful_cmp' > #define min_t(type, x, y) __careful_cmp((type)(x), (type)(y), <) > ^~~~~~~~~~~~~ > net/vmw_vsock/hyperv_transport.c:681:14: note: in expansion of macro 'min_t' > to_write = min_t(ssize_t, to_write, HVS_SEND_BUF_SIZE); > ^~~~~ > > vim +58 net/vmw_vsock/hyperv_transport.c > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Wed, Jul 31, 2019 at 01:02:03AM +0000, Michael Kelley wrote: >From: Himadri Pandya <himadrispandya@gmail.com> Sent: Wednesday, July 24, 2019 10:11 PM >> >> Older windows hosts require the hv_sock ring buffer to be defined >> using 4K pages. This was achieved by using the symbol PAGE_SIZE_4K >> defined specifically for this purpose. But now we have a new symbol >> HV_HYP_PAGE_SIZE defined in hyperv-tlfs which can be used for this. >> >> This patch removes the definition of symbol PAGE_SIZE_4K and replaces >> its usage with the symbol HV_HYP_PAGE_SIZE. This patch also aligns >> sndbuf and rcvbuf to hyper-v specific page size using HV_HYP_PAGE_SIZE >> instead of the guest page size(PAGE_SIZE) as hyper-v expects the page >> size to be 4K and it might not be the case on ARM64 architecture. >> >> Signed-off-by: Himadri Pandya <himadri18.07@gmail.com> >> --- >> net/vmw_vsock/hyperv_transport.c | 21 +++++++++++---------- >> 1 file changed, 11 insertions(+), 10 deletions(-) >> >> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c >> index f2084e3f7aa4..ecb5d72d8010 100644 >> --- a/net/vmw_vsock/hyperv_transport.c >> +++ b/net/vmw_vsock/hyperv_transport.c >> @@ -13,15 +13,16 @@ >> #include <linux/hyperv.h> >> #include <net/sock.h> >> #include <net/af_vsock.h> >> +#include <asm/hyperv-tlfs.h> >> > >Reviewed-by: Michael Kelley <mikelley@microsoft.com> > >This patch depends on a prerequisite patch in > > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/hyperv > >that defines HV_HYP_PAGE_SIZE. David, the above prerequisite patch is now upstream, so this patch should be good to go. Would you take it through the net tree or should I do it via the hyperv tree? -- Thanks, Sasha
On Fri, Oct 04, 2019 at 11:48:17AM -0400, Sasha Levin wrote: >On Wed, Jul 31, 2019 at 01:02:03AM +0000, Michael Kelley wrote: >>From: Himadri Pandya <himadrispandya@gmail.com> Sent: Wednesday, July 24, 2019 10:11 PM >>> >>>Older windows hosts require the hv_sock ring buffer to be defined >>>using 4K pages. This was achieved by using the symbol PAGE_SIZE_4K >>>defined specifically for this purpose. But now we have a new symbol >>>HV_HYP_PAGE_SIZE defined in hyperv-tlfs which can be used for this. >>> >>>This patch removes the definition of symbol PAGE_SIZE_4K and replaces >>>its usage with the symbol HV_HYP_PAGE_SIZE. This patch also aligns >>>sndbuf and rcvbuf to hyper-v specific page size using HV_HYP_PAGE_SIZE >>>instead of the guest page size(PAGE_SIZE) as hyper-v expects the page >>>size to be 4K and it might not be the case on ARM64 architecture. >>> >>>Signed-off-by: Himadri Pandya <himadri18.07@gmail.com> >>>--- >>> net/vmw_vsock/hyperv_transport.c | 21 +++++++++++---------- >>> 1 file changed, 11 insertions(+), 10 deletions(-) >>> >>>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c >>>index f2084e3f7aa4..ecb5d72d8010 100644 >>>--- a/net/vmw_vsock/hyperv_transport.c >>>+++ b/net/vmw_vsock/hyperv_transport.c >>>@@ -13,15 +13,16 @@ >>> #include <linux/hyperv.h> >>> #include <net/sock.h> >>> #include <net/af_vsock.h> >>>+#include <asm/hyperv-tlfs.h> >>> >> >>Reviewed-by: Michael Kelley <mikelley@microsoft.com> >> >>This patch depends on a prerequisite patch in >> >> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/hyperv >> >>that defines HV_HYP_PAGE_SIZE. > >David, the above prerequisite patch is now upstream, so this patch >should be good to go. Would you take it through the net tree or should I >do it via the hyperv tree? Ping?
On Thu, 10 Oct 2019 13:06:06 -0400, Sasha Levin wrote: > On Fri, Oct 04, 2019 at 11:48:17AM -0400, Sasha Levin wrote: > >On Wed, Jul 31, 2019 at 01:02:03AM +0000, Michael Kelley wrote: > >>From: Himadri Pandya <himadrispandya@gmail.com> Sent: Wednesday, July 24, 2019 10:11 PM > >>> > >>>Older windows hosts require the hv_sock ring buffer to be defined > >>>using 4K pages. This was achieved by using the symbol PAGE_SIZE_4K > >>>defined specifically for this purpose. But now we have a new symbol > >>>HV_HYP_PAGE_SIZE defined in hyperv-tlfs which can be used for this. > >>> > >>>This patch removes the definition of symbol PAGE_SIZE_4K and replaces > >>>its usage with the symbol HV_HYP_PAGE_SIZE. This patch also aligns > >>>sndbuf and rcvbuf to hyper-v specific page size using HV_HYP_PAGE_SIZE > >>>instead of the guest page size(PAGE_SIZE) as hyper-v expects the page > >>>size to be 4K and it might not be the case on ARM64 architecture. > >>> > >>>Signed-off-by: Himadri Pandya <himadri18.07@gmail.com> > >>>--- > >>> net/vmw_vsock/hyperv_transport.c | 21 +++++++++++---------- > >>> 1 file changed, 11 insertions(+), 10 deletions(-) > >>> > >>>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c > >>>index f2084e3f7aa4..ecb5d72d8010 100644 > >>>--- a/net/vmw_vsock/hyperv_transport.c > >>>+++ b/net/vmw_vsock/hyperv_transport.c > >>>@@ -13,15 +13,16 @@ > >>> #include <linux/hyperv.h> > >>> #include <net/sock.h> > >>> #include <net/af_vsock.h> > >>>+#include <asm/hyperv-tlfs.h> > >>> > >> > >>Reviewed-by: Michael Kelley <mikelley@microsoft.com> > >> > >>This patch depends on a prerequisite patch in > >> > >> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/hyperv > >> > >>that defines HV_HYP_PAGE_SIZE. > > > >David, the above prerequisite patch is now upstream, so this patch > >should be good to go. Would you take it through the net tree or should I > >do it via the hyperv tree? > > Ping? Is this a fix? It's slightly unclear from the description of the patch. I think the best course of action would be reposting it again, with either [PATCH net] in the subject and a Fixes tag if it's a fix, or [PATCH net-next] otherwise.
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c index f2084e3f7aa4..ecb5d72d8010 100644 --- a/net/vmw_vsock/hyperv_transport.c +++ b/net/vmw_vsock/hyperv_transport.c @@ -13,15 +13,16 @@ #include <linux/hyperv.h> #include <net/sock.h> #include <net/af_vsock.h> +#include <asm/hyperv-tlfs.h> /* Older (VMBUS version 'VERSION_WIN10' or before) Windows hosts have some - * stricter requirements on the hv_sock ring buffer size of six 4K pages. Newer - * hosts don't have this limitation; but, keep the defaults the same for compat. + * stricter requirements on the hv_sock ring buffer size of six 4K pages. + * hyperv-tlfs defines HV_HYP_PAGE_SIZE as 4K. Newer hosts don't have this + * limitation; but, keep the defaults the same for compat. */ -#define PAGE_SIZE_4K 4096 -#define RINGBUFFER_HVS_RCV_SIZE (PAGE_SIZE_4K * 6) -#define RINGBUFFER_HVS_SND_SIZE (PAGE_SIZE_4K * 6) -#define RINGBUFFER_HVS_MAX_SIZE (PAGE_SIZE_4K * 64) +#define RINGBUFFER_HVS_RCV_SIZE (HV_HYP_PAGE_SIZE * 6) +#define RINGBUFFER_HVS_SND_SIZE (HV_HYP_PAGE_SIZE * 6) +#define RINGBUFFER_HVS_MAX_SIZE (HV_HYP_PAGE_SIZE * 64) /* The MTU is 16KB per the host side's design */ #define HVS_MTU_SIZE (1024 * 16) @@ -54,7 +55,7 @@ struct hvs_recv_buf { * ringbuffer APIs that allow us to directly copy data from userspace buffer * to VMBus ringbuffer. */ -#define HVS_SEND_BUF_SIZE (PAGE_SIZE_4K - sizeof(struct vmpipe_proto_header)) +#define HVS_SEND_BUF_SIZE (HV_HYP_PAGE_SIZE - sizeof(struct vmpipe_proto_header)) struct hvs_send_buf { /* The header before the payload data */ @@ -388,10 +389,10 @@ static void hvs_open_connection(struct vmbus_channel *chan) } else { sndbuf = max_t(int, sk->sk_sndbuf, RINGBUFFER_HVS_SND_SIZE); sndbuf = min_t(int, sndbuf, RINGBUFFER_HVS_MAX_SIZE); - sndbuf = ALIGN(sndbuf, PAGE_SIZE); + sndbuf = ALIGN(sndbuf, HV_HYP_PAGE_SIZE); rcvbuf = max_t(int, sk->sk_rcvbuf, RINGBUFFER_HVS_RCV_SIZE); rcvbuf = min_t(int, rcvbuf, RINGBUFFER_HVS_MAX_SIZE); - rcvbuf = ALIGN(rcvbuf, PAGE_SIZE); + rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE); } ret = vmbus_open(chan, sndbuf, rcvbuf, NULL, 0, hvs_channel_cb, @@ -662,7 +663,7 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg, ssize_t ret = 0; ssize_t bytes_written = 0; - BUILD_BUG_ON(sizeof(*send_buf) != PAGE_SIZE_4K); + BUILD_BUG_ON(sizeof(*send_buf) != HV_HYP_PAGE_SIZE); send_buf = kmalloc(sizeof(*send_buf), GFP_KERNEL); if (!send_buf)
Older windows hosts require the hv_sock ring buffer to be defined using 4K pages. This was achieved by using the symbol PAGE_SIZE_4K defined specifically for this purpose. But now we have a new symbol HV_HYP_PAGE_SIZE defined in hyperv-tlfs which can be used for this. This patch removes the definition of symbol PAGE_SIZE_4K and replaces its usage with the symbol HV_HYP_PAGE_SIZE. This patch also aligns sndbuf and rcvbuf to hyper-v specific page size using HV_HYP_PAGE_SIZE instead of the guest page size(PAGE_SIZE) as hyper-v expects the page size to be 4K and it might not be the case on ARM64 architecture. Signed-off-by: Himadri Pandya <himadri18.07@gmail.com> --- net/vmw_vsock/hyperv_transport.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)