Patchwork [3/3] kvm: Fix obsolete comment about locking for kvm_io_bus_read/write

login
register
mail settings
Submitter Scott Wood
Date April 27, 2013, 12:53 a.m.
Message ID <1367024020-14204-3-git-send-email-scottwood@freescale.com>
Download mbox | patch
Permalink /patch/240064/
State New
Headers show

Comments

Scott Wood - April 27, 2013, 12:53 a.m.
Nobody is actually calling these functions with slots_lock held.
The srcu read lock, OTOH, is required.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 virt/kvm/kvm_main.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Alexander Graf - May 1, 2013, 11:24 p.m.
On 27.04.2013, at 02:53, Scott Wood wrote:

> Nobody is actually calling these functions with slots_lock held.
> The srcu read lock, OTOH, is required.
> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>

Marcelo, could you please ack?


Alex

> ---
> virt/kvm/kvm_main.c |    4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 5da9f02..54a14fa 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2828,7 +2828,7 @@ static int kvm_io_bus_get_first_dev(struct kvm_io_bus *bus,
> 	return off;
> }
> 
> -/* kvm_io_bus_write - called under kvm->slots_lock */
> +/* kvm_io_bus_write - called under kvm->srcu read lock */
> int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
> 		     int len, const void *val)
> {
> @@ -2856,7 +2856,7 @@ int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
> 	return -EOPNOTSUPP;
> }
> 
> -/* kvm_io_bus_read - called under kvm->slots_lock */
> +/* kvm_io_bus_read - called under kvm->srcu read lock */
> int kvm_io_bus_read(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
> 		    int len, void *val)
> {
> -- 
> 1.7.10.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov - May 2, 2013, 7:18 a.m.
On Thu, May 02, 2013 at 01:24:15AM +0200, Alexander Graf wrote:
> 
> On 27.04.2013, at 02:53, Scott Wood wrote:
> 
> > Nobody is actually calling these functions with slots_lock held.
> > The srcu read lock, OTOH, is required.
> > 
> > Signed-off-by: Scott Wood <scottwood@freescale.com>
> 
> Marcelo, could you please ack?
> 
Is something in the series depends on this patch? If not this should go
directly into kvm.git, not via ppc tree.

> 
> Alex
> 
> > ---
> > virt/kvm/kvm_main.c |    4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 5da9f02..54a14fa 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -2828,7 +2828,7 @@ static int kvm_io_bus_get_first_dev(struct kvm_io_bus *bus,
> > 	return off;
> > }
> > 
> > -/* kvm_io_bus_write - called under kvm->slots_lock */
> > +/* kvm_io_bus_write - called under kvm->srcu read lock */
> > int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
> > 		     int len, const void *val)
> > {
> > @@ -2856,7 +2856,7 @@ int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
> > 	return -EOPNOTSUPP;
> > }
> > 
> > -/* kvm_io_bus_read - called under kvm->slots_lock */
> > +/* kvm_io_bus_read - called under kvm->srcu read lock */
> > int kvm_io_bus_read(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
> > 		    int len, void *val)
> > {
> > -- 
> > 1.7.10.4
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf - May 2, 2013, 10:53 a.m.
On 02.05.2013, at 09:18, Gleb Natapov wrote:

> On Thu, May 02, 2013 at 01:24:15AM +0200, Alexander Graf wrote:
>> 
>> On 27.04.2013, at 02:53, Scott Wood wrote:
>> 
>>> Nobody is actually calling these functions with slots_lock held.
>>> The srcu read lock, OTOH, is required.
>>> 
>>> Signed-off-by: Scott Wood <scottwood@freescale.com>
>> 
>> Marcelo, could you please ack?
>> 
> Is something in the series depends on this patch? If not this should go
> directly into kvm.git, not via ppc tree.

If that's easier for you, I'm perfectly fine with that.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gleb Natapov - May 2, 2013, 11 a.m.
On Thu, May 02, 2013 at 12:53:44PM +0200, Alexander Graf wrote:
> 
> On 02.05.2013, at 09:18, Gleb Natapov wrote:
> 
> > On Thu, May 02, 2013 at 01:24:15AM +0200, Alexander Graf wrote:
> >> 
> >> On 27.04.2013, at 02:53, Scott Wood wrote:
> >> 
> >>> Nobody is actually calling these functions with slots_lock held.
> >>> The srcu read lock, OTOH, is required.
> >>> 
> >>> Signed-off-by: Scott Wood <scottwood@freescale.com>
> >> 
> >> Marcelo, could you please ack?
> >> 
> > Is something in the series depends on this patch? If not this should go
> > directly into kvm.git, not via ppc tree.
> 
> If that's easier for you, I'm perfectly fine with that.
> 
It really is. In the case of this patch it is not a big deal of course, but it
helps with tracking which changes an architecture code actually depends
on, and what are just generic fixes. When fixes for the common code are
hidden in the middle of a ppc patch set they are easy to miss. 

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf - May 2, 2013, 11:25 a.m.
On 02.05.2013, at 13:00, Gleb Natapov wrote:

> On Thu, May 02, 2013 at 12:53:44PM +0200, Alexander Graf wrote:
>> 
>> On 02.05.2013, at 09:18, Gleb Natapov wrote:
>> 
>>> On Thu, May 02, 2013 at 01:24:15AM +0200, Alexander Graf wrote:
>>>> 
>>>> On 27.04.2013, at 02:53, Scott Wood wrote:
>>>> 
>>>>> Nobody is actually calling these functions with slots_lock held.
>>>>> The srcu read lock, OTOH, is required.
>>>>> 
>>>>> Signed-off-by: Scott Wood <scottwood@freescale.com>
>>>> 
>>>> Marcelo, could you please ack?
>>>> 
>>> Is something in the series depends on this patch? If not this should go
>>> directly into kvm.git, not via ppc tree.
>> 
>> If that's easier for you, I'm perfectly fine with that.
>> 
> It really is. In the case of this patch it is not a big deal of course, but it
> helps with tracking which changes an architecture code actually depends
> on, and what are just generic fixes. When fixes for the common code are
> hidden in the middle of a ppc patch set they are easy to miss. 

I agree :). That's why Scott sent the other generic patch separately. I suppose he just figured that a comment change isn't too big of a deal.


Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 5da9f02..54a14fa 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2828,7 +2828,7 @@  static int kvm_io_bus_get_first_dev(struct kvm_io_bus *bus,
 	return off;
 }
 
-/* kvm_io_bus_write - called under kvm->slots_lock */
+/* kvm_io_bus_write - called under kvm->srcu read lock */
 int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 		     int len, const void *val)
 {
@@ -2856,7 +2856,7 @@  int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 	return -EOPNOTSUPP;
 }
 
-/* kvm_io_bus_read - called under kvm->slots_lock */
+/* kvm_io_bus_read - called under kvm->srcu read lock */
 int kvm_io_bus_read(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 		    int len, void *val)
 {