* [PATCH 1/2] hyperv: Use auto rcu_read macros
2019-12-13 13:19 [PATCH 0/2] rcu_read auto macro use Dr. David Alan Gilbert (git)
@ 2019-12-13 13:19 ` Dr. David Alan Gilbert (git)
2019-12-13 13:58 ` Roman Kagan
2019-12-13 13:19 ` [PATCH 2/2] qsp: Use WITH_RCU_READ_LOCK_GUARD Dr. David Alan Gilbert (git)
2019-12-13 14:04 ` [PATCH 0/2] rcu_read auto macro use Paolo Bonzini
2 siblings, 1 reply; 5+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-12-13 13:19 UTC (permalink / raw)
To: qemu-devel, pbonzini, cota, rkagan
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Use RCU_READ_LOCK_GUARD and WITH_RCU_READ_LOCK_GUARD
to replace the manual rcu_read_(un)lock calls.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hw/hyperv/hyperv.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
index 6ebf31c310..da8ce82725 100644
--- a/hw/hyperv/hyperv.c
+++ b/hw/hyperv/hyperv.c
@@ -546,14 +546,14 @@ uint16_t hyperv_hcall_post_message(uint64_t param, bool fast)
}
ret = HV_STATUS_INVALID_CONNECTION_ID;
- rcu_read_lock();
- QLIST_FOREACH_RCU(mh, &msg_handlers, link) {
- if (mh->conn_id == (msg->connection_id & HV_CONNECTION_ID_MASK)) {
- ret = mh->handler(msg, mh->data);
- break;
+ WITH_RCU_READ_LOCK_GUARD() {
+ QLIST_FOREACH_RCU(mh, &msg_handlers, link) {
+ if (mh->conn_id == (msg->connection_id & HV_CONNECTION_ID_MASK)) {
+ ret = mh->handler(msg, mh->data);
+ break;
+ }
}
}
- rcu_read_unlock();
unmap:
cpu_physical_memory_unmap(msg, len, 0, 0);
@@ -619,7 +619,6 @@ int hyperv_set_event_flag_handler(uint32_t conn_id, EventNotifier *notifier)
uint16_t hyperv_hcall_signal_event(uint64_t param, bool fast)
{
- uint16_t ret;
EventFlagHandler *handler;
if (unlikely(!fast)) {
@@ -645,15 +644,12 @@ uint16_t hyperv_hcall_signal_event(uint64_t param, bool fast)
return HV_STATUS_INVALID_HYPERCALL_INPUT;
}
- ret = HV_STATUS_INVALID_CONNECTION_ID;
- rcu_read_lock();
+ RCU_READ_LOCK_GUARD();
QLIST_FOREACH_RCU(handler, &event_flag_handlers, link) {
if (handler->conn_id == param) {
event_notifier_set(handler->notifier);
- ret = 0;
- break;
+ return 0;
}
}
- rcu_read_unlock();
- return ret;
+ return HV_STATUS_INVALID_CONNECTION_ID;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] hyperv: Use auto rcu_read macros
2019-12-13 13:19 ` [PATCH 1/2] hyperv: Use auto rcu_read macros Dr. David Alan Gilbert (git)
@ 2019-12-13 13:58 ` Roman Kagan
0 siblings, 0 replies; 5+ messages in thread
From: Roman Kagan @ 2019-12-13 13:58 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git); +Cc: pbonzini, cota, qemu-devel
On Fri, Dec 13, 2019 at 01:19:30PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Use RCU_READ_LOCK_GUARD and WITH_RCU_READ_LOCK_GUARD
> to replace the manual rcu_read_(un)lock calls.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> hw/hyperv/hyperv.c | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
> index 6ebf31c310..da8ce82725 100644
> --- a/hw/hyperv/hyperv.c
> +++ b/hw/hyperv/hyperv.c
> @@ -546,14 +546,14 @@ uint16_t hyperv_hcall_post_message(uint64_t param, bool fast)
> }
>
> ret = HV_STATUS_INVALID_CONNECTION_ID;
> - rcu_read_lock();
> - QLIST_FOREACH_RCU(mh, &msg_handlers, link) {
> - if (mh->conn_id == (msg->connection_id & HV_CONNECTION_ID_MASK)) {
> - ret = mh->handler(msg, mh->data);
> - break;
> + WITH_RCU_READ_LOCK_GUARD() {
> + QLIST_FOREACH_RCU(mh, &msg_handlers, link) {
> + if (mh->conn_id == (msg->connection_id & HV_CONNECTION_ID_MASK)) {
> + ret = mh->handler(msg, mh->data);
> + break;
> + }
> }
> }
> - rcu_read_unlock();
>
> unmap:
> cpu_physical_memory_unmap(msg, len, 0, 0);
> @@ -619,7 +619,6 @@ int hyperv_set_event_flag_handler(uint32_t conn_id, EventNotifier *notifier)
>
> uint16_t hyperv_hcall_signal_event(uint64_t param, bool fast)
> {
> - uint16_t ret;
> EventFlagHandler *handler;
>
> if (unlikely(!fast)) {
> @@ -645,15 +644,12 @@ uint16_t hyperv_hcall_signal_event(uint64_t param, bool fast)
> return HV_STATUS_INVALID_HYPERCALL_INPUT;
> }
>
> - ret = HV_STATUS_INVALID_CONNECTION_ID;
> - rcu_read_lock();
> + RCU_READ_LOCK_GUARD();
> QLIST_FOREACH_RCU(handler, &event_flag_handlers, link) {
> if (handler->conn_id == param) {
> event_notifier_set(handler->notifier);
> - ret = 0;
> - break;
> + return 0;
> }
> }
> - rcu_read_unlock();
> - return ret;
> + return HV_STATUS_INVALID_CONNECTION_ID;
> }
I have a slight preference towards using WITH_RCU_READ_LOCK_GUARD
instead of sticking RCU_READ_LOCK_GUARD in the middle of the function
and implicitly relying on there being none but trivial statements past
the rcu-protected section.
Nothing that I would insist on, though, so
Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] qsp: Use WITH_RCU_READ_LOCK_GUARD
2019-12-13 13:19 [PATCH 0/2] rcu_read auto macro use Dr. David Alan Gilbert (git)
2019-12-13 13:19 ` [PATCH 1/2] hyperv: Use auto rcu_read macros Dr. David Alan Gilbert (git)
@ 2019-12-13 13:19 ` Dr. David Alan Gilbert (git)
2019-12-13 14:04 ` [PATCH 0/2] rcu_read auto macro use Paolo Bonzini
2 siblings, 0 replies; 5+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-12-13 13:19 UTC (permalink / raw)
To: qemu-devel, pbonzini, cota, rkagan
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
The automatic rcu read lock maintenance works quite
nicely in this case where it previously relied on a comment to
delimit the lifetime and now has a block.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
util/qsp.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/util/qsp.c b/util/qsp.c
index 62265417fd..7d5147f1b2 100644
--- a/util/qsp.c
+++ b/util/qsp.c
@@ -598,7 +598,6 @@ static void qsp_ht_delete(void *p, uint32_t h, void *htp)
static void qsp_mktree(GTree *tree, bool callsite_coalesce)
{
- QSPSnapshot *snap;
struct qht ht, coalesce_ht;
struct qht *htp;
@@ -610,20 +609,19 @@ static void qsp_mktree(GTree *tree, bool callsite_coalesce)
* We must remain in an RCU read-side critical section until we're done
* with the snapshot.
*/
- rcu_read_lock();
- snap = atomic_rcu_read(&qsp_snapshot);
+ WITH_RCU_READ_LOCK_GUARD() {
+ QSPSnapshot *snap = atomic_rcu_read(&qsp_snapshot);
- /* Aggregate all results from the global hash table into a local one */
- qht_init(&ht, qsp_entry_no_thread_cmp, QSP_INITIAL_SIZE,
- QHT_MODE_AUTO_RESIZE | QHT_MODE_RAW_MUTEXES);
- qht_iter(&qsp_ht, qsp_aggregate, &ht);
+ /* Aggregate all results from the global hash table into a local one */
+ qht_init(&ht, qsp_entry_no_thread_cmp, QSP_INITIAL_SIZE,
+ QHT_MODE_AUTO_RESIZE | QHT_MODE_RAW_MUTEXES);
+ qht_iter(&qsp_ht, qsp_aggregate, &ht);
- /* compute the difference wrt the snapshot, if any */
- if (snap) {
- qsp_diff(&snap->ht, &ht);
+ /* compute the difference wrt the snapshot, if any */
+ if (snap) {
+ qsp_diff(&snap->ht, &ht);
+ }
}
- /* done with the snapshot; RCU can reclaim it */
- rcu_read_unlock();
htp = &ht;
if (callsite_coalesce) {
--
2.23.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/2] rcu_read auto macro use
2019-12-13 13:19 [PATCH 0/2] rcu_read auto macro use Dr. David Alan Gilbert (git)
2019-12-13 13:19 ` [PATCH 1/2] hyperv: Use auto rcu_read macros Dr. David Alan Gilbert (git)
2019-12-13 13:19 ` [PATCH 2/2] qsp: Use WITH_RCU_READ_LOCK_GUARD Dr. David Alan Gilbert (git)
@ 2019-12-13 14:04 ` Paolo Bonzini
2 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2019-12-13 14:04 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git), qemu-devel, cota, rkagan
On 13/12/19 14:19, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Hi,
> A couple more uses of the rcu_read macros; in qsp and
> hyperv (neither of which list maintainers, so I guess
> best through RCU).
>
> The hyperv case saves a temporary.
> The qsp case uses an rcu_read_lock around the lifetime
> of a snapshot and carefully comments that; but now
> it's automatic.
>
> [Hyperv not tested]
Queued, thanks.
Paolo
> Dave
>
> Dr. David Alan Gilbert (2):
> hyperv: Use auto rcu_read macros
> qsp: Use WITH_RCU_READ_LOCK_GUARD
>
> hw/hyperv/hyperv.c | 22 +++++++++-------------
> util/qsp.c | 22 ++++++++++------------
> 2 files changed, 19 insertions(+), 25 deletions(-)
>
^ permalink raw reply [flat|nested] 5+ messages in thread