Re: [igt-dev] [PATCH i-g-t 1/6] runner: Use a bigger buffer for receiving test outputs

[Petri Latvala <petri.latvala@intel.com>]

On Mon, Nov 07, 2022 at 05:45:26PM +0100, Kamil Konieczny wrote:
> Hi Petri,
> 
> On 2022-11-07 at 14:01:46 +0200, Petri Latvala wrote:
> > Stream-based receiving was able to use buffers of any size for read(),
> > but with datagrams we actually need to prepare for actual sizes. In
> > practice, some file dumps from tests come as single log packets of a
> > couple of kB.
> > 
> > Signed-off-by: Petri Latvala <petri.latvala@intel.com>
> > Cc: Arkadiusz Hiler <arek@hiler.eu>
> > Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > ---
> >  runner/executor.c | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/runner/executor.c b/runner/executor.c
> > index a79a34e0..1fcc9afe 100644
> > --- a/runner/executor.c
> > +++ b/runner/executor.c
> > @@ -857,7 +857,8 @@ static int monitor_output(pid_t child,
> >  			  char **abortreason)
> >  {
> >  	fd_set set;
> > -	char buf[2048];
> > +	char *buf;
> > +	size_t bufsize;
> >  	char *outbuf = NULL;
> >  	size_t outbufsize = 0;
> >  	char current_subtest[256] = {};
> > @@ -910,6 +911,9 @@ static int monitor_output(pid_t child,
> >  		}
> >  	}
> >  
> > +	bufsize = 262144; /* 256 kiB */
> 
> imho it is better to use define and also write this as 256*1024
> and put it in header.

Sure.

> If you increase this size maybe you can drop splitting ?

Splitting is only done for log packets. Currently there's no obvious
way for other packet types to be larger than reasonable but for
futureproofing it's good to be defensive in both ends.

Having e.g. assertion failure reason fields (when that is stuffed into
the subtest result packet) go multiple kilobytes sounds wild but who
knows what kind of autogenerated igt_assert macros people will attempt
to write...


-- 
Petri Latvala