On Sun, Sep 20, 2026 at 6:36 AM Barry Song <[email protected]> wrote: [...] > > > Related to the previous comment, you treat the special values as part of > > > the range. It works currently, but creates a layout dependency on the > > > enum. I think it would be more future-proof if you did: > > > > > > if (swappiness != -1 && swappiness != SWAPPINESS_ANON_ONLY) { > > > if (swappiness < MIN_SWAPPINESS || swappiness > MAX_SWAPPINESS) > > > return -EINVAL; > > > } > > > > > > The previous comments I brought up are now resolved, so assuming you'll > > > make the changes above you can include: > > > > > > Reviewed-by: JP Kobryn <[email protected]> > > > > Hui, please make these changes and just send this patch in next version. > > Hi JP and Shakeel, > > This feels a bit weird to me, since 0–201 are all valid > swappiness values. > > 201 (`SWAPPINESS_ANON_ONLY`) is just as valid a swappiness value > as 200. In contrast, -1 is a different story, as it indicates > completely different semantics. > > If 201 is treated as a special value, then 0 (`MIN_SWAPPINESS`) > should be treated the same way for MGLRU, since 0 means file-only > reclaim there. This isn't the case for CLRU, where swappiness 0 > can still reclaim anon folios. > > If possible, I'd vote for keeping Hui's current code :-)
On second thought, you might be right, as there could theoretically be a case where `SWAPPINESS_ANON_ONLY` is not defined as `MAX_SWAPPINESS + 1`, but as, for example, 400 or some other value. That said, I find `MAX_SWAPPINESS` and `SWAPPINESS_ANON_ONLY` really weird. Somehow, I feel `MAX_SWAPPINESS` should be `SWAPPINESS_ANON_ONLY`, since 201 is actually the maximum valid swappiness value, rather than 200. For the change suggested by JP, I think it is right if we don't clean up `MAX_SWAPPINESS` and `SWAPPINESS_ANON_ONLY` properly. With the change suggested by JP, Reviewed-by: Barry Song <[email protected]>
