loader: tftp: Add preload method

[manu]

Summary

The preload method will transfer the whole file in a buffer and cache it
so read/lseek operations are faster.

MFC after: 2 weeks
Sponsored by: Beckhoff Automation GmbH & Co. KG

Test Plan

x time-base
+ time-tftp-prefetch
+-----------------------------------------------------------------------------------------------------------------------------+
|+                                                                                                                            |
|+                                                                                                                            |
|+                                                                                                                            |
|+                                                                                                                            |
|+                                                                                                                            |
|+                                                                                                                            |
|+                                                                                                                            |
|+                                                                                                                            |
|+                                                            x                                                               |
|+                                                            xx   x x x     x                    x x                        x|
|A                                                        |___________M________A_____________________|                        |
+-----------------------------------------------------------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x  10           104           193           116         128.9     29.856881
+  10            19            19            19            19             0
Difference at 95.0% confidence
	-109.9 +/- 19.8367
	-85.2599% +/- 2.26839%
	(Student's t, pooled s = 21.112)

[tsoome]

stand/libsa/tftp.c

669

malloc can fail.

672

Probably should be printed if TFTP_DEBUG is defined.

[tsoome]

I was starting to think.... we do have read ahead buffer (f_rabuf) in struct open_file, perhaps we could extend that interface to accommodate preload data?

[imp]

Core functionality looks good, have some edge case questions.

stand/libsa/tftp.c

673

We're not aborting, we're moving to uncached operations. The calls to preload() don't fail the whole transfer.

686

Do you need to invalidate and flush the cache when there's an error filling it? Maybe it's just as simple as freeing cache.
It might also be nice to have an error message here (pretty sure always, not just under debug, but I'm not 100% sure about that).

[manu]

stand/libsa/tftp.c

673

Well we are aborting preloading, but if you have better wording I'm ok to change it.

686

Yeah freeing the cache file we avoid leeking.
I'll add that and some error message too.

[imp]

stand/libsa/tftp.c

673

"Couldn't allocate %ju bytes for preload caching, disabling caching."

[manu]

Address @imp's comment.

[imp]

I like the new messages for failure.
One niggle wrt free, but either way I'm happy now.

stand/libsa/tftp.c

619

You don't need this if. free(NULL) works in the loader.

[manu]

stand/libsa/tftp.c

619

Fixed locally.
I'll push the series tomorow.

[mmel]

Unfortunately thiss broke tftp kernel loading.
The tftp_open() function will prefetch the first block and leave tftpfile->currblock set to 1, so the first block is never loaded into the cache.
The trivial fix is to reset it back to 0, it works for me but I'm not sure if that's the right solution.

stand/libsa/tftp.c

678

	tftpfile->currblock = 0;

[manu]

In D33410#760343, @mmel wrote:

Unfortunately thiss broke tftp kernel loading.

I'm obviously using tftp kernel loading and this whole series point was to improve it :)

The tftp_open() function will prefetch the first block and leave tftpfile->currblock set to 1, so the first block is never loaded into the cache.

Mhm, I can see that happening but I wonder why you hit this error while I don't ...

The trivial fix is to reset it back to 0, it works for me but I'm not sure if that's the right solution.

That would work yes but we will restart the transfer, I think that the correct fix should be to copy the data if currclock == 1 and reset to 0 only if currblock > 1 (as we don't have the other block anymore).