From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
"qemu-block@nongnu.org" <qemu-block@nongnu.org>,
"mreitz@redhat.com" <mreitz@redhat.com>,
"eblake@redhat.com" <eblake@redhat.com>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH] file-posix: Cache lseek result for data regions
Date: Fri, 25 Jan 2019 11:30:15 +0100 [thread overview]
Message-ID: <20190125103015.GA9055@linux.fritz.box> (raw)
In-Reply-To: <3cf90de9-5fc5-c24c-b232-bd0f564202a9@redhat.com>
Am 25.01.2019 um 11:10 hat Paolo Bonzini geschrieben:
> On 24/01/19 16:42, Kevin Wolf wrote:
> >> Recently Paolo asked me not to add more users of AioContext lock. Unfortunately
> >> I don't understand the whole picture around it.. Doesn't this apply here?
> >> https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg03410.html
> > I don't know. Honestly I feel nobody except Paolo knows, because we
> > don't know his patches yet.
>
> This is true. On the other hand, the AioContext lock is only used in
> some special cases around block jobs and blk_set_aio_context, and in
> general the block devices already should not have any dependencies
> (unless they crept in without me noticing).
It's also used in those cases where coroutines don't need locking, but
threads would. Did you audit all of the drivers for such cases?
> In particular...
>
> > But raw doesn't have an s->lock yet, so I
> > think removing the AioContext lock involves some work on it anyway and
> > adding this doesn't really change the amount of work.
>
> ... BDRVRawState doesn't have any data that changes after open, does it?
> This is why it doesn't have an s->lock.
No important data anyway. We do things like setting s->has_write_zeroes
= false after failure, but if we got a race and end up trying twice
before disabling it, it doesn't really hurt either.
Then there is reopen, but that involves a drain anyway. And that's it
probably.
So do you think I should introduce a CoMutex for raw here? Or QemuMutex?
Kevin
next prev parent reply other threads:[~2019-01-25 10:30 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-24 14:17 [Qemu-devel] [PATCH] file-posix: Cache lseek result for data regions Kevin Wolf
2019-01-24 14:40 ` Vladimir Sementsov-Ogievskiy
2019-01-24 15:11 ` Kevin Wolf
2019-01-24 15:22 ` Vladimir Sementsov-Ogievskiy
2019-01-24 15:42 ` Kevin Wolf
2019-01-25 10:10 ` Paolo Bonzini
2019-01-25 10:30 ` Kevin Wolf [this message]
2019-02-04 10:17 ` Paolo Bonzini
2019-01-24 15:56 ` Eric Blake
2019-01-29 10:56 ` Kevin Wolf
2019-01-29 21:03 ` Eric Blake
2019-01-24 16:18 ` Vladimir Sementsov-Ogievskiy
2019-01-24 16:36 ` Kevin Wolf
2019-01-25 9:13 ` Vladimir Sementsov-Ogievskiy
2019-01-25 13:26 ` Eric Blake
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=20190125103015.GA9055@linux.fritz.box \
--to=kwolf@redhat.com \
--cc=eblake@redhat.com \
--cc=mreitz@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@virtuozzo.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).