Skip to content

FreeBSD: avoid memory allocation in arc_prune_async #11927

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

mjguzik
Copy link
Contributor

@mjguzik mjguzik commented Apr 22, 2021

Signed-off-by: Mateusz Guzik [email protected]

Motivation and Context

Description

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Apr 22, 2021
Copy link
Member

@amotin amotin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no objections about the idea, but have few about implementation.

int64_t nr_scan = (int64_t)arg;
#else
int64_t nr_scan = (int32_t)arg;
#endif
Copy link
Member

@amotin amotin Apr 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not intptr_t?

@@ -185,13 +188,14 @@ arc_prune_task(void *arg)
void
arc_prune_async(int64_t adjust)
{

int64_t *adjustptr;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd make it void * or remove completely.


*adjustptr = adjust;
adjustptr = (void *)adjust;
Copy link
Member

@amotin amotin Apr 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better be brought to intptr_t first.

@behlendorf behlendorf added Status: Code Review Needed Ready for review and testing and removed Status: Accepted Ready to integrate (reviewed, tested) labels Apr 22, 2021
@behlendorf behlendorf added the Status: Revision Needed Changes are required for the PR to be accepted label Apr 29, 2021
@behlendorf
Copy link
Contributor

@mjguzik when you get a chance can you address the review feedback in this PR.

@tonynguien tonynguien requested a review from behlendorf May 24, 2021 20:46
@behlendorf behlendorf removed the Status: Code Review Needed Ready for review and testing label May 24, 2021
@behlendorf
Copy link
Contributor

@tonynguien let's go ahead and move forward with the updated version of this in PR #12049.

@amotin
Copy link
Member

amotin commented May 25, 2021

Mateusz just today answered me in private email that he does not care which of the versions is committed.

@behlendorf behlendorf closed this May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Revision Needed Changes are required for the PR to be accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants