On (21/09/24 09:11), Brian Geffon wrote:
[..]
Some silly nits:
> +static void mark_idle(struct zram *zram, ktime_t cutoff)
> {
> - struct zram *zram = dev_to_zram(dev);
> + int is_idle = 1;
> unsigned long nr_pages = zram->disksize >> PAGE_SHIFT;
> int index;
>
> - if (!sysfs_streq(buf, "all"))
> - return -EINVAL;
> -
> - down_read(&zram->init_lock);
> - if (!init_done(zram)) {
> - up_read(&zram->init_lock);
> - return -EINVAL;
> - }
> -
> for (index = 0; index < nr_pages; index++) {
> /*
> * Do not mark ZRAM_UNDER_WB slot as ZRAM_IDLE to close race.
> @@ -314,14 +308,49 @@ static ssize_t idle_store(struct device *dev,
> */
> zram_slot_lock(zram, index);
> if (zram_allocated(zram, index) &&
> - !zram_test_flag(zram, index, ZRAM_UNDER_WB))
> - zram_set_flag(zram, index, ZRAM_IDLE);
> + !zram_test_flag(zram, index, ZRAM_UNDER_WB)) {
> +#ifdef CONFIG_ZRAM_MEMORY_TRACKING
> + is_idle = (!cutoff || ktime_after(cutoff, zram->table[index].ac_time));
checkpatch "WARNING: suspect code indent for conditional statements (16, 32)"
Looks like `is_idle` is at one extra indent level.
> +#endif
> + if (is_idle)
> + zram_set_flag(zram, index, ZRAM_IDLE);
> + }
> zram_slot_unlock(zram, index);
> }
> +}
>
> - up_read(&zram->init_lock);
> +static ssize_t idle_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t len)
> +{
> + struct zram *zram = dev_to_zram(dev);
> + ktime_t cutoff_time = 0;
> + ssize_t rv = -EINVAL;
>
> - return len;
> + if (!sysfs_streq(buf, "all")) {
> + /*
> + * If it did not parse as 'all' try to treat it as an integer when
> + * we have memory tracking enabled.
> + */
> + u64 age_sec;
checkpatch "WARNING: Missing a blank line after declarations"
> + if (IS_ENABLED(CONFIG_ZRAM_MEMORY_TRACKING) && !kstrtoull(buf, 0, &age_sec))
> + cutoff_time = ktime_sub(ktime_get_boottime(),
> + ns_to_ktime(age_sec * NSEC_PER_SEC));
> + else
> + goto out;
> + }
> +
> + down_read(&zram->init_lock);
> + if (!init_done(zram))
> + goto out_unlock;
> +
> + /* A age_sec of 0 marks everything as idle, this is the "all" behavior */
s/age_sec/cutoff_time/
> + mark_idle(zram, cutoff_time);
> + rv = len;
