[LU-5051] HSM recv buffer reallocated unnecessarily

• Type: Improvement
• Resolution: Fixed
• Priority: Minor
• Fix Version/s: Lustre 2.6.0, Lustre 2.5.4
• Affects Version/s: Lustre 2.5.0, Lustre 2.6.0
• Labels: hsm patch
• Assignee: Cliff White (Inactive)
• Reporter: Patrick Farrell (Inactive)

Description

Currently, the 1 MB buffer used by llapi_hsm_copytool_recv is allocated and freed every time through the ct_run loop.

This is unnecessary, as long as the copy tool clears the the data before calling llapi_hsm_copytool_recv, which lhsmtool_posix already does.

This patch allocates the buffer once in llapi_hsm_copytool_register and frees it in unregister.

Patch will be available in Gerrit shortly.

Comments

[James Nunez (Inactive)]

Patch for b2_5 at http://review.whamcloud.com/#/c/10716

[Peter Jones]

Landed for 2.6

[Cliff White (Inactive)]

I will monitor this one, thanks for the patch.

[Patrick Farrell (Inactive)]

Patch is here:
http://review.whamcloud.com/#/c/10299/

Needs refresh, doing that shortly...

[Patrick Farrell (Inactive)]

I opened LU-5059 via Maloo for the test failures. (On all of the test runs, all of the sub tests passed, but the overall suite still reported failure...)

[Robert Read]

Yes, please change the label, in case you are still wondering about that.

As for the memset issue - Patrick mentioned that the copytool is currently clearing the data but I didn't see it doing that. Since this buffer was being managed by the API then it made sense to me to clear it there, but I agree it might not be required, either.

[Frank Zago (Inactive)]

The existing code is not memset'ing the buffer (using malloc, not calloc), and it belongs to the application, so there's no leak involved. Since it's also a very large buffer, I don't think we should add that.

[Patrick Farrell (Inactive)]

Good point, I can update the patch to do that. Before I do that, would you weigh in on my comment in the patch:
[...] In llapi_hsm_copytool_recv, now that we no longer free the buffer at the end of this function, should we rename "out_free" to "out_err"?

I'm probably going to make that change unless anyone disagrees.

[Robert Read]

Would it make sense fir llapi_hsm_copytool_recv() to reinitialize the buffer memset(0) each time rather than relying on the application to do this?