[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->chi
From: |
Stefan Hajnoczi |
Subject: |
Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock |
Date: |
Tue, 24 May 2022 11:20:47 +0100 |
On Tue, May 24, 2022 at 11:17:06AM +0200, Paolo Bonzini wrote:
> On 5/24/22 10:08, Stefan Hajnoczi wrote:
> > On Tue, May 24, 2022 at 09:55:39AM +0200, Paolo Bonzini wrote:
> > > On 5/22/22 17:06, Stefan Hajnoczi wrote:
> > > > However, I hit on a problem that I think Emanuele and Paolo have already
> > > > pointed out: draining is GS & IO. This might have worked under the 1
> > > > IOThread
> > > > model but it does not make sense for multi-queue. It is possible to
> > > > submit I/O
> > > > requests in drained sections. How can multiple threads be in drained
> > > > sections
> > > > simultaneously and possibly submit further I/O requests in their drained
> > > > sections? Those sections wouldn't be "drained" in any useful sense of
> > > > the word.
> > >
> > > Yeah, that works only if the drained sections are well-behaved.
> > >
> > > "External" sources of I/O are fine; they are disabled using is_external,
> > > and
> > > don't drain themselves I think.
> >
> > I/O requests for a given BDS may be executing in multiple AioContexts,
> > so how do you call aio_disable_external() on all relevant AioContexts?
>
> With multiqueue yeah, we have to replace aio_disable_external() with
> drained_begin/end() callbacks; but I'm not talking about that yet.
>
> > > In parallel to the block layer discussions, it's possible to work on
> > > introducing a request queue lock in virtio-blk and virtio-scsi. That's
> > > the
> > > only thing that relies on the AioContext lock outside the block layer.
> >
> > I'm not sure what the request queue lock protects in virtio-blk? In
> > virtio-scsi I guess a lock is needed to protect SCSI target emulation
> > state?
>
> Yes, but even in virtio-blk there is this code that runs in the main thread
> and is currently protected by aio_context_acquire/release:
>
> blk_drain(s->blk);
>
> /* We drop queued requests after blk_drain() because blk_drain()
> * itself can produce them. */
> while (s->rq) {
> req = s->rq;
> s->rq = req->next;
> virtqueue_detach_element(req->vq, &req->elem, 0);
> virtio_blk_free_request(req);
> }
>
> Maybe it's safe to run it without a lock because it runs after
> virtio_set_status(vdev, 0) but I'd rather play it safe and protect s->rq
> with a lock.
What does the lock protect?
A lock can prevent s->rq or req->vq corruption but it cannot prevent
request leaks. This loop's job is to free all requests so there is no
leak. If a lock is necessary then this code is already broken in a more
fundamental way because it can leak.
Stefan
signature.asc
Description: PGP signature
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, (continued)
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, Kevin Wolf, 2022/05/23
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, Stefan Hajnoczi, 2022/05/23
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, Kevin Wolf, 2022/05/23
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, Stefan Hajnoczi, 2022/05/23
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, Paolo Bonzini, 2022/05/24
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, Stefan Hajnoczi, 2022/05/24
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, Paolo Bonzini, 2022/05/24
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock,
Stefan Hajnoczi <=
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, Paolo Bonzini, 2022/05/24
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, Kevin Wolf, 2022/05/24
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, Paolo Bonzini, 2022/05/25
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, Stefan Hajnoczi, 2022/05/18
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, Kevin Wolf, 2022/05/24
- Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock, Emanuele Giuseppe Esposito, 2022/05/25