netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Schoenebeck <linux_oss@crudebyte.com>
To: Dominique Martinet <asmadeus@codewreck.org>
Cc: Nikolay Kichukov <nikolay@oldum.net>,
	v9fs-developer@lists.sourceforge.net, netdev@vger.kernel.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: Mon, 22 Nov 2021 14:32:23 +0100	[thread overview]
Message-ID: <1797352.eH9cFvQebf@silver> (raw)
In-Reply-To: <YZrEPj9WLx36Pm3k@codewreck.org>

On Sonntag, 21. November 2021 23:12:14 CET Dominique Martinet wrote:
> Christian Schoenebeck wrote on Sun, Nov 21, 2021 at 05:57:30PM +0100:
> > > Although frankly as I said if we're going to do this, we actual can
> > > majorate the actual max for all operations pretty easily thanks to the
> > > count parameter -- I guess it's a bit more work but we can put arbitrary
> > > values (e.g. 8k for all the small stuff) instead of trying to figure it
> > > out more precisely; I'd just like the code path to be able to do it so
> > > we only do that rechurn once.
> > 
> > Looks like we had a similar idea on this. My plan was something like this:
> > 
> > static int max_msg_size(enum msg_type) {
> > 
> >     switch (msg_type) {
> >     
> >         /* large zero copy messages */
> >         case Twrite:
> >         case Tread:
> >         
> >         case Treaddir:
> >             BUG_ON(true);
> >         
> >         /* small messages */
> >         case Tversion:
> >         ....
> >         
> >             return 8k; /* to be replaced with appropriate max value */
> >     
> >     }
> > 
> > }
> > 
> > That would be a quick start and allow to fine grade in future. It would
> > also provide a safety net, e.g. the compiler would bark if a new message
> > type is added in future.
> 
> I assume that'd only be used if the caller does not set an explicit
> limit, at which point we're basically using a constant and the function
> coud be replaced by a P9_SMALL_MSG_SIZE constant... But yes, I agree
> with the idea, it's these three calls that deal with big buffers in
> either emission or reception (might as well not allocate a 128MB send
> buffer for Tread ;))

I "think" this could be used for all 9p message types except for the listed 
former three, but I'll review the 9p specs more carefully before v4. For Tread 
and Twrite we already received the requested size, which just leaves Treaddir, 
which is however indeed tricky, because I don't think we have any info how 
many directory entries we could expect.

A simple compile time constant (e.g. one macro) could be used instead of this 
function. If you prefer a constant instead, I could go for it in v4 of course. 
For this 9p client I would recommend a function though, simply because this 
code has already seen some authors come and go over the years, so it might be 
worth the redundant code for future safety. But I'll adapt to what others 
think.

Greg, opinion?

Best regards,
Christian Schoenebeck



  reply	other threads:[~2021-11-22 13:32 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
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 [this message]
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=1797352.eH9cFvQebf@silver \
    --to=linux_oss@crudebyte.com \
    --cc=asmadeus@codewreck.org \
    --cc=ericvh@gmail.com \
    --cc=groug@kaod.org \
    --cc=lucho@ionkov.net \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@oldum.net \
    --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).