mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

9pfs: performance fix and cleanup

Browse Source 
* First patch fixes suboptimal I/O performance on guest due to previously
   incorrect block size being transmitted to 9p client.
 
 * Subsequent patches are cleanup ones intended to reduce code complexity.
 -----BEGIN PGP SIGNATURE-----
 
 iQJLBAABCgA1FiEEltjREM96+AhPiFkBNMK1h2Wkc5UFAmF5UZoXHHFlbXVfb3Nz
 QGNydWRlYnl0ZS5jb20ACgkQNMK1h2Wkc5XGSw//XdlI3Xj90uyoRKRgsMmnFOi6
 4q4dDiigC4ut2BAsMnJOBT3me4Dj+6/fipargxVWDaSrpaAfMKIGbRItiMsrM4sp
 3ILZEMF33lxF1ITw36pQlztaTccdZlFCbwQ/SrRphhX7AzDkDVuFfZvMxw6/FC8j
 yXv8fmKDr7BU5Q0uCiisx9hvTmzXgywPTDoRsJVrtcnwhD9EGl8Fx8RSBRElC8Zb
 c7+tvtSsSyEhqNyH9NdLB6IF/+7cSp8AU99pXwq9qEV2I3UmHS4TceYslHjuGkvg
 xaBHLcYYvV0wP3SNjv+bSRj3WDdrNvrkj35JutgR5NdhSlLxF1eRkk3F+1DzpOCW
 DXBRpxptMdOyX9nEu3+1tZ480AUIbxcHrf3HYUzMNBExnpS1JftpVyRu5gYOCXUd
 nPSnWTAMI9YwkmNag4Gm0mZdnNyO15PtFZ5lF+HjG6Nam/MCBJBB8rCGGuTQkGQf
 JwS1GrLvGjSFAgWF9Zm84aCX6f0Mzy84k+6vKVYk3jzicNmY9rdX+4x+e0ffHgol
 BxvL2i077ZYcu1NXXawaWmLmcA5V6VKcRzXbSrgWknMJ4xB6nK2B1mXpCjIzFVgl
 WDzZy6+rzk4tBbHRc1okmgc70gRk1EafYfg29Y2NvdGnNX+UUp5vpwUSqNpaYr2X
 cGyAuS+H7bxi//DRk+U=
 =2P3q
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/cschoenebeck/tags/pull-9p-20211027' into staging

9pfs: performance fix and cleanup

* First patch fixes suboptimal I/O performance on guest due to previously
  incorrect block size being transmitted to 9p client.

* Subsequent patches are cleanup ones intended to reduce code complexity.

* remotes/cschoenebeck/tags/pull-9p-20211027:
  9pfs: use P9Array in v9fs_walk()
  9pfs: make V9fsPath usable via P9Array API
  9pfs: make V9fsString usable via P9Array API
  fsdev/p9array.h: check scalar type in P9ARRAY_NEW()
  9pfs: introduce P9Array
  9pfs: simplify blksize_to_iounit()
  9pfs: deduplicate iounit code
  9pfs: fix wrong I/O block size in Rgetattr

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
This commit is contained in:
Richard Henderson 2021-10-27 11:45:11 -07:00
commit c52d69e7db
5 changed files with 208 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
fsdev/9p-marshal.c
View File

@ -18,6 +18,8 @@
#include "9p-marshal.h"
P9ARRAY_DEFINE_TYPE(V9fsString, v9fs_string_free);
void v9fs_string_free(V9fsString *str)
{
g_free(str->data);

3
fsdev/9p-marshal.h
View File

@ -1,10 +1,13 @@
#ifndef QEMU_9P_MARSHAL_H
#define QEMU_9P_MARSHAL_H
#include "p9array.h"
typedef struct V9fsString {
uint16_t size;
char *data;
} V9fsString;
P9ARRAY_DECLARE_TYPE(V9fsString);
typedef struct V9fsQID {
uint8_t type;

2
fsdev/file-op-9p.h
View File

@ -18,6 +18,7 @@
#include <utime.h>
#include <sys/vfs.h>
#include "qemu-fsdev-throttle.h"
#include "p9array.h"
#define SM_LOCAL_MODE_BITS 0600
#define SM_LOCAL_DIR_MODE_BITS 0700
@ -105,6 +106,7 @@ struct V9fsPath {
uint16_t size;
char *data;
};
P9ARRAY_DECLARE_TYPE(V9fsPath);
typedef union V9fsFidOpenState V9fsFidOpenState;

160
fsdev/p9array.h Normal file
View File

@ -0,0 +1,160 @@
/*
* P9Array - deep auto free C-array
*
* Copyright (c) 2021 Crudebyte
*
* Authors:
* Christian Schoenebeck <qemu_oss@crudebyte.com>
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
* THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
#ifndef QEMU_P9ARRAY_H
#define QEMU_P9ARRAY_H
#include "qemu/compiler.h"
/**
* P9Array provides a mechanism to access arrays in common C-style (e.g. by
* square bracket [] operator) in conjunction with reference variables that
* perform deep auto free of the array when leaving the scope of the auto
* reference variable. That means not only is the array itself automatically
* freed, but also memory dynamically allocated by the individual array
* elements.
*
* Example:
*
* Consider the following user struct @c Foo which shall be used as scalar
* (element) type of an array:
* @code
* typedef struct Foo {
* int i;
* char *s;
* } Foo;
* @endcode
* and assume it has the following function to free memory allocated by @c Foo
* instances:
* @code
* void free_foo(Foo *foo) {
* free(foo->s);
* }
* @endcode
* Add the following to a shared header file:
* @code
* P9ARRAY_DECLARE_TYPE(Foo);
* @endcode
* and the following to a C unit file:
* @code
* P9ARRAY_DEFINE_TYPE(Foo, free_foo);
* @endcode
* Finally the array may then be used like this:
* @code
* void doSomething(size_t n) {
* P9ARRAY_REF(Foo) foos = NULL;
* P9ARRAY_NEW(Foo, foos, n);
* for (size_t i = 0; i < n; ++i) {
* foos[i].i = i;
* foos[i].s = calloc(4096, 1);
* snprintf(foos[i].s, 4096, "foo %d", i);
* if (...) {
* return; // array auto freed here
* }
* }
* // array auto freed here
* }
* @endcode
*/
/**
* Declares an array type for the passed @a scalar_type.
*
* This is typically used from a shared header file.
*
* @param scalar_type - type of the individual array elements
*/
#define P9ARRAY_DECLARE_TYPE(scalar_type) \
typedef struct P9Array##scalar_type { \
size_t len; \
scalar_type first[]; \
} P9Array##scalar_type; \
\
void p9array_new_##scalar_type(scalar_type **auto_var, size_t len); \
void p9array_auto_free_##scalar_type(scalar_type **auto_var); \
/**
* Defines an array type for the passed @a scalar_type and appropriate
* @a scalar_cleanup_func.
*
* This is typically used from a C unit file.
*
* @param scalar_type - type of the individual array elements
* @param scalar_cleanup_func - appropriate function to free memory dynamically
* allocated by individual array elements before
*/
#define P9ARRAY_DEFINE_TYPE(scalar_type, scalar_cleanup_func) \
void p9array_new_##scalar_type(scalar_type **auto_var, size_t len) \
{ \
p9array_auto_free_##scalar_type(auto_var); \
P9Array##scalar_type *arr = g_malloc0(sizeof(P9Array##scalar_type) + \
len * sizeof(scalar_type)); \
arr->len = len; \
*auto_var = &arr->first[0]; \
} \
\
void p9array_auto_free_##scalar_type(scalar_type **auto_var) \
{ \
scalar_type *first = (*auto_var); \
if (!first) { \
return; \
} \
P9Array##scalar_type *arr = (P9Array##scalar_type *) ( \
((char *)first) - offsetof(P9Array##scalar_type, first) \
); \
for (size_t i = 0; i < arr->len; ++i) { \
scalar_cleanup_func(&arr->first[i]); \
} \
g_free(arr); \
} \
/**
* Used to declare a reference variable (unique pointer) for an array. After
* leaving the scope of the reference variable, the associated array is
* automatically freed.
*
* @param scalar_type - type of the individual array elements
*/
#define P9ARRAY_REF(scalar_type) \
__attribute((__cleanup__(p9array_auto_free_##scalar_type))) scalar_type*
/**
* Allocates a new array of passed @a scalar_type with @a len number of array
* elements and assigns the created array to the reference variable
* @a auto_var.
*
* @param scalar_type - type of the individual array elements
* @param auto_var - destination reference variable
* @param len - amount of array elements to be allocated immediately
*/
#define P9ARRAY_NEW(scalar_type, auto_var, len) \
QEMU_BUILD_BUG_MSG( \
!__builtin_types_compatible_p(scalar_type, typeof(*auto_var)), \
"P9Array scalar type mismatch" \
); \
p9array_new_##scalar_type((&auto_var), len)
#endif /* QEMU_P9ARRAY_H */

70
hw/9pfs/9p.c
View File

@ -50,6 +50,8 @@ enum {
Oappend = 0x80,
};
P9ARRAY_DEFINE_TYPE(V9fsPath, v9fs_path_free);
static ssize_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
{
ssize_t ret;
@ -1262,6 +1264,37 @@ static int coroutine_fn stat_to_v9stat(V9fsPDU *pdu, V9fsPath *path,
#define P9_STATS_ALL 0x00003fffULL /* Mask for All fields above */
/**
* Convert host filesystem's block size into an appropriate block size for
* 9p client (guest OS side). The value returned suggests an "optimum" block
* size for 9p I/O, i.e. to maximize performance.
*
* @pdu: 9p client request
* @blksize: host filesystem's block size
*/
static int32_t blksize_to_iounit(const V9fsPDU *pdu, int32_t blksize)
{
int32_t iounit = 0;
V9fsState *s = pdu->s;
/*
* iounit should be multiples of blksize (host filesystem block size)
* as well as less than (client msize - P9_IOHDRSZ)
*/
if (blksize) {
iounit = QEMU_ALIGN_DOWN(s->msize - P9_IOHDRSZ, blksize);
}
if (!iounit) {
iounit = s->msize - P9_IOHDRSZ;
}
return iounit;
}
static int32_t stat_to_iounit(const V9fsPDU *pdu, const struct stat *stbuf)
{
return blksize_to_iounit(pdu, stbuf->st_blksize);
}
static int stat_to_v9stat_dotl(V9fsPDU *pdu, const struct stat *stbuf,
V9fsStatDotl *v9lstat)
{
@ -1273,7 +1306,7 @@ static int stat_to_v9stat_dotl(V9fsPDU *pdu, const struct stat *stbuf,
v9lstat->st_gid = stbuf->st_gid;
v9lstat->st_rdev = stbuf->st_rdev;
v9lstat->st_size = stbuf->st_size;
v9lstat->st_blksize = stbuf->st_blksize;
v9lstat->st_blksize = stat_to_iounit(pdu, stbuf);
v9lstat->st_blocks = stbuf->st_blocks;
v9lstat->st_atime_sec = stbuf->st_atime;
v9lstat->st_atime_nsec = stbuf->st_atim.tv_nsec;
@ -1705,13 +1738,14 @@ static void coroutine_fn v9fs_walk(void *opaque)
int name_idx;
g_autofree V9fsQID *qids = NULL;
int i, err = 0;
V9fsPath dpath, path, *pathes = NULL;
V9fsPath dpath, path;
P9ARRAY_REF(V9fsPath) pathes = NULL;
uint16_t nwnames;
struct stat stbuf, fidst;
g_autofree struct stat *stbufs = NULL;
size_t offset = 7;
int32_t fid, newfid;
V9fsString *wnames = NULL;
P9ARRAY_REF(V9fsString) wnames = NULL;
V9fsFidState *fidp;
V9fsFidState *newfidp = NULL;
V9fsPDU *pdu = opaque;
@ -1732,10 +1766,10 @@ static void coroutine_fn v9fs_walk(void *opaque)
goto out_nofid;
}
if (nwnames) {
wnames = g_new0(V9fsString, nwnames);
P9ARRAY_NEW(V9fsString, wnames, nwnames);
qids = g_new0(V9fsQID, nwnames);
stbufs = g_new0(struct stat, nwnames);
pathes = g_new0(V9fsPath, nwnames);
P9ARRAY_NEW(V9fsPath, pathes, nwnames);
for (i = 0; i < nwnames; i++) {
err = pdu_unmarshal(pdu, offset, "s", &wnames[i]);
if (err < 0) {
@ -1867,36 +1901,14 @@ out:
v9fs_path_free(&path);
out_nofid:
pdu_complete(pdu, err);
if (nwnames && nwnames <= P9_MAXWELEM) {
for (name_idx = 0; name_idx < nwnames; name_idx++) {
v9fs_string_free(&wnames[name_idx]);
v9fs_path_free(&pathes[name_idx]);
}
g_free(wnames);
g_free(pathes);
}
}
static int32_t coroutine_fn get_iounit(V9fsPDU *pdu, V9fsPath *path)
{
struct statfs stbuf;
int32_t iounit = 0;
V9fsState *s = pdu->s;
int err = v9fs_co_statfs(pdu, path, &stbuf);
/*
* iounit should be multiples of f_bsize (host filesystem block size
* and as well as less than (client msize - P9_IOHDRSZ))
*/
if (!v9fs_co_statfs(pdu, path, &stbuf)) {
if (stbuf.f_bsize) {
iounit = stbuf.f_bsize;
iounit *= (s->msize - P9_IOHDRSZ) / stbuf.f_bsize;
}
}
if (!iounit) {
iounit = s->msize - P9_IOHDRSZ;
}
return iounit;
return blksize_to_iounit(pdu, (err >= 0) ? stbuf.f_bsize : 0);
}
static void coroutine_fn v9fs_open(void *opaque)