Support post-3.12 shrinker semantics #403
Conversation
| @@ -199,4 +199,8 @@ fn ## _scan_objects(struct shrinker *shrink, struct shrink_control *sc) \ | |||
| #error "Unknown shrinker callback" | |||
| #endif | |||
|
|
|||
| #if !defined(HAVE_SPLIT_SHRINKER_CALLBACK) | |||
There was a problem hiding this comment.
#if !defined(SHRINK_STOP)?
There was a problem hiding this comment.
I went back and forth on this. Functionally it's not going to matter because SHRINK_STOP was added in the same commit the shrinker callbacks were split. In then end I thought this would be a little more consistent. Can you make a case for why #if defined(SHRINK_STOP) is better?
There was a problem hiding this comment.
I was pretty sure that SHRINK_STOP was added in the new shrinker API, but I was considering the perspective of someone examining the code without that prior knowledge. I'm more accustomed to seeing this idiom:
#ifndef SOMETHING
#define SOMETHING ...
#endif
For example, in sysmacros.h, we see:
#ifndef howmany
#define howmany(x, y) (((x) + ((y) - 1)) / (y))
#endif
as opposed to "#ifndef something_else_not_necessarily_directly_tied_to_howmany".
To me, as a code reviewer, that idiom implies the preprocessor token is defined in some versions of the surrounding environment but not in others and that in environment where it's not implicitly defined, we want to give it our own particular definition.
That said, I've got no strong feelings on this type of idiom one way or another and it is certainly more consistent with the rest of the code (in spl-kmem.c) to use #if !defined(HAVE_SPLIT_SHRINKER_CALLBACK).
[Sorry for the too-long comment for something which was an off-the-cuff thought earlier today.]
|
@behlendorf Other than the couple of notes I added, LGTM. |
|
@dweeezil refreshed. |
|
@behlendorf Refreshed my branch to match "shrinker" in your repository and updated shrinker in splat to use the new semantics. |
The new shrinker API as of Linux 3.12 modifies "struct shrinker" by replacing the @shrink callback with the pair of @count_objects and @scan_objects. It also requires the return value of @count_objects to return the number of objects actually freed whereas the previous @shrink callback returned the number of remaining freeable objects. This patch adds support for the new @scan_objects return value semantics and updates the splat shrinker test case appropriately. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
|
@behlendorf behlendorf/spl@f9757c6 LGTM. Thanks for fixing this up. I'll look at your version of the ARC shrinker patch in a moment. |
The new shrinker API as of Linux 3.12 modifies "struct shrinker" by replacing the @shrink callback with the pair of @count_objects and @scan_objects. It also requires the return value of @count_objects to return the number of objects actually freed whereas the previous @shrink callback returned the number of remaining freeable objects. This patch adds support for the new @scan_objects return value semantics and updates the splat shrinker test case appropriately. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Tim Chase <tim@chase2k.com> Closes openzfs#403 Conflicts: module/spl/spl-kmem.c module/splat/splat-linux.c
The new shrinker API as of Linux 3.12 modifies "struct shrinker" by replacing the @shrink callback with the pair of @count_objects and @scan_objects. It also requires the return value of @count_objects to return the number of objects actually freed whereas the previous @shrink callback returned the number of remaining freeable objects. This patch adds support for the new @scan_objects return value semantics and updates the splat shrinker test case appropriately. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Tim Chase <tim@chase2k.com> Closes #403
The new shrinker API as of Linux 3.12 modifies "struct shrinker" by replacing the @shrink callback with the pair of @count_objects and @scan_objects. It also requires the return value of @count_objects to return the number of objects actually freed whereas the previous @shrink callback returned the number of remaining freeable objects. This patch adds support for the new @scan_objects return value semantics and updates the splat shrinker test case appropriately. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Tim Chase <tim@chase2k.com> Closes openzfs#403 Conflicts: module/spl/spl-kmem.c module/splat/splat-linux.c
The new shrinker API as of Linux 3.12 modifies "struct shrinker" by
replacing the @shrink callback with the pair of @count_objects and
@scan_objects. It also requires the return value of @count_objects to
return the number of objects actually freed whereas the previous @shrink
callback returned the number of remaining freeable objects.
This patch adds support for the new @scan_objects return value semantics.
References:
torvalds/linux@24f7c6b9