Re: [PATCH 3/3] KVM: Add proper lockdep assertion in I/O bus unregister
On Thu, Apr 15, 2021, Jim Mattson wrote: > On Mon, Apr 12, 2021 at 3:23 PM Sean Christopherson wrote: > > > > Convert a comment above kvm_io_bus_unregister_dev() into an actual > > lockdep assertion, and opportunistically add curly braces to a multi-line > > for-loop. > > > > Signed-off-by: Sean Christopherson > > --- > > virt/kvm/kvm_main.c | 6 -- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > > index ab1fa6f92c82..ccc2ef1dbdda 100644 > > --- a/virt/kvm/kvm_main.c > > +++ b/virt/kvm/kvm_main.c > > @@ -4485,21 +4485,23 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum > > kvm_bus bus_idx, gpa_t addr, > > return 0; > > } > > > > -/* Caller must hold slots_lock. */ > > int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx, > > struct kvm_io_device *dev) > > { > > int i, j; > > struct kvm_io_bus *new_bus, *bus; > > > > + lockdep_assert_held(>slots_lock); > > + > > bus = kvm_get_bus(kvm, bus_idx); > > if (!bus) > > return 0; > > > > - for (i = 0; i < bus->dev_count; i++) > > + for (i = 0; i < bus->dev_count; i++) { > > if (bus->range[i].dev == dev) { > > break; > > } > > + } > Per coding-style.rst, neither the for loop nor the if-block should have > braces. > > "Do not unnecessarily use braces where a single statement will do." Doh, the if-statement should indeed not use braces. I think I meant to clean that up, and then saw something shiny... But the for-loop... keep reading :-D Also, use braces when a loop contains more than a single simple statement: .. code-block:: c while (condition) { if (test) do_something(); }
Re: [PATCH 3/3] KVM: Add proper lockdep assertion in I/O bus unregister
On Mon, Apr 12, 2021 at 3:23 PM Sean Christopherson wrote: > > Convert a comment above kvm_io_bus_unregister_dev() into an actual > lockdep assertion, and opportunistically add curly braces to a multi-line > for-loop. > > Signed-off-by: Sean Christopherson > --- > virt/kvm/kvm_main.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index ab1fa6f92c82..ccc2ef1dbdda 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -4485,21 +4485,23 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum > kvm_bus bus_idx, gpa_t addr, > return 0; > } > > -/* Caller must hold slots_lock. */ > int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx, > struct kvm_io_device *dev) > { > int i, j; > struct kvm_io_bus *new_bus, *bus; > > + lockdep_assert_held(>slots_lock); > + > bus = kvm_get_bus(kvm, bus_idx); > if (!bus) > return 0; > > - for (i = 0; i < bus->dev_count; i++) > + for (i = 0; i < bus->dev_count; i++) { > if (bus->range[i].dev == dev) { > break; > } > + } Per coding-style.rst, neither the for loop nor the if-block should have braces. "Do not unnecessarily use braces where a single statement will do." Stylistic nits aside, Reviewed-by: Jim Mattson
[PATCH 3/3] KVM: Add proper lockdep assertion in I/O bus unregister
Convert a comment above kvm_io_bus_unregister_dev() into an actual lockdep assertion, and opportunistically add curly braces to a multi-line for-loop. Signed-off-by: Sean Christopherson --- virt/kvm/kvm_main.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index ab1fa6f92c82..ccc2ef1dbdda 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -4485,21 +4485,23 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr, return 0; } -/* Caller must hold slots_lock. */ int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx, struct kvm_io_device *dev) { int i, j; struct kvm_io_bus *new_bus, *bus; + lockdep_assert_held(>slots_lock); + bus = kvm_get_bus(kvm, bus_idx); if (!bus) return 0; - for (i = 0; i < bus->dev_count; i++) + for (i = 0; i < bus->dev_count; i++) { if (bus->range[i].dev == dev) { break; } + } if (i == bus->dev_count) return 0; -- 2.31.1.295.g9ea45b61b8-goog