All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alberto Garcia <berto@igalia.com>
To: Manos Pitsidianakis <el13635@mail.ntua.gr>,
	Stefan Hajnoczi <stefanha@redhat.com>
Cc: qemu-devel <qemu-devel@nongnu.org>, Kevin Wolf <kwolf@redhat.com>,
	qemu-block <qemu-block@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH v3 4/7] block: convert ThrottleGroup to object with QOM
Date: Tue, 08 Aug 2017 15:01:10 +0200	[thread overview]
Message-ID: <w51y3qujjs9.fsf@maestria.local.igalia.com> (raw)
In-Reply-To: <20170802105704.m2jbxnivl32msbx6@postretch>

On Wed 02 Aug 2017 12:57:04 PM CEST, Manos Pitsidianakis wrote:
>> At the moment I think throttle_groups_lock isn't strictly needed
>> because incref/decref callers hold the QEMU global mutex anyway.
>>
>> But code accessing throttle_groups still has to be disciplined.
>> Since throttle_groups_lock exists, please use it consistently in all
>> code paths.
>>
>> Alternatively you could remove the lock and document that
>> throttle_groups is protected by the global mutex.  What we can't do
>> is sometimes use throttle_groups_lock and sometimes not use it.
>
> If we use throttle_groups_lock in throttle_group_obj_init() then we
> must give it up in throttle_group_incref() and retake it in
> throttle_group_obj_init(). Maybe indeed it's better to drop
> throttle_groups_lock altogether, since the ThrottleGroup refcounting
> always happens in a QMP or startup/cleanup context.

I checked the code and I also don't see any manipulation of the group
list outside the global mutex, so you can remove throttle_groups_lock,
but please document very clearly that all these calls can only happen
when they are protected by the global mutex.

Berto

  parent reply	other threads:[~2017-08-08 13:01 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-31  9:54 [Qemu-devel] [PATCH v3 0/7] add throttle block driver filter Manos Pitsidianakis
2017-07-31  9:54 ` [Qemu-devel] [PATCH v3 1/7] block: move ThrottleGroup membership to ThrottleGroupMember Manos Pitsidianakis
2017-08-04 11:59   ` Alberto Garcia
2017-07-31  9:54 ` [Qemu-devel] [PATCH v3 2/7] block: add aio_context field in ThrottleGroupMember Manos Pitsidianakis
2017-08-04 12:14   ` Alberto Garcia
2017-07-31  9:54 ` [Qemu-devel] [PATCH v3 3/7] block: tidy ThrottleGroupMember initializations Manos Pitsidianakis
2017-08-04 12:35   ` Alberto Garcia
2017-07-31  9:54 ` [Qemu-devel] [PATCH v3 4/7] block: convert ThrottleGroup to object with QOM Manos Pitsidianakis
2017-08-01 15:47   ` Stefan Hajnoczi
2017-08-01 16:49     ` Manos Pitsidianakis
2017-08-02 10:39       ` Stefan Hajnoczi
2017-08-02 10:57         ` Manos Pitsidianakis
2017-08-02 14:43           ` Stefan Hajnoczi
2017-08-03  8:08           ` Kevin Wolf
2017-08-03 10:53             ` Stefan Hajnoczi
2017-08-03 11:17               ` Kevin Wolf
2017-08-03 12:29                 ` Manos Pitsidianakis
2017-08-08 13:01           ` Alberto Garcia [this message]
2017-07-31  9:54 ` [Qemu-devel] [PATCH v3 5/7] block: add throttle block filter driver Manos Pitsidianakis
2017-08-01 16:14   ` Stefan Hajnoczi
2017-08-03  8:07   ` Kevin Wolf
2017-08-03 11:48     ` Manos Pitsidianakis
2017-08-03 12:05       ` Kevin Wolf
2017-08-03 11:58     ` Eric Blake
2017-08-03 13:56       ` Manos Pitsidianakis
2017-08-08 13:13   ` Alberto Garcia
2017-08-08 13:45     ` Manos Pitsidianakis
2017-08-08 14:53       ` Alberto Garcia
2017-08-08 14:56         ` Manos Pitsidianakis
2017-08-08 15:04           ` Alberto Garcia
2017-08-09  9:36             ` Manos Pitsidianakis
2017-08-09 12:36               ` Alberto Garcia
2017-08-09 13:42                 ` Manos Pitsidianakis
2017-08-09 14:45                   ` Alberto Garcia
2017-08-09 15:39                     ` Kevin Wolf
2017-08-14 12:15                       ` Manos Pitsidianakis
2017-07-31  9:54 ` [Qemu-devel] [PATCH v3 6/7] block: add BlockDevOptionsThrottle to QAPI Manos Pitsidianakis
2017-08-01 16:16   ` Stefan Hajnoczi
2017-07-31  9:54 ` [Qemu-devel] [PATCH v3 7/7] block: add throttle block filter driver interface tests Manos Pitsidianakis
2017-08-03  8:07   ` Kevin Wolf
2017-08-03 13:24     ` Manos Pitsidianakis
2017-08-03 13:32       ` Kevin Wolf
2017-08-03 13:52         ` Manos Pitsidianakis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=w51y3qujjs9.fsf@maestria.local.igalia.com \
    --to=berto@igalia.com \
    --cc=el13635@mail.ntua.gr \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.