Re: [PATCH v3 6/7] 9p/trans_virtio: support larger msize values

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:

quoted

quoted

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