[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 for-6.0? 0/3] qcow2: fix parallel rewrite and discard (rw-
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH v4 for-6.0? 0/3] qcow2: fix parallel rewrite and discard (rw-lock) |
Date: |
Tue, 30 Mar 2021 13:51:40 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 |
30.03.2021 12:49, Max Reitz wrote:
On 25.03.21 20:12, Vladimir Sementsov-Ogievskiy wrote:
ping. Do we want it for 6.0?
I’d rather wait. I think the conclusion was that guests shouldn’t hit this
because they serialize discards?
I think, that we never had bugs, so we of course can wait.
There’s also something Kevin wrote on IRC a couple of weeks ago, for which I
had hoped he’d sent an email but I don’t think he did, so I’ll try to remember
and paraphrase as well as I can...
He basically asked whether it wouldn’t be conceptually simpler to take a
reference to some cluster in get_cluster_offset() and later release it with a
to-be-added put_cluster_offset().
He also noted that reading is problematic, too, because if you read a discarded
and reused cluster, this might result in an information leak (some guest
application might be able to read data it isn’t allowed to read); that’s why
making get_cluster_offset() the point of locking clusters against discarding
would be better.
Yes, I thought about read too, (RFCed in cover letter of [PATCH v5 0/6] qcow2:
fix parallel rewrite and discard (lockless))
This would probably work with both of your solutions. For the in-memory
solutions, you’d take a refcount to an actual cluster; in the CoRwLock
solution, you’d take that lock.
What do you think?
Hmm. What do you mean? Just rename my qcow2_inflight_writes_inc() and
qcow2_inflight_writes_dec() to get_cluster_offset()/put_cluster_offset(), to
make it more native to use for read operations as well?
Or to update any kind of "getting cluster offset" in the whole qcow2 driver to take a kind of
"dynamic reference count" by get_cluster_offset() and then call corresponding put() somewhere? In this
case I'm afraid it's a lot more work.. It would be also the problem that a lot of paths in qcow2 are not in
coroutine and don't even take s->lock when they actually should. This will also mean that we do same job as
normal qcow2 refcounts already do: no sense in keeping additional "dynamic refcount" for L2 table
cluster while reading it, as we already have non-zero qcow2 normal refcount for it..
--
Best regards,
Vladimir
- [PATCH v4 0/3] qcow2: fix parallel rewrite and discard (rw-lock), Vladimir Sementsov-Ogievskiy, 2021/03/19
- [PATCH v4 1/3] qemu-io: add aio_discard, Vladimir Sementsov-Ogievskiy, 2021/03/19
- [PATCH v4 2/3] iotests: add qcow2-discard-during-rewrite, Vladimir Sementsov-Ogievskiy, 2021/03/19
- [PATCH v4 3/3] block/qcow2: introduce discard_rw_lock: fix discarding host clusters, Vladimir Sementsov-Ogievskiy, 2021/03/19
- Re: [PATCH v4 for-6.0? 0/3] qcow2: fix parallel rewrite and discard (rw-lock), Vladimir Sementsov-Ogievskiy, 2021/03/25
- Re: [PATCH v4 for-6.0? 0/3] qcow2: fix parallel rewrite and discard (rw-lock), Max Reitz, 2021/03/30
- Re: [PATCH v4 for-6.0? 0/3] qcow2: fix parallel rewrite and discard (rw-lock),
Vladimir Sementsov-Ogievskiy <=
- Re: [PATCH v4 for-6.0? 0/3] qcow2: fix parallel rewrite and discard (rw-lock), Max Reitz, 2021/03/30
- Re: [PATCH v4 for-6.0? 0/3] qcow2: fix parallel rewrite and discard (rw-lock), Vladimir Sementsov-Ogievskiy, 2021/03/30
- Re: [PATCH v4 for-6.0? 0/3] qcow2: fix parallel rewrite and discard (rw-lock), Max Reitz, 2021/03/30
- Re: [PATCH v4 for-6.0? 0/3] qcow2: fix parallel rewrite and discard (rw-lock), Vladimir Sementsov-Ogievskiy, 2021/03/30
- Re: [PATCH v4 for-6.0? 0/3] qcow2: fix parallel rewrite and discard (rw-lock), Vladimir Sementsov-Ogievskiy, 2021/03/31