qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH] file-posix: Cache lseek result for data regions


From: Kevin Wolf
Subject: Re: [Qemu-block] [PATCH] file-posix: Cache lseek result for data regions
Date: Fri, 25 Jan 2019 11:30:15 +0100
User-agent: Mutt/1.10.1 (2018-07-13)

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]