[EXT] Re: [PATCH 1/1] app/mldev: add internal function for file read

[Srikanth Yalavarthi]

> -----Original Message-----
> From: Stephen Hemminger <stephen at networkplumber.org>
> Sent: 28 March 2023 21:22
> To: Srikanth Yalavarthi <syalavarthi at marvell.com>
> Cc: Anup Prabhu <aprabhu at marvell.com>; dev at dpdk.org; Shivah Shankar
> Shankar Narayan Rao <sshankarnara at marvell.com>; Prince Takkar
> <ptakkar at marvell.com>; Srikanth Yalavarthi <syalavarthi at marvell.com>
> Subject: [EXT] Re: [PATCH 1/1] app/mldev: add internal function for file read
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Thu, 23 Mar 2023 08:28:01 -0700
> Srikanth Yalavarthi <syalavarthi at marvell.com> wrote:
> 
> > +	if (fseek(fp, 0, SEEK_END) == 0) {
> > +		file_size = ftell(fp);
> > +		if (file_size == -1) {
> > +			ret = -EIO;
> > +			goto error;
> > +		}
> > +
> > +		file_buffer = rte_malloc(NULL, file_size,
> RTE_CACHE_LINE_SIZE);
> > +		if (file_buffer == NULL) {
> > +			ml_err("Failed to allocate memory: %s\n", file);
> > +			ret = -ENOMEM;
> > +			goto error;
> > +		}
> > +
> > +		if (fseek(fp, 0, SEEK_SET) != 0) {
> > +			ret = -EIO;
> > +			goto error;
> > +		}
> > +
> > +		if (fread(file_buffer, sizeof(char), file_size, fp) != (unsigned
> long)file_size) {
> > +			ml_err("Failed to read file : %s\n", file);
> > +			ret = -EIO;
> > +			goto error;
> > +		}
> > +		fclose(fp);
> > +	} else {
> > +		ret = -EIO;
> > +		goto error;
> > +	}
> > +
> > +	*buffer = file_buffer;
> > +	*size = file_size;
> > +
> > +	return 0;
> 
> Granted this only test code, but is the slowest way to do this.
> Stdio is buffered (in 4K chunks). And using rte_malloc comes from
> hugepages.
> 
> Three levels of improvement are possible:
>   1. don't use rte_malloc() use malloc() instead.
Agree on this. Will update in next version.

>   2. use direct system call for I/O
>   3. use mmap() to directly map in the file instead read
Agree on the improvements.
But, considering that this is a test code and these operations are done in slow-path, I would prefer to have the implementation based on C library calls rather than using system calls.

Also, using system calls may not make this code portable? Though we are not supporting this app on platforms other than Linux, as of now.
Pls let me know what you think.