[PATCH] Avoid holding the zero page on slab init failure

[PATCH] Avoid holding the zero page on slab init failure

[Benoît Canet]

Spotted by visual inspection.

Applies on libceph: Remove spurious kunmap() of the zero page.

Benoît Canet (1):
  libceph: Avoid holding the zero page on ceph_msgr_slab_init errors

 net/ceph/messenger.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] libceph: Avoid holding the zero page on ceph_msgr_slab_init errors

[Benoît Canet]

ceph_msgr_slab_init may fail due to a temporary ENOMEM.

Delay a bit the initialization of zero_page in ceph_msgr_init and
reorder it's cleanup in _ceph_msgr_exit for readability sake.

BUG_ON() will not suffer to be postponed in case it is triggered.

Signed-off-by: Benoît Canet <benoit.canet@nodalink.com>
---
 net/ceph/messenger.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 38f06a4..ec68cd3 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -275,22 +275,22 @@ static void _ceph_msgr_exit(void)
 		ceph_msgr_wq = NULL;
 	}
 
-	ceph_msgr_slab_exit();
-
 	BUG_ON(zero_page == NULL);
 	page_cache_release(zero_page);
 	zero_page = NULL;
+
+	ceph_msgr_slab_exit();
 }
 
 int ceph_msgr_init(void)
 {
+	if (ceph_msgr_slab_init())
+		return -ENOMEM;
+
 	BUG_ON(zero_page != NULL);
 	zero_page = ZERO_PAGE(0);
 	page_cache_get(zero_page);
 
-	if (ceph_msgr_slab_init())
-		return -ENOMEM;
-
 	/*
 	 * The number of active work items is limited by the number of
 	 * connections, so leave @max_active at default.
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libceph: Avoid holding the zero page on ceph_msgr_slab_init errors

[Alex Elder]

On 06/24/2015 08:27 PM, Benoît Canet wrote:
> ceph_msgr_slab_init may fail due to a temporary ENOMEM.

Looks good.

> Delay a bit the initialization of zero_page in ceph_msgr_init and
> reorder it's cleanup in _ceph_msgr_exit for readability sake.

I'd say it's not readability, but a proper ordering (tear down
in reverse order of set up).

Reviewed-by: Alex Elder <elder@linaro.org>

> BUG_ON() will not suffer to be postponed in case it is triggered.

This is unrelated, but in cases like this (where there's already
something in place to return an error) we should replace these
BUG_ON() calls with logged errors and a return.  They should
never ever happen, of course (they represent design problems)
but if it did it's better to allow the system to keep running.

> Signed-off-by: Benoît Canet <benoit.canet@nodalink.com>
> ---
>   net/ceph/messenger.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
> index 38f06a4..ec68cd3 100644
> --- a/net/ceph/messenger.c
> +++ b/net/ceph/messenger.c
> @@ -275,22 +275,22 @@ static void _ceph_msgr_exit(void)
>   		ceph_msgr_wq = NULL;
>   	}
>
> -	ceph_msgr_slab_exit();
> -
>   	BUG_ON(zero_page == NULL);
>   	page_cache_release(zero_page);
>   	zero_page = NULL;
> +
> +	ceph_msgr_slab_exit();
>   }
>
>   int ceph_msgr_init(void)
>   {
> +	if (ceph_msgr_slab_init())
> +		return -ENOMEM;
> +
>   	BUG_ON(zero_page != NULL);
>   	zero_page = ZERO_PAGE(0);
>   	page_cache_get(zero_page);
>
> -	if (ceph_msgr_slab_init())
> -		return -ENOMEM;
> -
>   	/*
>   	 * The number of active work items is limited by the number of
>   	 * connections, so leave @max_active at default.
>

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Avoid holding the zero page on slab init failure

[Alex Elder]

On 06/24/2015 08:27 PM, Benoît Canet wrote:
> Spotted by visual inspection.
>
> Applies on libceph: Remove spurious kunmap() of the zero page.
>
> Benoît Canet (1):
>    libceph: Avoid holding the zero page on ceph_msgr_slab_init errors
>
>   net/ceph/messenger.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>

Benoit, in cases where you have just a single patch, there is
no need to include an additional mail message like this one.

If you have a series, that's when you'll supply a PATCH 0/N
message.

					-Alex
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html