netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nikolay Kichukov <nikolay@oldum.net>
To: Christian Schoenebeck <linux_oss@crudebyte.com>,
	v9fs-developer@lists.sourceforge.net
Cc: netdev@vger.kernel.org,
	Dominique Martinet <asmadeus@codewreck.org>,
	Eric Van Hensbergen <ericvh@gmail.com>,
	Latchesar Ionkov <lucho@ionkov.net>, Greg Kurz <groug@kaod.org>,
	Vivek Goyal <vgoyal@redhat.com>
Subject: Re: [PATCH v3 6/7] 9p/trans_virtio: support larger msize values
Date: Sat, 20 Nov 2021 12:20:35 +0100	[thread overview]
Message-ID: <cef6a6c6f8313a609ef104cc64ee6cf9d0ed6adb.camel@oldum.net> (raw)
In-Reply-To: <8119d4d93a39758075bb83730dcb571f5d071af6.1632327421.git.linux_oss@crudebyte.com>

[-- Attachment #1: Type: text/plain, Size: 5787 bytes --]

Thanks for the patches and sorry for top-posting.

I've tested them on GNU/Gentoo Linux, kernel 5.15.3 on amd64
architecture on both guest and KVM host.

The patches from this series, v3 have been applied to the host kernel
and also to the guest kernel. Guest kernel is clang compiled and host
kernel is compiled with gcc-11.

The host also received the qemu patches:
https://github.com/cschoenebeck/qemu/commit/04a7f9e55e0930b87805f7c97851eea4610e78fc
https://github.com/cschoenebeck/qemu/commit/b565bccb00afe8b73d529bbc3a38682996dac5c7
https://github.com/cschoenebeck/qemu/commit/669ced09b3b6070d478acce51810591b78ab0ccd

Qemu version on the host is 6.0.0-r54.

When the client mounts the share via virtio, requested msize is:
10485760 or 104857600

however the mount succeeds with:
msize=507904 in the end as per the /proc filesystem. This is less than
the previous maximum value.

However, performance-wise, I do see an improvement in throughput,
perhaps related to the qemu patches or some combination.

In addition to the above, when the kernel on the guest boots and loads
9pfs support, the attached memory allocation failure trace is generated.

Is anyone else seeing similar and was anybody able to get msize set to
10MB via virtio protocol with these patches?

Thank you,
-Nikolay

On Wed, 2021-09-22 at 18:00 +0200, Christian Schoenebeck wrote:
> The virtio transport supports by default a 9p 'msize' of up to
> approximately 500 kB. This patch adds support for larger 'msize'
> values by resizing the amount of scatter/gather lists if required.
> 
> Signed-off-by: Christian Schoenebeck <linux_oss@crudebyte.com>
> ---
>  net/9p/trans_virtio.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
> index e478a34326f1..147ebf647a95 100644
> --- a/net/9p/trans_virtio.c
> +++ b/net/9p/trans_virtio.c
> @@ -203,6 +203,31 @@ static struct virtqueue_sg *vq_sg_alloc(unsigned
> int nsgl)
>         return vq_sg;
>  }
>  
> +/**
> + * vq_sg_resize - resize passed virtqueue scatter/gather lists to the
> passed
> + * amount of lists
> + * @_vq_sg: scatter/gather lists to be resized
> + * @nsgl: new amount of scatter/gather lists
> + */
> +static int vq_sg_resize(struct virtqueue_sg **_vq_sg, unsigned int
> nsgl)
> +{
> +       struct virtqueue_sg *vq_sg;
> +
> +       BUG_ON(!_vq_sg || !nsgl);
> +       vq_sg = *_vq_sg;
> +       if (vq_sg->nsgl == nsgl)
> +               return 0;
> +
> +       /* lazy resize implementation for now */
> +       vq_sg = vq_sg_alloc(nsgl);
> +       if (!vq_sg)
> +               return -ENOMEM;
> +
> +       kfree(*_vq_sg);
> +       *_vq_sg = vq_sg;
> +       return 0;
> +}
> +
>  /**
>   * p9_virtio_close - reclaim resources of a channel
>   * @client: client instance
> @@ -774,6 +799,10 @@ p9_virtio_create(struct p9_client *client, const
> char *devname, char *args)
>         struct virtio_chan *chan;
>         int ret = -ENOENT;
>         int found = 0;
> +#if !defined(CONFIG_ARCH_NO_SG_CHAIN)
> +       size_t npages;
> +       size_t nsgl;
> +#endif
>  
>         if (devname == NULL)
>                 return -EINVAL;
> @@ -796,6 +825,38 @@ p9_virtio_create(struct p9_client *client, const
> char *devname, char *args)
>                 return ret;
>         }
>  
> +       /*
> +        * if user supplied an 'msize' option that's larger than what
> this
> +        * transport supports by default, then try to allocate more sg
> lists
> +        */
> +       if (client->msize > client->trans_maxsize) {
> +#ifdef CONFIG_ARCH_NO_SG_CHAIN
> +               pr_info("limiting 'msize' to %d because architecture
> does not "
> +                       "support chained scatter gather lists\n",
> +                       client->trans_maxsize);
> +#else
> +               npages = DIV_ROUND_UP(client->msize, PAGE_SIZE);
> +               if (npages > chan->p9_max_pages) {
> +                       npages = chan->p9_max_pages;
> +                       pr_info("limiting 'msize' as it would exceed the
> max. "
> +                               "of %lu pages allowed on this system\n",
> +                               chan->p9_max_pages);
> +               }
> +               nsgl = DIV_ROUND_UP(npages, SG_USER_PAGES_PER_LIST);
> +               if (nsgl > chan->vq_sg->nsgl) {
> +                       /*
> +                        * if resize fails, no big deal, then just
> +                        * continue with default msize instead
> +                        */
> +                       if (!vq_sg_resize(&chan->vq_sg, nsgl)) {
> +                               client->trans_maxsize =
> +                                       PAGE_SIZE *
> +                                       ((nsgl * SG_USER_PAGES_PER_LIST)
> - 3);
> +                       }
> +               }
> +#endif /* CONFIG_ARCH_NO_SG_CHAIN */
> +       }
> +
>         client->trans = (void *)chan;
>         client->status = Connected;
>         chan->client = client;


[-- Attachment #2: 9p-msize.txt --]
[-- Type: text/plain, Size: 3403 bytes --]

[    1.527981] 9p: Installing v9fs 9p2000 file system support
[    1.528173] ------------[ cut here ]------------
[    1.528174] WARNING: CPU: 1 PID: 791 at mm/page_alloc.c:5356 __alloc_pages+0x1ed/0x290
[    1.528179] Modules linked in: 9p 9pnet_virtio virtio_net net_failover failover virtio_console 9pnet virtio_balloon efivarfs
[    1.528182] CPU: 1 PID: 791 Comm: mount Not tainted 5.15.3-gentoo-x86_64 #1
[    1.528184] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
[    1.528185] RIP: 0010:__alloc_pages+0x1ed/0x290
[    1.528187] Code: ef 44 89 f6 e8 34 13 00 00 31 ed e9 5b ff ff ff 81 e3 3f ff ff ff 89 d9 89 cb 83 e3 f7 a9 00 00 00 10 0f 44 d9 e9 64 fe ff ff <0f> 0b 31 ed e9 42 ff ff ff 41 89 df 65 8b 05 08 65 9f 69 41 81 cf
[    1.528188] RSP: 0018:ffffb666405039e8 EFLAGS: 00010246
[    1.528189] RAX: c491259349f3ef00 RBX: 0000000000040c40 RCX: 0000000000000000
[    1.528190] RDX: 0000000000000000 RSI: 000000000000000c RDI: 0000000000040c40
[    1.528191] RBP: 000000000000000c R08: 0000000000000090 R09: fffffa3cc0111640
[    1.528192] R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000c40
[    1.528192] R13: ffff9fc944459c88 R14: 000000000000000c R15: 0000000000000c40
[    1.528194] FS:  00007ff620e57740(0000) GS:ffff9fc9bbc40000(0000) knlGS:0000000000000000
[    1.528195] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    1.528196] CR2: 00007fcfc14e8000 CR3: 0000000003358000 CR4: 0000000000350ea0
[    1.528198] Call Trace:
[    1.528201]  <TASK>
[    1.528202]  kmalloc_order+0x39/0xf0
[    1.528204]  kmalloc_order_trace+0x13/0x70
[    1.528205]  __kmalloc+0x1fc/0x2b0
[    1.528208]  p9_fcall_init+0x3d/0x60 [9pnet]
[    1.528210]  p9_client_prepare_req+0x82/0x2b0 [9pnet]
[    1.528212]  p9_client_rpc+0x80/0x350 [9pnet]
[    1.528214]  ? p9_virtio_create+0x253/0x2b0 [9pnet_virtio]
[    1.528216]  ? kfree+0x260/0x350
[    1.528217]  p9_client_version+0x60/0x1d0 [9pnet]
[    1.528219]  p9_client_create+0x3b4/0x460 [9pnet]
[    1.528221]  v9fs_session_init+0xab/0x760 [9p]
[    1.528222]  ? user_path_at_empty+0x7b/0x90
[    1.528224]  ? kmem_cache_alloc_trace+0x188/0x260
[    1.528226]  v9fs_mount+0x43/0x2d0 [9p]
[    1.528227]  legacy_get_tree.llvm.612377983374665620+0x22/0x40
[    1.528230]  vfs_get_tree+0x21/0xb0
[    1.528232]  path_mount+0x70d/0xcd0
[    1.528234]  __x64_sys_mount+0x148/0x1c0
[    1.528236]  do_syscall_64+0x4a/0xb0
[    1.528238]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[    1.528240] RIP: 0033:0x7ff620f92d5a
[    1.528241] Code: 48 8b 0d 11 c1 0b 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d de c0 0b 00 f7 d8 64 89 01 48
[    1.528242] RSP: 002b:00007ffe4675f9f8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
[    1.528244] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007ff620f92d5a
[    1.528244] RDX: 0000000000541f90 RSI: 0000000000541f40 RDI: 0000000000541f20
[    1.528245] RBP: 0000000000000c00 R08: 00000000005421f0 R09: 00007ffe4675e790
[    1.528246] R10: 0000000000000c00 R11: 0000000000000246 R12: 000000000053d4f0
[    1.528246] R13: 0000000000541f20 R14: 0000000000541f90 R15: 00007ff62109dcf4
[    1.528247]  </TASK>
[    1.528248] ---[ end trace 84f05b2aa35f19b3 ]---

[   90.894853] 9pnet: Limiting 'msize' to 507904 as this is the maximum supported by transport virtio

  reply	other threads:[~2021-11-20 11:20 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-22 16:00 [PATCH v3 0/7] net/9p: remove msize limit in virtio transport Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 1/7] net/9p: show error message if user 'msize' cannot be satisfied Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 2/7] 9p/trans_virtio: separate allocation of scatter gather list Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 3/7] 9p/trans_virtio: turn amount of sg lists into runtime info Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 4/7] 9p/trans_virtio: introduce struct virtqueue_sg Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 5/7] net/9p: add trans_maxsize to struct p9_client Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 6/7] 9p/trans_virtio: support larger msize values Christian Schoenebeck
2021-11-20 11:20   ` Nikolay Kichukov [this message]
2021-11-20 11:45     ` Dominique Martinet
2021-11-20 14:46       ` Christian Schoenebeck
2021-11-20 21:28         ` Nikolay Kichukov
2021-11-20 23:02         ` Dominique Martinet
2021-11-21 16:57           ` Christian Schoenebeck
2021-11-21 22:12             ` Dominique Martinet
2021-11-22 13:32               ` Christian Schoenebeck
2021-11-22 22:35                 ` Dominique Martinet
2021-11-24 12:22                   ` Christian Schoenebeck
2021-09-22 16:00 ` [PATCH v3 7/7] 9p/trans_virtio: resize sg lists to whatever is possible Christian Schoenebeck

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=cef6a6c6f8313a609ef104cc64ee6cf9d0ed6adb.camel@oldum.net \
    --to=nikolay@oldum.net \
    --cc=asmadeus@codewreck.org \
    --cc=ericvh@gmail.com \
    --cc=groug@kaod.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=netdev@vger.kernel.org \
    --cc=v9fs-developer@lists.sourceforge.net \
    --cc=vgoyal@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).