[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Virtio-fs] [PATCH 07/24] DAX: virtio-fs: Add vhost-user slave comma
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Virtio-fs] [PATCH 07/24] DAX: virtio-fs: Add vhost-user slave commands for mapping |
Date: |
Mon, 15 Feb 2021 13:25:13 +0000 |
User-agent: |
Mutt/2.0.5 (2021-01-21) |
* Chirantan Ekbote (chirantan@chromium.org) wrote:
> On Wed, Feb 10, 2021 at 4:04 AM Dr. David Alan Gilbert (git)
> <dgilbert@redhat.com> wrote:
> >
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > +
> > +typedef struct {
> > + /* Offsets within the file being mapped */
> > + uint64_t fd_offset[VHOST_USER_FS_SLAVE_ENTRIES];
> > + /* Offsets within the cache */
> > + uint64_t c_offset[VHOST_USER_FS_SLAVE_ENTRIES];
> > + /* Lengths of sections */
> > + uint64_t len[VHOST_USER_FS_SLAVE_ENTRIES];
> > + /* Flags, from VHOST_USER_FS_FLAG_* */
> > + uint64_t flags[VHOST_USER_FS_SLAVE_ENTRIES];
> > +} VhostUserFSSlaveMsg;
> > +
>
> Is it too late to change this?
No; this is a message defined as part of this series so still up for
review. It's not guest visible; just on the vhist-user pipe.
> This struct allocates space for up to
> 8 entries but most of the time the server will only try to set up one
> mapping at a time so only 32 out of the 256 bytes in the message are
> actually being used. We're just wasting time memcpy'ing bytes that
> will never be used. Is there some reason this can't be dynamically
> sized? Something like:
>
> typedef struct {
> /* Number of mapping requests */
> uint16_t num_requests;
> /* `num_requests` mapping requests */
> MappingRequest requests[];
> } VhostUserFSSlaveMsg;
>
> typedef struct {
> /* Offset within the file being mapped */
> uint64_t fd_offset;
> /* Offset within the cache */
> uint64_t c_offset;
> /* Length of section */
> uint64_t len;
> /* Flags, from VHOST_USER_FS_FLAG_* */
> uint64_t flags;
> } MappingRequest;
>
> The current pre-allocated structure both wastes space when there are
> fewer than 8 requests and requires extra messages to be sent when
> there are more than 8 requests. I realize that in the grand scheme of
> things copying 224 extra bytes is basically not noticeable but it just
> irks me that we could fix this really easily before it gets propagated
> to too many other places.
Sure; I'll have a look. I think at the moment the only
more-than-one-entry case is the remove mapping case.
Dave
> Chirantan
>
> > --
> > 2.29.2
> >
> > _______________________________________________
> > Virtio-fs mailing list
> > Virtio-fs@redhat.com
> > https://www.redhat.com/mailman/listinfo/virtio-fs
> >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
- Re: [PATCH 03/24] DAX: libvhost-user: Allow popping a queue element with bad pointers, (continued)
- [PATCH 04/24] DAX subprojects/libvhost-user: Add virtio-fs slave types, Dr. David Alan Gilbert (git), 2021/02/09
- [PATCH 05/24] DAX: virtio: Add shared memory capability, Dr. David Alan Gilbert (git), 2021/02/09
- [PATCH 06/24] DAX: virtio-fs: Add cache BAR, Dr. David Alan Gilbert (git), 2021/02/09
- [PATCH 07/24] DAX: virtio-fs: Add vhost-user slave commands for mapping, Dr. David Alan Gilbert (git), 2021/02/09
- [PATCH 08/24] DAX: virtio-fs: Fill in slave commands for mapping, Dr. David Alan Gilbert (git), 2021/02/09
- [PATCH 09/24] DAX: virtiofsd Add cache accessor functions, Dr. David Alan Gilbert (git), 2021/02/09
- [PATCH 10/24] DAX: virtiofsd: Add setup/remove mappings fuse commands, Dr. David Alan Gilbert (git), 2021/02/09