[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/1] 9pfs: log warning if msize <= 8192
From: |
Christian Schoenebeck |
Subject: |
Re: [PATCH v2 1/1] 9pfs: log warning if msize <= 8192 |
Date: |
Thu, 03 Sep 2020 18:32:54 +0200 |
On Donnerstag, 3. September 2020 18:02:48 CEST Greg Kurz wrote:
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 7bb994bbf2..99b6f24fd6 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -1353,6 +1353,15 @@ static void coroutine_fn v9fs_version(void *opaque)
> >
> > goto out;
> >
> > }
> >
> > + /* 8192 is the default msize of Linux clients */
> > + if (s->msize <= 8192) {
>
> I agree that an msize of 8192 suck from a performance standpoint but
> I guess many users are msize agnostic, and they use the default value
> without even knowing it. They might not even care for performance at
> all, otherwise they'd likely ask google and they will eventually land
> on:
>
> https://wiki.qemu.org/Documentation/9psetup#Performance_Considerations
>
> But with this patch, they will now get a warning each time they start
> QEMU, maybe freak out and file reports in launchpad. So I suggest you
> turn <= into < to avoid bothering these placid users with a warning.
Mmm, that's actually precisely my intended target group: people who have never
been aware about the existence of 'msize' before.
I keep '<=' for now, I think the log message is already clear that you simply
have to make it any 'msize' > 8192 and the warning is gone.
> Anyway, it's your choice :) so if you manage to get the #msize anchor to
> work as expected:
Works now (using raw html anchor instead):
https://wiki.qemu.org/Documentation/9psetup#msize
> Reviewed-by: Greg Kurz <groug@kaod.org>
Thanks Greg!
> > + warn_report_once(
> > + "9p: degraded performance: a reasonable high msize should be
> > "
> > + "chosen on client/guest side (chosen msize is <= 8192). See "
> > + "https://wiki.qemu.org/Documentation/9psetup#msize for
> > details." + );
> > + }
> > +
> >
> > marshal:
> > err = pdu_marshal(pdu, offset, "ds", s->msize, &version);
> > if (err < 0) {
Best regards,
Christian Schoenebeck