From: Stefan Hajnoczi <stefanha@redhat.com>
To: Stefano Garzarella <sgarzare@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
"Jason Wang" <jasowang@redhat.com>,
"Cornelia Huck" <cohuck@redhat.com>,
qemu-devel@nongnu.org, "Paolo Bonzini" <pbonzini@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [RFC PATCH 3/3] hw/virtio: Have virtqueue_get_avail_bytes() pass caches arg to callees
Date: Thu, 2 Sep 2021 13:12:33 +0100 [thread overview]
Message-ID: <YTC/savtIjlbzFsO@stefanha-x1.localdomain> (raw)
In-Reply-To: <20210901155538.vbtxakrtbjwon3pt@steredhat>
[-- Attachment #1: Type: text/plain, Size: 1007 bytes --]
On Wed, Sep 01, 2021 at 05:55:38PM +0200, Stefano Garzarella wrote:
> On Thu, Aug 26, 2021 at 07:26:58PM +0200, Philippe Mathieu-Daudé wrote:
> > Both virtqueue_packed_get_avail_bytes() and
> > virtqueue_split_get_avail_bytes() access the region cache, but
> > their caller also does. Simplify by having virtqueue_get_avail_bytes
> > calling both with RCU lock held, and passing the caches as argument.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> > RFC because I'm not sure this is safe enough
>
> It seems safe to me.
>
> While reviewing I saw that vring_get_region_caches() has
> /* Called within rcu_read_lock(). */ comment, but it seems to me that we
> call that function in places where we haven't acquired it, which shouldn't
> be a problem, but should we remove that comment?
Do you have specific examples? That sounds worrying because the caller
can't do much with the returned pointer if it was fetched outside the
RCU read lock.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2021-09-02 12:27 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-26 17:26 [PATCH 0/3] hw/virtio: Minor housekeeping patches Philippe Mathieu-Daudé
2021-08-26 17:26 ` [PATCH 1/3] hw/virtio: Document virtio_queue_packed_empty_rcu is called within RCU Philippe Mathieu-Daudé
2021-09-01 15:56 ` Stefano Garzarella
2021-09-02 12:07 ` Stefan Hajnoczi
2021-08-26 17:26 ` [PATCH 2/3] hw/virtio: Remove NULL check in virtio_free_region_cache() Philippe Mathieu-Daudé
2021-09-01 15:56 ` Stefano Garzarella
2021-09-02 12:07 ` Stefan Hajnoczi
2021-08-26 17:26 ` [RFC PATCH 3/3] hw/virtio: Have virtqueue_get_avail_bytes() pass caches arg to callees Philippe Mathieu-Daudé
2021-09-01 15:55 ` Stefano Garzarella
2021-09-02 12:12 ` Stefan Hajnoczi [this message]
2021-09-02 13:09 ` Stefano Garzarella
2021-09-02 15:08 ` Stefan Hajnoczi
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=YTC/savtIjlbzFsO@stefanha-x1.localdomain \
--to=stefanha@redhat.com \
--cc=cohuck@redhat.com \
--cc=jasowang@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=philmd@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=sgarzare@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 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).