[PATCH v2 1/5] drivers/misc/sgi-gru: add return on error

[PATCH v2 1/5] drivers/misc/sgi-gru: add return on error

[Sudip Mukherjee]

If the buffer is too small then return the error and in the process
remove the variables which became unused.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

v1: only removed variables.

 drivers/misc/sgi-gru/grukdump.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/sgi-gru/grukdump.c b/drivers/misc/sgi-gru/grukdump.c
index a3700a5..7e9aae5 100644
--- a/drivers/misc/sgi-gru/grukdump.c
+++ b/drivers/misc/sgi-gru/grukdump.c
@@ -78,11 +78,10 @@ static int gru_dump_tfm(struct gru_state *gru,
 		void __user *ubuf, void __user *ubufend)
 {
 	struct gru_tlb_fault_map *tfm;
-	int i, ret, bytes;
+	int i;
 
-	bytes = GRU_NUM_TFM * GRU_CACHE_LINE_BYTES;
-	if (bytes > ubufend - ubuf)
-		ret = -EFBIG;
+	if (GRU_NUM_TFM * GRU_CACHE_LINE_BYTES > ubufend - ubuf)
+		return -EFBIG;
 
 	for (i = 0; i < GRU_NUM_TFM; i++) {
 		tfm = get_tfm(gru->gs_gru_base_vaddr, i);
@@ -99,11 +98,10 @@ static int gru_dump_tgh(struct gru_state *gru,
 		void __user *ubuf, void __user *ubufend)
 {
 	struct gru_tlb_global_handle *tgh;
-	int i, ret, bytes;
+	int i;
 
-	bytes = GRU_NUM_TGH * GRU_CACHE_LINE_BYTES;
-	if (bytes > ubufend - ubuf)
-		ret = -EFBIG;
+	if (GRU_NUM_TGH * GRU_CACHE_LINE_BYTES > ubufend - ubuf)
+		return -EFBIG;
 
 	for (i = 0; i < GRU_NUM_TGH; i++) {
 		tgh = get_tgh(gru->gs_gru_base_vaddr, i);
-- 
1.9.1

[PATCH v2 2/5] drivers/misc/sgi-gru: make functions static

[Sudip Mukherjee]

The functions gru_get_cb_exception_detail_str() and gru_abort() were
only called locally from that file. We can make them static.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
Acked-by: Dimitri Sivanich <sivanich@sgi.com>
---

v2: no change, sent as a part of the series.

 drivers/misc/sgi-gru/grukservices.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/sgi-gru/grukservices.c b/drivers/misc/sgi-gru/grukservices.c
index 913de07..490b79a 100644
--- a/drivers/misc/sgi-gru/grukservices.c
+++ b/drivers/misc/sgi-gru/grukservices.c
@@ -429,8 +429,8 @@ int gru_get_cb_exception_detail(void *cb,
 	return 0;
 }
 
-char *gru_get_cb_exception_detail_str(int ret, void *cb,
-				      char *buf, int size)
+static char *gru_get_cb_exception_detail_str(int ret, void *cb,
+					     char *buf, int size)
 {
 	struct gru_control_block_status *gen = (void *)cb;
 	struct control_block_extended_exc_detail excdet;
@@ -505,7 +505,7 @@ int gru_wait_proc(void *cb)
 	return ret;
 }
 
-void gru_abort(int ret, void *cb, char *str)
+static void gru_abort(int ret, void *cb, char *str)
 {
 	char buf[GRU_EXC_STR_SIZE];
 
-- 
1.9.1

[PATCH v2 3/5] drivers/misc/sgi-gru: remove always false condition

[Sudip Mukherjee]

The member gid in struct gru_dump_chiplet_state_req is unsigned int. So
it can never be less than 0.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
Acked-by: Dimitri Sivanich <sivanich@sgi.com>
---

v2: no change, sent as a part of the series.

 drivers/misc/sgi-gru/grukdump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/sgi-gru/grukdump.c b/drivers/misc/sgi-gru/grukdump.c
index 7e9aae5..313da31 100644
--- a/drivers/misc/sgi-gru/grukdump.c
+++ b/drivers/misc/sgi-gru/grukdump.c
@@ -194,7 +194,7 @@ int gru_dump_chiplet_request(unsigned long arg)
 		return -EFAULT;
 
 	/* Currently, only dump by gid is implemented */
-	if (req.gid >= gru_max_gids || req.gid < 0)
+	if (req.gid >= gru_max_gids)
 		return -EINVAL;
 
 	gru = GID_TO_GRU(req.gid);
-- 
1.9.1

[PATCH v2 4/5] drivers/misc/sgi-gru: fix dereference of ERR_PTR

[Sudip Mukherjee]

gru_alloc_gts() can fail and it can return ERR_PTR(errvalue). We should
not dereference it if it has returned error. And incase it has returned
error then wait for some time and try again.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

v2: on error retry after msleep(1).
v1: returned error.

 drivers/misc/sgi-gru/grukservices.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/sgi-gru/grukservices.c b/drivers/misc/sgi-gru/grukservices.c
index 490b79a..d64f2a0 100644
--- a/drivers/misc/sgi-gru/grukservices.c
+++ b/drivers/misc/sgi-gru/grukservices.c
@@ -160,7 +160,14 @@ static void gru_load_kernel_context(struct gru_blade_state *bs, int blade_id)
 	down_write(&bs->bs_kgts_sema);
 
 	if (!bs->bs_kgts) {
-		bs->bs_kgts = gru_alloc_gts(NULL, 0, 0, 0, 0, 0);
+		do {
+			bs->bs_kgts = gru_alloc_gts(NULL, 0, 0, 0, 0, 0);
+			if (IS_ERR(bs->bs_kgts)) {
+				msleep(1);
+				continue;
+			}
+			break;
+		} while (true);
 		bs->bs_kgts->ts_user_blade_id = blade_id;
 	}
 	kgts = bs->bs_kgts;
-- 
1.9.1

Re: [PATCH v2 4/5] drivers/misc/sgi-gru: fix dereference of ERR_PTR

[Dimitri Sivanich]

On Thu, Sep 03, 2015 at 08:20:50PM +0530, Sudip Mukherjee wrote:
> gru_alloc_gts() can fail and it can return ERR_PTR(errvalue). We should
> not dereference it if it has returned error. And incase it has returned
> error then wait for some time and try again.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> 
> v2: on error retry after msleep(1).
> v1: returned error.
> 
>  drivers/misc/sgi-gru/grukservices.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/sgi-gru/grukservices.c b/drivers/misc/sgi-gru/grukservices.c
> index 490b79a..d64f2a0 100644
> --- a/drivers/misc/sgi-gru/grukservices.c
> +++ b/drivers/misc/sgi-gru/grukservices.c
> @@ -160,7 +160,14 @@ static void gru_load_kernel_context(struct gru_blade_state *bs, int blade_id)
>  	down_write(&bs->bs_kgts_sema);
>  
>  	if (!bs->bs_kgts) {
> -		bs->bs_kgts = gru_alloc_gts(NULL, 0, 0, 0, 0, 0);
> +		do {
> +			bs->bs_kgts = gru_alloc_gts(NULL, 0, 0, 0, 0, 0);
> +			if (IS_ERR(bs->bs_kgts)) {
> +				msleep(1);
> +				continue;
> +			}
> +			break;
> +		} while (true);

How about this way (for a little more compactness)?

+		do {
+			bs->bs_kgts = gru_alloc_gts(NULL, 0, 0, 0, 0, 0);
+			if (!IS_ERR(bs->bs_kgts))
+				break;
+			msleep(1)
+		} while (true);
>  		bs->bs_kgts->ts_user_blade_id = blade_id;
>  	}
>  	kgts = bs->bs_kgts;
> -- 
> 1.9.1

[PATCH v3] drivers/misc/sgi-gru: fix dereference of ERR_PTR

[Sudip Mukherjee]

gru_alloc_gts() can fail and it can return ERR_PTR(errvalue). We should
not dereference it if it has returned error. And incase it has returned
error then wait for some time and try again.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

v3: compact code for if logic
v2: on error retry after msleep(1).
v1: returned error.


 drivers/misc/sgi-gru/grukservices.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/sgi-gru/grukservices.c b/drivers/misc/sgi-gru/grukservices.c
index 490b79a..7812e34 100644
--- a/drivers/misc/sgi-gru/grukservices.c
+++ b/drivers/misc/sgi-gru/grukservices.c
@@ -160,7 +160,12 @@ static void gru_load_kernel_context(struct gru_blade_state *bs, int blade_id)
 	down_write(&bs->bs_kgts_sema);
 
 	if (!bs->bs_kgts) {
-		bs->bs_kgts = gru_alloc_gts(NULL, 0, 0, 0, 0, 0);
+		do {
+			bs->bs_kgts = gru_alloc_gts(NULL, 0, 0, 0, 0, 0);
+			if (!IS_ERR(bs->bs_kgts))
+				break;
+			msleep(1);
+		} while (true);
 		bs->bs_kgts->ts_user_blade_id = blade_id;
 	}
 	kgts = bs->bs_kgts;
-- 
1.9.1

Re: [PATCH v3] drivers/misc/sgi-gru: fix dereference of ERR_PTR

[Dimitri Sivanich]

Acked-by: Dimitri Sivanich <sivanich@sgi.com>

On Fri, Sep 04, 2015 at 09:56:20AM +0530, Sudip Mukherjee wrote:
> gru_alloc_gts() can fail and it can return ERR_PTR(errvalue). We should
> not dereference it if it has returned error. And incase it has returned
> error then wait for some time and try again.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> 
> v3: compact code for if logic
> v2: on error retry after msleep(1).
> v1: returned error.
> 
> 
>  drivers/misc/sgi-gru/grukservices.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/sgi-gru/grukservices.c b/drivers/misc/sgi-gru/grukservices.c
> index 490b79a..7812e34 100644
> --- a/drivers/misc/sgi-gru/grukservices.c
> +++ b/drivers/misc/sgi-gru/grukservices.c
> @@ -160,7 +160,12 @@ static void gru_load_kernel_context(struct gru_blade_state *bs, int blade_id)
>  	down_write(&bs->bs_kgts_sema);
>  
>  	if (!bs->bs_kgts) {
> -		bs->bs_kgts = gru_alloc_gts(NULL, 0, 0, 0, 0, 0);
> +		do {
> +			bs->bs_kgts = gru_alloc_gts(NULL, 0, 0, 0, 0, 0);
> +			if (!IS_ERR(bs->bs_kgts))
> +				break;
> +			msleep(1);
> +		} while (true);
>  		bs->bs_kgts->ts_user_blade_id = blade_id;
>  	}
>  	kgts = bs->bs_kgts;
> -- 
> 1.9.1

[PATCH v2 5/5] drivers/misc/sgi-gru: remove unused variable

[Sudip Mukherjee]

dw was only assigned some value and was never reused.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

v2: It was part of first patch in v1. But since one logical change in
one patch so it had to be removed from first patch and became separate.

 drivers/misc/sgi-gru/grukservices.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/misc/sgi-gru/grukservices.c b/drivers/misc/sgi-gru/grukservices.c
index d64f2a0..ba2d2e5 100644
--- a/drivers/misc/sgi-gru/grukservices.c
+++ b/drivers/misc/sgi-gru/grukservices.c
@@ -1004,7 +1004,6 @@ static int quicktest1(unsigned long arg)
 {
 	struct gru_message_queue_desc mqd;
 	void *p, *mq;
-	unsigned long *dw;
 	int i, ret = -EIO;
 	char mes[GRU_CACHE_LINE_BYTES], *m;
 
@@ -1014,7 +1013,6 @@ static int quicktest1(unsigned long arg)
 		return -ENOMEM;
 	mq = ALIGNUP(p, 1024);
 	memset(mes, 0xee, sizeof(mes));
-	dw = mq;
 
 	gru_create_message_queue(&mqd, mq, 8 * GRU_CACHE_LINE_BYTES, 0, 0, 0);
 	for (i = 0; i < 6; i++) {
-- 
1.9.1

Re: [PATCH v2 5/5] drivers/misc/sgi-gru: remove unused variable

[Dimitri Sivanich]

Acked-by: Dimitri Sivanich <sivanich@sgi.com>

On Thu, Sep 03, 2015 at 08:20:51PM +0530, Sudip Mukherjee wrote:
> dw was only assigned some value and was never reused.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> 
> v2: It was part of first patch in v1. But since one logical change in
> one patch so it had to be removed from first patch and became separate.
> 
>  drivers/misc/sgi-gru/grukservices.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/misc/sgi-gru/grukservices.c b/drivers/misc/sgi-gru/grukservices.c
> index d64f2a0..ba2d2e5 100644
> --- a/drivers/misc/sgi-gru/grukservices.c
> +++ b/drivers/misc/sgi-gru/grukservices.c
> @@ -1004,7 +1004,6 @@ static int quicktest1(unsigned long arg)
>  {
>  	struct gru_message_queue_desc mqd;
>  	void *p, *mq;
> -	unsigned long *dw;
>  	int i, ret = -EIO;
>  	char mes[GRU_CACHE_LINE_BYTES], *m;
>  
> @@ -1014,7 +1013,6 @@ static int quicktest1(unsigned long arg)
>  		return -ENOMEM;
>  	mq = ALIGNUP(p, 1024);
>  	memset(mes, 0xee, sizeof(mes));
> -	dw = mq;
>  
>  	gru_create_message_queue(&mqd, mq, 8 * GRU_CACHE_LINE_BYTES, 0, 0, 0);
>  	for (i = 0; i < 6; i++) {
> -- 
> 1.9.1

Re: [PATCH v2 1/5] drivers/misc/sgi-gru: add return on error

[Dimitri Sivanich]

Acked-by: Dimitri Sivanich <sivanich@sgi.com>

On Thu, Sep 03, 2015 at 08:20:47PM +0530, Sudip Mukherjee wrote:
> If the buffer is too small then return the error and in the process
> remove the variables which became unused.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> 
> v1: only removed variables.
> 
>  drivers/misc/sgi-gru/grukdump.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/misc/sgi-gru/grukdump.c b/drivers/misc/sgi-gru/grukdump.c
> index a3700a5..7e9aae5 100644
> --- a/drivers/misc/sgi-gru/grukdump.c
> +++ b/drivers/misc/sgi-gru/grukdump.c
> @@ -78,11 +78,10 @@ static int gru_dump_tfm(struct gru_state *gru,
>  		void __user *ubuf, void __user *ubufend)
>  {
>  	struct gru_tlb_fault_map *tfm;
> -	int i, ret, bytes;
> +	int i;
>  
> -	bytes = GRU_NUM_TFM * GRU_CACHE_LINE_BYTES;
> -	if (bytes > ubufend - ubuf)
> -		ret = -EFBIG;
> +	if (GRU_NUM_TFM * GRU_CACHE_LINE_BYTES > ubufend - ubuf)
> +		return -EFBIG;
>  
>  	for (i = 0; i < GRU_NUM_TFM; i++) {
>  		tfm = get_tfm(gru->gs_gru_base_vaddr, i);
> @@ -99,11 +98,10 @@ static int gru_dump_tgh(struct gru_state *gru,
>  		void __user *ubuf, void __user *ubufend)
>  {
>  	struct gru_tlb_global_handle *tgh;
> -	int i, ret, bytes;
> +	int i;
>  
> -	bytes = GRU_NUM_TGH * GRU_CACHE_LINE_BYTES;
> -	if (bytes > ubufend - ubuf)
> -		ret = -EFBIG;
> +	if (GRU_NUM_TGH * GRU_CACHE_LINE_BYTES > ubufend - ubuf)
> +		return -EFBIG;
>  
>  	for (i = 0; i < GRU_NUM_TGH; i++) {
>  		tgh = get_tgh(gru->gs_gru_base_vaddr, i);
> -- 
> 1.9.1

Re: [PATCH v2 1/5] drivers/misc/sgi-gru: add return on error

[Sudip Mukherjee]

On Thu, Sep 03, 2015 at 01:21:38PM -0500, Dimitri Sivanich wrote:
> Acked-by: Dimitri Sivanich <sivanich@sgi.com>
> 
> On Thu, Sep 03, 2015 at 08:20:47PM +0530, Sudip Mukherjee wrote:
> > If the buffer is too small then return the error and in the process
> > remove the variables which became unused.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > ---
Hi Greg,
I know its too early to ping you but just wanted to know that this
should go through your char-misc tree or through Andrew.
Looking at all the previous patches, it all went through Andrew but
being a char/misc I thought it will go through you.

regards
sudip

Re: [PATCH v2 1/5] drivers/misc/sgi-gru: add return on error

[Greg Kroah-Hartman]

On Tue, Sep 15, 2015 at 01:20:41PM +0530, Sudip Mukherjee wrote:
> On Thu, Sep 03, 2015 at 01:21:38PM -0500, Dimitri Sivanich wrote:
> > Acked-by: Dimitri Sivanich <sivanich@sgi.com>
> > 
> > On Thu, Sep 03, 2015 at 08:20:47PM +0530, Sudip Mukherjee wrote:
> > > If the buffer is too small then return the error and in the process
> > > remove the variables which became unused.
> > > 
> > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > > ---
> Hi Greg,
> I know its too early to ping you but just wanted to know that this
> should go through your char-misc tree or through Andrew.
> Looking at all the previous patches, it all went through Andrew but
> being a char/misc I thought it will go through you.

I'll take it, thanks.

greg k-h

Re: [PATCH v2 1/5] drivers/misc/sgi-gru: add return on error

[Sudip Mukherjee]

On Sun, Sep 20, 2015 at 07:26:29PM -0700, Greg Kroah-Hartman wrote:
> On Tue, Sep 15, 2015 at 01:20:41PM +0530, Sudip Mukherjee wrote:
> > On Thu, Sep 03, 2015 at 01:21:38PM -0500, Dimitri Sivanich wrote:
> > > Acked-by: Dimitri Sivanich <sivanich@sgi.com>
> > > 
> > > On Thu, Sep 03, 2015 at 08:20:47PM +0530, Sudip Mukherjee wrote:
> > > > If the buffer is too small then return the error and in the process
> > > > remove the variables which became unused.
> > > > 
> > > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > > > ---
> > Hi Greg,
> > I know its too early to ping you but just wanted to know that this
> > should go through your char-misc tree or through Andrew.
> > Looking at all the previous patches, it all went through Andrew but
> > being a char/misc I thought it will go through you.
> 
> I'll take it, thanks.
Thanks. But just to let you know that getmaintainer.pl is not showing
your and Arnd's name. But MAINTAINER shows drivers/misc/* for you and
Arnd. And besides I noticed there are one or two patches still lying
around with maintainer ACK but never picked up as they didnot have you
or Andrew in the To or CC list.

regards
sudip

Re: [PATCH v2 1/5] drivers/misc/sgi-gru: add return on error

[Greg Kroah-Hartman]

On Mon, Sep 21, 2015 at 11:04:10AM +0530, Sudip Mukherjee wrote:
> On Sun, Sep 20, 2015 at 07:26:29PM -0700, Greg Kroah-Hartman wrote:
> > On Tue, Sep 15, 2015 at 01:20:41PM +0530, Sudip Mukherjee wrote:
> > > On Thu, Sep 03, 2015 at 01:21:38PM -0500, Dimitri Sivanich wrote:
> > > > Acked-by: Dimitri Sivanich <sivanich@sgi.com>
> > > > 
> > > > On Thu, Sep 03, 2015 at 08:20:47PM +0530, Sudip Mukherjee wrote:
> > > > > If the buffer is too small then return the error and in the process
> > > > > remove the variables which became unused.
> > > > > 
> > > > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > > > > ---
> > > Hi Greg,
> > > I know its too early to ping you but just wanted to know that this
> > > should go through your char-misc tree or through Andrew.
> > > Looking at all the previous patches, it all went through Andrew but
> > > being a char/misc I thought it will go through you.
> > 
> > I'll take it, thanks.
> Thanks. But just to let you know that getmaintainer.pl is not showing
> your and Arnd's name. But MAINTAINER shows drivers/misc/* for you and
> Arnd. And besides I noticed there are one or two patches still lying
> around with maintainer ACK but never picked up as they didnot have you
> or Andrew in the To or CC list.

That's odd, care to forward them on to me?

Re: [PATCH v2 1/5] drivers/misc/sgi-gru: add return on error

[Sudip Mukherjee]

On Sun, Sep 20, 2015 at 10:36:15PM -0700, Greg Kroah-Hartman wrote:
> On Mon, Sep 21, 2015 at 11:04:10AM +0530, Sudip Mukherjee wrote:
> > On Sun, Sep 20, 2015 at 07:26:29PM -0700, Greg Kroah-Hartman wrote:
> > > On Tue, Sep 15, 2015 at 01:20:41PM +0530, Sudip Mukherjee wrote:
> > > > On Thu, Sep 03, 2015 at 01:21:38PM -0500, Dimitri Sivanich wrote:
> > > > > Acked-by: Dimitri Sivanich <sivanich@sgi.com>
> > > > > 
> > > > > On Thu, Sep 03, 2015 at 08:20:47PM +0530, Sudip Mukherjee wrote:
> > > > > > If the buffer is too small then return the error and in the process
> > > > > > remove the variables which became unused.
> > > > > > 
> > > > > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > > > > > ---
> > > > Hi Greg,
> > > > I know its too early to ping you but just wanted to know that this
> > > > should go through your char-misc tree or through Andrew.
> > > > Looking at all the previous patches, it all went through Andrew but
> > > > being a char/misc I thought it will go through you.
> > > 
> > > I'll take it, thanks.
> > Thanks. But just to let you know that getmaintainer.pl is not showing
> > your and Arnd's name. But MAINTAINER shows drivers/misc/* for you and
> > Arnd. And besides I noticed there are one or two patches still lying
> > around with maintainer ACK but never picked up as they didnot have you
> > or Andrew in the To or CC list.
> 
> That's odd, care to forward them on to me?
OMG... There are quite a few. Even from 2014 and one of them is a bugfix
for a bugzilla.
I am sending them all by the end of the day (IST).

regards
sudip