Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
On Wed, Dec 02, 2015 at 10:45:28AM -0800, Greg Kroah-Hartman wrote:
> On Wed, Dec 02, 2015 at 09:26:34AM -0800, Dmitry Torokhov wrote:
> > On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > > > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh
> > > > > > > wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > > >> > */
> > > > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > > >> > -({ \
> > > > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > > > >> > - "=a"(out1), \
> > > > > > > >> > - "=b"(out2), \
> > > > > > > >> > - "=c"(out3), \
> > > > > > > >> > - "=d"(out4), \
> > > > > > > >> > - "=S"(__dummy1), \
> > > > > > > >> > - "=D"(__dummy2) :\
> > > > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > > > >> > - "b"(in1), \
> > > > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > > > >> > - "memory"); \
> > > > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4)
> > > > > > > >> > \
> > > > > > > >> > +({
> > > > > > > >> > \
> > > > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0;
> > > > > > > >> > \
> > > > > > > >>
> > > > > > > >> Why do we need to initialize dummies?
> > > > > > > >
> > > > > > > > Because for some commands those parameters to VMW_PORT() can be
> > > > > > > > both
> > > > > > > > input and outout.
> > > > > > >
> > > > > > > The vmmouse commands do not use them as input though, so it seems
> > > > > > > we
> > > > > > > are simply wasting CPU cycles setting them to 0 just because we
> > > > > > > are
> > > > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > > > benefit of doing this?
> > > > > >
> > > > > > There are two reasons. One is to make the code more readable and
> > > > > > maintainable. Rather than having mostly similar inline assembly
> > > > > > code sprinkled across multiple modules, we can just use the macros
> > > > > > and document that.
> > > > >
> > > > > But the macro is only used here, and the variables aren't used at all,
> > > > > so it makes no sense in this file.
> > > >
> > > > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > > > sure what the proper distribution list is for each part.
> > >
> > > Use scripts/get_maintainer.pl, that's what it is there for. A number of
> > > those patches should go through me, if not all of them, if you want them
> > > merged...
> > >
> > > >
> > > > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > > > vmw_balloon.c
> > >
> > > And it's used inconsistantly in those patches (you don't set the dummy
> > > variables to 0 in all of them...) Now maybe that's just how the asm
> > > functions work, but it's not very obvious as to why this is at all.
> > >
> > > > > > The second reason is this organization makes some on-going future
> > > > > > development easier.
> > > > >
> > > > > We don't plan for "future" development other than a single patch
> > > > > series,
> > > > > as we have no idea what that development is, nor if it will really
> > > > > happen. You can always change this file later if you need to, nothing
> > > > > is keeping that from happening.
> > > >
> > > > So the intent of this series is to centralize similar lines of inline
> > > > assembly code that are currently used by 3 different kernel modules
> > > > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > > > to include for common guest-host communication needs.
> > >
> > > Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> > > create yet-another-.h-file for your bus? You already have 2, this would
> > > make it 3, which seems like a lot...
> >
> > Umm, you are not saying that vmmouse should include vmci header file(s),
> > are you? Because the 2 are unrelated and vmci does not use the
> > hypervisor port to communicate with host IIRC.
>
> vmmouse should include some type of "vmware bus" .h file, if it's not
> the vmw_
Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
On Wed, Dec 02, 2015 at 10:45:28AM -0800, Greg Kroah-Hartman wrote:
> On Wed, Dec 02, 2015 at 09:26:34AM -0800, Dmitry Torokhov wrote:
> > On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > > > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > > > Hi,
> > > > > >
> > > > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh
> > > > > > > wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > > >> > */
> > > > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > > >> > -({ \
> > > > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > > > >> > - "=a"(out1), \
> > > > > > > >> > - "=b"(out2), \
> > > > > > > >> > - "=c"(out3), \
> > > > > > > >> > - "=d"(out4), \
> > > > > > > >> > - "=S"(__dummy1), \
> > > > > > > >> > - "=D"(__dummy2) :\
> > > > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > > > >> > - "b"(in1), \
> > > > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > > > >> > - "memory"); \
> > > > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4)
> > > > > > > >> > \
> > > > > > > >> > +({
> > > > > > > >> > \
> > > > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0;
> > > > > > > >> > \
> > > > > > > >>
> > > > > > > >> Why do we need to initialize dummies?
> > > > > > > >
> > > > > > > > Because for some commands those parameters to VMW_PORT() can be
> > > > > > > > both
> > > > > > > > input and outout.
> > > > > > >
> > > > > > > The vmmouse commands do not use them as input though, so it seems
> > > > > > > we
> > > > > > > are simply wasting CPU cycles setting them to 0 just because we
> > > > > > > are
> > > > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > > > benefit of doing this?
> > > > > >
> > > > > > There are two reasons. One is to make the code more readable and
> > > > > > maintainable. Rather than having mostly similar inline assembly
> > > > > > code sprinkled across multiple modules, we can just use the macros
> > > > > > and document that.
> > > > >
> > > > > But the macro is only used here, and the variables aren't used at all,
> > > > > so it makes no sense in this file.
> > > >
> > > > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > > > sure what the proper distribution list is for each part.
> > >
> > > Use scripts/get_maintainer.pl, that's what it is there for. A number of
> > > those patches should go through me, if not all of them, if you want them
> > > merged...
> > >
> > > >
> > > > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > > > vmw_balloon.c
> > >
> > > And it's used inconsistantly in those patches (you don't set the dummy
> > > variables to 0 in all of them...) Now maybe that's just how the asm
> > > functions work, but it's not very obvious as to why this is at all.
> > >
> > > > > > The second reason is this organization makes some on-going future
> > > > > > development easier.
> > > > >
> > > > > We don't plan for "future" development other than a single patch
> > > > > series,
> > > > > as we have no idea what that development is, nor if it will really
> > > > > happen. You can always change this file later if you need to, nothing
> > > > > is keeping that from happening.
> > > >
> > > > So the intent of this series is to centralize similar lines of inline
> > > > assembly code that are currently used by 3 different kernel modules
> > > > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > > > to include for common guest-host communication needs.
> > >
> > > Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> > > create yet-another-.h-file for your bus? You already have 2, this would
> > > make it 3, which seems like a lot...
> >
> > Umm, you are not saying that vmmouse should include vmci header file(s),
> > are you? Because the 2 are unrelated and vmci does not use the
> > hypervisor port to communicate with host IIRC.
>
> vmmouse should include some type of "vmware bus" .h file, if it's not
> the vmw_
Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
On Wed, Dec 02, 2015 at 09:26:34AM -0800, Dmitry Torokhov wrote:
> On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> > On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > > Hi,
> > > > >
> > > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh
> > > > > > wrote:
> > > > > > > Hi,
> > > > > > >
> > > > >
> > > > >
> > > > >
> > > > > > >> > */
> > > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > > >> > -({ \
> > > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > > >> > - "=a"(out1), \
> > > > > > >> > - "=b"(out2), \
> > > > > > >> > - "=c"(out3), \
> > > > > > >> > - "=d"(out4), \
> > > > > > >> > - "=S"(__dummy1), \
> > > > > > >> > - "=D"(__dummy2) :\
> > > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > > >> > - "b"(in1), \
> > > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > > >> > - "memory"); \
> > > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4)
> > > > > > >> > \
> > > > > > >> > +({
> > > > > > >> > \
> > > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0;
> > > > > > >> > \
> > > > > > >>
> > > > > > >> Why do we need to initialize dummies?
> > > > > > >
> > > > > > > Because for some commands those parameters to VMW_PORT() can be
> > > > > > > both
> > > > > > > input and outout.
> > > > > >
> > > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > > benefit of doing this?
> > > > >
> > > > > There are two reasons. One is to make the code more readable and
> > > > > maintainable. Rather than having mostly similar inline assembly
> > > > > code sprinkled across multiple modules, we can just use the macros
> > > > > and document that.
> > > >
> > > > But the macro is only used here, and the variables aren't used at all,
> > > > so it makes no sense in this file.
> > >
> > > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > > sure what the proper distribution list is for each part.
> >
> > Use scripts/get_maintainer.pl, that's what it is there for. A number of
> > those patches should go through me, if not all of them, if you want them
> > merged...
> >
> > >
> > > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > > vmw_balloon.c
> >
> > And it's used inconsistantly in those patches (you don't set the dummy
> > variables to 0 in all of them...) Now maybe that's just how the asm
> > functions work, but it's not very obvious as to why this is at all.
> >
> > > > > The second reason is this organization makes some on-going future
> > > > > development easier.
> > > >
> > > > We don't plan for "future" development other than a single patch series,
> > > > as we have no idea what that development is, nor if it will really
> > > > happen. You can always change this file later if you need to, nothing
> > > > is keeping that from happening.
> > >
> > > So the intent of this series is to centralize similar lines of inline
> > > assembly code that are currently used by 3 different kernel modules
> > > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > > to include for common guest-host communication needs.
> >
> > Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> > create yet-another-.h-file for your bus? You already have 2, this would
> > make it 3, which seems like a lot...
>
> Umm, you are not saying that vmmouse should include vmci header file(s),
> are you? Because the 2 are unrelated and vmci does not use the
> hypervisor port to communicate with host IIRC.
vmmouse should include some type of "vmware bus" .h file, if it's not
the vmw_* files, what are they for? My point being, I didn't see the
need to add another .h file when we should probably already have one for
this bus, right?
thanks,
greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://li
Re: [Linux-graphics-maintainer] [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
On 12/02/2015 06:26 PM, Dmitry Torokhov wrote:
> On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
>> On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
>>> On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> Hi,
>
> On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
>> On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh wrote:
>>> Hi,
>>>
>
>
> */
> -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> -({ \
> - unsigned long __dummy1, __dummy2; \
> - __asm__ __volatile__ ("inl %%dx" : \
> - "=a"(out1), \
> - "=b"(out2), \
> - "=c"(out3), \
> - "=d"(out4), \
> - "=S"(__dummy1), \
> - "=D"(__dummy2) :\
> - "a"(VMMOUSE_PROTO_MAGIC), \
> - "b"(in1), \
> - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> - "d"(VMMOUSE_PROTO_PORT) : \
> - "memory"); \
> +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4)
> \
> +({\
> + unsigned long __dummy1 = 0, __dummy2 = 0; \
Why do we need to initialize dummies?
>>> Because for some commands those parameters to VMW_PORT() can be both
>>> input and outout.
>> The vmmouse commands do not use them as input though, so it seems we
>> are simply wasting CPU cycles setting them to 0 just because we are
>> using the new VMW_PORT here. Why do we need to switch? What is the
>> benefit of doing this?
> There are two reasons. One is to make the code more readable and
> maintainable. Rather than having mostly similar inline assembly
> code sprinkled across multiple modules, we can just use the macros
> and document that.
But the macro is only used here, and the variables aren't used at all,
so it makes no sense in this file.
>>> Maybe it's because I didn't CC you on the rest of the series. I wasn't
>>> sure what the proper distribution list is for each part.
>> Use scripts/get_maintainer.pl, that's what it is there for. A number of
>> those patches should go through me, if not all of them, if you want them
>> merged...
>>
>>> This new macro is also used in arch/x86/kernel/cpu/vmware.c and
>>> vmw_balloon.c
>> And it's used inconsistantly in those patches (you don't set the dummy
>> variables to 0 in all of them...) Now maybe that's just how the asm
>> functions work, but it's not very obvious as to why this is at all.
>>
> The second reason is this organization makes some on-going future
> development easier.
We don't plan for "future" development other than a single patch series,
as we have no idea what that development is, nor if it will really
happen. You can always change this file later if you need to, nothing
is keeping that from happening.
>>> So the intent of this series is to centralize similar lines of inline
>>> assembly code that are currently used by 3 different kernel modules
>>> to a central place. The new vmware.h [patch 0/6] becomes the one header
>>> to include for common guest-host communication needs.
>> Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
>> create yet-another-.h-file for your bus? You already have 2, this would
>> make it 3, which seems like a lot...
> Umm, you are not saying that vmmouse should include vmci header file(s),
> are you? Because the 2 are unrelated and vmci does not use the
> hypervisor port to communicate with host IIRC.
Also the platform setup code uses the hypervisor port, so it's a natural
place for the macro defines.
/Thomas
>
> Thanks.
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > Hi,
> > > >
> > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh wrote:
> > > > > > Hi,
> > > > > >
> > > >
> > > >
> > > >
> > > > > >> > */
> > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > >> > -({ \
> > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > >> > - "=a"(out1), \
> > > > > >> > - "=b"(out2), \
> > > > > >> > - "=c"(out3), \
> > > > > >> > - "=d"(out4), \
> > > > > >> > - "=S"(__dummy1), \
> > > > > >> > - "=D"(__dummy2) :\
> > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > >> > - "b"(in1), \
> > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > >> > - "memory"); \
> > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4)
> > > > > >> > \
> > > > > >> > +({\
> > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > >>
> > > > > >> Why do we need to initialize dummies?
> > > > > >
> > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > input and outout.
> > > > >
> > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > benefit of doing this?
> > > >
> > > > There are two reasons. One is to make the code more readable and
> > > > maintainable. Rather than having mostly similar inline assembly
> > > > code sprinkled across multiple modules, we can just use the macros
> > > > and document that.
> > >
> > > But the macro is only used here, and the variables aren't used at all,
> > > so it makes no sense in this file.
> >
> > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > sure what the proper distribution list is for each part.
>
> Use scripts/get_maintainer.pl, that's what it is there for. A number of
> those patches should go through me, if not all of them, if you want them
> merged...
>
> >
> > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > vmw_balloon.c
>
> And it's used inconsistantly in those patches (you don't set the dummy
> variables to 0 in all of them...) Now maybe that's just how the asm
> functions work, but it's not very obvious as to why this is at all.
>
> > > > The second reason is this organization makes some on-going future
> > > > development easier.
> > >
> > > We don't plan for "future" development other than a single patch series,
> > > as we have no idea what that development is, nor if it will really
> > > happen. You can always change this file later if you need to, nothing
> > > is keeping that from happening.
> >
> > So the intent of this series is to centralize similar lines of inline
> > assembly code that are currently used by 3 different kernel modules
> > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > to include for common guest-host communication needs.
>
> Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> create yet-another-.h-file for your bus? You already have 2, this would
> make it 3, which seems like a lot...
Umm, you are not saying that vmmouse should include vmci header file(s),
are you? Because the 2 are unrelated and vmci does not use the
hypervisor port to communicate with host IIRC.
Thanks.
--
Dmitry
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] virtio: Do not drop __GFP_HIGH in alloc_indirect
From: Michal Hocko
b92b1b89a33c ("virtio: force vring descriptors to be allocated from
lowmem") tried to exclude highmem pages for descriptors so it cleared
__GFP_HIGHMEM from a given gfp mask. The patch also cleared __GFP_HIGH
which doesn't make much sense for this fix because __GFP_HIGH only
controls access to memory reserves and it doesn't have any influence
on the zone selection. Some of the call paths use GFP_ATOMIC and
dropping __GFP_HIGH will reduce their changes for success because the
lack of access to memory reserves.
Signed-off-by: Michal Hocko
---
Hi,
I have stumbled over this code while looking at other issue [1]. I think
that using __GFP_HIGH simply got there because of its confusing name. It
doesn't have anything to do with the highmem zone.
The patch is based on the current linux-next.
I think that clearing __GFP_HIGHMEM is bogus in the current code because
all code paths either use GFP_KERNEL or GFP_ATOMIC and those do not fall
back to the highmem zone but I have kept it because clearing the flag
cannot be harmful.
[1] http://lkml.kernel.org/r/87h9k4kzcv.fsf%40yhuang-dev.intel.com
drivers/virtio/virtio_ring.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 14e7ce9b3e96..734de927c89d 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -110,7 +110,7 @@ static struct vring_desc *alloc_indirect(struct virtqueue
*_vq,
* otherwise virt_to_phys will give us bogus addresses in the
* virtqueue.
*/
- gfp &= ~(__GFP_HIGHMEM | __GFP_HIGH);
+ gfp &= ~__GFP_HIGHMEM;
desc = kmalloc(total_sg * sizeof(struct vring_desc), gfp);
if (!desc)
--
2.6.2
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 6/6] VMware balloon: Update vmw_balloon.c to use the VMW_PORT macro
Hey Sinclair,
On Tue, Dec 01, 2015 at 02:18:52PM -0800, Sinclair Yeh wrote:
> +#define VMWARE_BALLOON_CMD(cmd, data, result) \
> +({ \
> + unsigned long __status, __dummy1, __dummy2;\
> + unsigned long __si = 0, __di = 0; \
> + VMW_PORT(data, VMW_BALLOON_CMD_##cmd, VMW_BALLOON_HV_PORT, \
> + VMW_BALLOON_HV_MAGIC, \
> + __status, result, __dummy1, __dummy2, __si, __di);\
> + if (VMW_BALLOON_CMD_##cmd == VMW_BALLOON_CMD_START)\
> + result = __dummy1; \
> + result &= -1UL;\
> + __status & -1UL; \
> })
You need to indent the '\' with tabs only, and it looks like spaces are
present here (which is also why they don't look aligned).
Other than that I'm good with this:
Acked-by: Xavier Deguillard
Xavier
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [Linux-graphics-maintainer] [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
On 12/02/2015 01:04 AM, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
>> Hi,
>>
>> On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
>>> On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh wrote:
Hi,
>>
>>
>> */
>> -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> -({ \
>> - unsigned long __dummy1, __dummy2; \
>> - __asm__ __volatile__ ("inl %%dx" : \
>> - "=a"(out1), \
>> - "=b"(out2), \
>> - "=c"(out3), \
>> - "=d"(out4), \
>> - "=S"(__dummy1), \
>> - "=D"(__dummy2) :\
>> - "a"(VMMOUSE_PROTO_MAGIC), \
>> - "b"(in1), \
>> - "c"(VMMOUSE_PROTO_CMD_##cmd), \
>> - "d"(VMMOUSE_PROTO_PORT) : \
>> - "memory"); \
>> +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
>> +({\
>> + unsigned long __dummy1 = 0, __dummy2 = 0; \
> Why do we need to initialize dummies?
Because for some commands those parameters to VMW_PORT() can be both
input and outout.
>>> The vmmouse commands do not use them as input though, so it seems we
>>> are simply wasting CPU cycles setting them to 0 just because we are
>>> using the new VMW_PORT here. Why do we need to switch? What is the
>>> benefit of doing this?
>> There are two reasons. One is to make the code more readable and
>> maintainable. Rather than having mostly similar inline assembly
>> code sprinkled across multiple modules, we can just use the macros
>> and document that.
> But the macro is only used here, and the variables aren't used at all,
> so it makes no sense in this file.
>
IMO, this makes a lot of sense because we now get a single definition of
VMW_PORT in the platform code that a developer can refer to to
understand things like 32-64 bit compatibilty, and usage conditions and
it also forces the developer to adopt the good practice of clearing
currently unused input variables rather than to leave them undefined. In
addition, if something needs to be changed we have one single place to
change rather than a lot of places scattered all over various kernel
modules.
Things that we (I) previously, for example, didn't get quite right in
the vmmouse module despite spending a considerable amount of time on the
subject.
Thanks,
Thomas
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/6] x86: Add VMWare Host Communication Macros
Hey Sinclair,
On Tue, Dec 01, 2015 at 02:18:47PM -0800, Sinclair Yeh wrote:
> +/**
> + * Hypervisor-specific bi-directional communication channel. Should never
> + * execute on bare metal hardware. The caller must make sure to check for
> + * supported hypervisor before using these macros.
> + *
> + * Several of the parameters are both input and output and must be
> initialized.
> + *
> + * @in1: [IN] Message Len or Message Cmd (HB)
> + * @in2: [IN] Message Len (HB) or Message Cmd
Can you make in1 always be the "Message Cmd" and in2 always be the
"Message len"?
> + * @port_num: [IN] port number + [channel id]
> + * @magic: [IN] hypervisor magic value
> + * @eax: [OUT] value of EAX register
> + * @ebx: [OUT] e.g. status from an HB message status command
> + * @ecx: [OUT] e.g. status from a non-HB message status command
> + * @edx: [OUT] e.g. channel id
> + * @si: [INOUT] set to 0 if not used
> + * @di: [INOUT] set to 0 if not used
> + * @bp: [INOUT] set to 0 if not used
> + */
> +#define VMW_PORT(in1, in2, port_num, magic, eax, ebx, ecx, edx, si, di) \
> +({ \
> + __asm__ __volatile__ ("inl %%dx" : \
Are those '\' aligned in the code?
> +
> +#define VMW_PORT_HB_OUT(in1, in2, port_num, magic, \
> + eax, ebx, ecx, edx, si, di, bp) \
> +({ \
> + __asm__ __volatile__ ("movq %13, %%rbp;"\
Same here.
> +
> +#define VMW_PORT_HB_IN(in1, in2, port_num, magic,\
> +eax, ebx, ecx, edx, si, di, bp) \
> +({ \
> + __asm__ __volatile__ ("push %%rbp; movq %13, %%rbp;" \
Same.
Xavier
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
On Wed, Dec 02, 2015 at 07:31:24AM -0800, Greg Kroah-Hartman wrote:
> On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> > On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > > Hi,
> > > >
> > > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh wrote:
> > > > > > Hi,
> > > > > >
> > > >
> > > >
> > > >
> > > > > >> > */
> > > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > > >> > -({ \
> > > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > > >> > - "=a"(out1), \
> > > > > >> > - "=b"(out2), \
> > > > > >> > - "=c"(out3), \
> > > > > >> > - "=d"(out4), \
> > > > > >> > - "=S"(__dummy1), \
> > > > > >> > - "=D"(__dummy2) :\
> > > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > > >> > - "b"(in1), \
> > > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > > >> > - "memory"); \
> > > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4)
> > > > > >> > \
> > > > > >> > +({\
> > > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > > >>
> > > > > >> Why do we need to initialize dummies?
> > > > > >
> > > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > > input and outout.
> > > > >
> > > > > The vmmouse commands do not use them as input though, so it seems we
> > > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > > benefit of doing this?
> > > >
> > > > There are two reasons. One is to make the code more readable and
> > > > maintainable. Rather than having mostly similar inline assembly
> > > > code sprinkled across multiple modules, we can just use the macros
> > > > and document that.
> > >
> > > But the macro is only used here, and the variables aren't used at all,
> > > so it makes no sense in this file.
> >
> > Maybe it's because I didn't CC you on the rest of the series. I wasn't
> > sure what the proper distribution list is for each part.
>
> Use scripts/get_maintainer.pl, that's what it is there for. A number of
> those patches should go through me, if not all of them, if you want them
> merged...
>
> >
> > This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> > vmw_balloon.c
>
> And it's used inconsistantly in those patches (you don't set the dummy
> variables to 0 in all of them...) Now maybe that's just how the asm
> functions work, but it's not very obvious as to why this is at all.
>
> > > > The second reason is this organization makes some on-going future
> > > > development easier.
> > >
> > > We don't plan for "future" development other than a single patch series,
> > > as we have no idea what that development is, nor if it will really
> > > happen. You can always change this file later if you need to, nothing
> > > is keeping that from happening.
> >
> > So the intent of this series is to centralize similar lines of inline
> > assembly code that are currently used by 3 different kernel modules
> > to a central place. The new vmware.h [patch 0/6] becomes the one header
> > to include for common guest-host communication needs.
>
> Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
> create yet-another-.h-file for your bus? You already have 2, this would
> make it 3, which seems like a lot...
Ok, thanks. Let me see if it make sense to use one of the existing 2
files. Either way, I'll respin this series to include all the comments
so far.
>
> thanks,
>
> greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 3/6] Input: Update vmmouse.c to use the common VMW_PORT macros
On Tue, Dec 01, 2015 at 06:21:06PM -0800, Sinclair Yeh wrote:
> On Tue, Dec 01, 2015 at 04:04:08PM -0800, Greg Kroah-Hartman wrote:
> > On Tue, Dec 01, 2015 at 02:54:20PM -0800, Sinclair Yeh wrote:
> > > Hi,
> > >
> > > On Tue, Dec 01, 2015 at 02:45:27PM -0800, Dmitry Torokhov wrote:
> > > > On Tue, Dec 1, 2015 at 2:32 PM, Sinclair Yeh wrote:
> > > > > Hi,
> > > > >
> > >
> > >
> > >
> > > > >> > */
> > > > >> > -#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4) \
> > > > >> > -({ \
> > > > >> > - unsigned long __dummy1, __dummy2; \
> > > > >> > - __asm__ __volatile__ ("inl %%dx" : \
> > > > >> > - "=a"(out1), \
> > > > >> > - "=b"(out2), \
> > > > >> > - "=c"(out3), \
> > > > >> > - "=d"(out4), \
> > > > >> > - "=S"(__dummy1), \
> > > > >> > - "=D"(__dummy2) :\
> > > > >> > - "a"(VMMOUSE_PROTO_MAGIC), \
> > > > >> > - "b"(in1), \
> > > > >> > - "c"(VMMOUSE_PROTO_CMD_##cmd), \
> > > > >> > - "d"(VMMOUSE_PROTO_PORT) : \
> > > > >> > - "memory"); \
> > > > >> > +#define VMMOUSE_CMD(cmd, in1, out1, out2, out3, out4)
> > > > >> > \
> > > > >> > +({\
> > > > >> > + unsigned long __dummy1 = 0, __dummy2 = 0; \
> > > > >>
> > > > >> Why do we need to initialize dummies?
> > > > >
> > > > > Because for some commands those parameters to VMW_PORT() can be both
> > > > > input and outout.
> > > >
> > > > The vmmouse commands do not use them as input though, so it seems we
> > > > are simply wasting CPU cycles setting them to 0 just because we are
> > > > using the new VMW_PORT here. Why do we need to switch? What is the
> > > > benefit of doing this?
> > >
> > > There are two reasons. One is to make the code more readable and
> > > maintainable. Rather than having mostly similar inline assembly
> > > code sprinkled across multiple modules, we can just use the macros
> > > and document that.
> >
> > But the macro is only used here, and the variables aren't used at all,
> > so it makes no sense in this file.
>
> Maybe it's because I didn't CC you on the rest of the series. I wasn't
> sure what the proper distribution list is for each part.
Use scripts/get_maintainer.pl, that's what it is there for. A number of
those patches should go through me, if not all of them, if you want them
merged...
>
> This new macro is also used in arch/x86/kernel/cpu/vmware.c and
> vmw_balloon.c
And it's used inconsistantly in those patches (you don't set the dummy
variables to 0 in all of them...) Now maybe that's just how the asm
functions work, but it's not very obvious as to why this is at all.
> > > The second reason is this organization makes some on-going future
> > > development easier.
> >
> > We don't plan for "future" development other than a single patch series,
> > as we have no idea what that development is, nor if it will really
> > happen. You can always change this file later if you need to, nothing
> > is keeping that from happening.
>
> So the intent of this series is to centralize similar lines of inline
> assembly code that are currently used by 3 different kernel modules
> to a central place. The new vmware.h [patch 0/6] becomes the one header
> to include for common guest-host communication needs.
Why can't it go into vmw_vmci_defs.h instead, or your other .h file, why
create yet-another-.h-file for your bus? You already have 2, this would
make it 3, which seems like a lot...
thanks,
greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH net-next 3/3] vhost_net: basic polling support
On Wed, Dec 02, 2015 at 01:04:03PM +0800, Jason Wang wrote: > > > On 12/01/2015 10:43 PM, Michael S. Tsirkin wrote: > > On Tue, Dec 01, 2015 at 01:17:49PM +0800, Jason Wang wrote: > >> > >> On 11/30/2015 06:44 PM, Michael S. Tsirkin wrote: > >>> On Wed, Nov 25, 2015 at 03:11:29PM +0800, Jason Wang wrote: > > This patch tries to poll for new added tx buffer or socket receive > > queue for a while at the end of tx/rx processing. The maximum time > > spent on polling were specified through a new kind of vring ioctl. > > > > Signed-off-by: Jason Wang > >>> One further enhancement would be to actually poll > >>> the underlying device. This should be reasonably > >>> straight-forward with macvtap (especially in the > >>> passthrough mode). > >>> > >>> > >> Yes, it is. I have some patches to do this by replacing > >> skb_queue_empty() with sk_busy_loop() but for tap. > > We probably don't want to do this unconditionally, though. > > > >> Tests does not show > >> any improvement but some regression. > > Did you add code to call sk_mark_napi_id on tap then? > > sk_busy_loop won't do anything useful without. > > Yes I did. Probably something wrong elsewhere. Is this for guest-to-guest? the patch to do napi for tap is still not upstream due to minor performance regression. Want me to repost it? > > > >> Maybe it's better to test macvtap. > > Same thing ... > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 0/9] vhost-nvme: new qemu nvme backend using nvme target
On 02/12/2015 06:13, Ming Lin wrote: > On Tue, 2015-12-01 at 11:59 -0500, Paolo Bonzini wrote: >>> What do you think about virtio-nvme+vhost-nvme? >> >> What would be the advantage over virtio-blk? Multiqueue is not supported >> by QEMU but it's already supported by Linux (commit 6a27b656fc). > > I expect performance would be better. Why? nvme and virtio-blk are almost the same, even more so with the doorbell extension. virtio is designed to only hit paths that are not slowed down by virtualization. It's really hard to do better, except perhaps with VFIO (and then you don't need your vendor extension). >> To me, the advantage of nvme is that it provides more than decent >> performance on >> unmodified Windows guests, and thanks to your vendor extension can be used >> on Linux as well with speeds comparable to virtio-blk. So it's potentially >> a very good choice for a cloud provider that wants to support Windows guests >> (together with e.g. a fast SAS emulated controller to replace virtio-scsi, >> and emulated igb or ixgbe to replace virtio-net). > > vhost-nvme patches are learned from rts-megasas, which could possibly be > a fast SAS emulated controller. > https://github.com/Datera/rts-megasas Why the hate for userspace? :) I don't see a reason why vhost-nvme would be faster than a userspace implementation. vhost-blk was never committed upstream for similar reasons: it lost all the userspace features (snapshots, storage migration, etc.)---which are nice to have and do not cost performance if you do not use them---without any compelling performance gain. Without the doorbell extension you'd have to go back to userspace on every write and ioctl to vhost (see MEGASAS_IOC_FRAME in rts-megasas). With the doorbell extension you're doing exactly the same work, and then kernel thread vs. userspace thread shouldn't matter much given similar optimization effort. A userspace NVMe, however, will gain all optimization that is done to QEMU's block layer for free. We have done a lot and have more planned. >> Which features are supported by NVMe and not virtio-blk? Having read the driver, the main improvements of NVMe compared to virtio-blk are support for discard and FUA. Discard is easy to add to virtio-blk. In the past the idea was "just use virtio-scsi", but it may be worth adding it now that SSDs are more common. Thus, FUA is pretty much the only reason for a kernel-based implementation, because it is not exported in userspace. However, does it actually make a difference on real-world workloads? Local SSDs on Google Cloud are not even persistent, so you never need to flush to them. Paolo ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
