> -----Original Message----- > From: Stephen Hemminger <[email protected]> > Sent: 28 March 2023 21:22 > To: Srikanth Yalavarthi <[email protected]> > Cc: Anup Prabhu <[email protected]>; [email protected]; Shivah Shankar > Shankar Narayan Rao <[email protected]>; Prince Takkar > <[email protected]>; Srikanth Yalavarthi <[email protected]> > 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 <[email protected]> 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.
