Messages in this thread | | | Date | Fri, 8 Apr 2022 09:43:03 -0400 | From | Dan Schatzberg <> | Subject | Re: [PATCH v3 1/4] memcg: introduce per-memcg reclaim interface |
| |
On Fri, Apr 08, 2022 at 04:57:40AM +0000, Yosry Ahmed wrote: > +static ssize_t memory_reclaim(struct kernfs_open_file *of, char *buf, > + size_t nbytes, loff_t off) > +{ > + struct mem_cgroup *memcg = mem_cgroup_from_css(of_css(of)); > + unsigned int nr_retries = MAX_RECLAIM_RETRIES; > + unsigned long nr_to_reclaim, nr_reclaimed = 0; > + int err; > + > + buf = strstrip(buf); > + err = page_counter_memparse(buf, "", &nr_to_reclaim);
Is there a reason not to support "max"? Empty string seems odd to me here.
> + if (err) > + return err; > + > + while (nr_reclaimed < nr_to_reclaim) { > + unsigned long reclaimed; > + > + if (signal_pending(current)) > + break;
I think this should be `return -EINTR;`
> + > + reclaimed = try_to_free_mem_cgroup_pages(memcg, > + nr_to_reclaim - nr_reclaimed, > + GFP_KERNEL, true); > + > + if (!reclaimed && !nr_retries--) > + break;
Here you can just `return -EAGAIN;`
> + > + nr_reclaimed += reclaimed; > + } > + > + return nr_reclaimed < nr_to_reclaim ? -EAGAIN : nbytes;
Then this can just be `return nbytes;`
I'm very much in favor of this new interface. Thanks for working on it!
| |