On Thu 22-10-15 00:21:29, Johannes Weiner wrote:
> page_counter_try_charge() currently returns 0 on success and -ENOMEM
> on failure, which is surprising behavior given the function name.
>
> Make it follow the expected pattern of try_stuff() functions that
> return a boolean true to indicate success, or false for failure.
>
> Signed-off-by: Johannes Weiner <han...@cmpxchg.org>
Acked-by: Michal Hocko <mho...@suse.com>
> ---
> include/linux/page_counter.h | 6 +++---
> mm/hugetlb_cgroup.c | 3 ++-
> mm/memcontrol.c | 11 +++++------
> mm/page_counter.c | 14 +++++++-------
> 4 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/page_counter.h b/include/linux/page_counter.h
> index 17fa4f8..7e62920 100644
> --- a/include/linux/page_counter.h
> +++ b/include/linux/page_counter.h
> @@ -36,9 +36,9 @@ static inline unsigned long page_counter_read(struct
> page_counter *counter)
>
> void page_counter_cancel(struct page_counter *counter, unsigned long
> nr_pages);
> void page_counter_charge(struct page_counter *counter, unsigned long
> nr_pages);
> -int page_counter_try_charge(struct page_counter *counter,
> - unsigned long nr_pages,
> - struct page_counter **fail);
> +bool page_counter_try_charge(struct page_counter *counter,
> + unsigned long nr_pages,
> + struct page_counter **fail);
> void page_counter_uncharge(struct page_counter *counter, unsigned long
> nr_pages);
> int page_counter_limit(struct page_counter *counter, unsigned long limit);
> int page_counter_memparse(const char *buf, const char *max,
> diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
> index 6a44263..d8fb10d 100644
> --- a/mm/hugetlb_cgroup.c
> +++ b/mm/hugetlb_cgroup.c
> @@ -186,7 +186,8 @@ again:
> }
> rcu_read_unlock();
>
> - ret = page_counter_try_charge(&h_cg->hugepage[idx], nr_pages, &counter);
> + if (!page_counter_try_charge(&h_cg->hugepage[idx], nr_pages, &counter))
> + ret = -ENOMEM;
> css_put(&h_cg->css);
> done:
> *ptr = h_cg;
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index c71fe40..a8ccdbc 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2018,8 +2018,8 @@ retry:
> return 0;
>
> if (!do_swap_account ||
> - !page_counter_try_charge(&memcg->memsw, batch, &counter)) {
> - if (!page_counter_try_charge(&memcg->memory, batch, &counter))
> + page_counter_try_charge(&memcg->memsw, batch, &counter)) {
> + if (page_counter_try_charge(&memcg->memory, batch, &counter))
> goto done_restock;
> if (do_swap_account)
> page_counter_uncharge(&memcg->memsw, batch);
> @@ -2383,14 +2383,13 @@ int __memcg_kmem_charge_memcg(struct page *page,
> gfp_t gfp, int order,
> {
> unsigned int nr_pages = 1 << order;
> struct page_counter *counter;
> - int ret = 0;
> + int ret;
>
> if (!memcg_kmem_is_active(memcg))
> return 0;
>
> - ret = page_counter_try_charge(&memcg->kmem, nr_pages, &counter);
> - if (ret)
> - return ret;
> + if (!page_counter_try_charge(&memcg->kmem, nr_pages, &counter))
> + return -ENOMEM;
>
> ret = try_charge(memcg, gfp, nr_pages);
> if (ret) {
> diff --git a/mm/page_counter.c b/mm/page_counter.c
> index 11b4bed..7c6a63d 100644
> --- a/mm/page_counter.c
> +++ b/mm/page_counter.c
> @@ -56,12 +56,12 @@ void page_counter_charge(struct page_counter *counter,
> unsigned long nr_pages)
> * @nr_pages: number of pages to charge
> * @fail: points first counter to hit its limit, if any
> *
> - * Returns 0 on success, or -ENOMEM and @fail if the counter or one of
> - * its ancestors has hit its configured limit.
> + * Returns %true on success, or %false and @fail if the counter or one
> + * of its ancestors has hit its configured limit.
> */
> -int page_counter_try_charge(struct page_counter *counter,
> - unsigned long nr_pages,
> - struct page_counter **fail)
> +bool page_counter_try_charge(struct page_counter *counter,
> + unsigned long nr_pages,
> + struct page_counter **fail)
> {
> struct page_counter *c;
>
> @@ -99,13 +99,13 @@ int page_counter_try_charge(struct page_counter *counter,
> if (new > c->watermark)
> c->watermark = new;
> }
> - return 0;
> + return true;
>
> failed:
> for (c = counter; c != *fail; c = c->parent)
> page_counter_cancel(c, nr_pages);
>
> - return -ENOMEM;
> + return false;
> }
>
> /**
> --
> 2.6.1
--
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
