Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1290616 > unrolled thread

[PATCH 0/7] staging-Lustre: Fine-tuning for some function implementations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 0/7] staging-Lustre: Fine-tuning for some function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-13 14:50 +0100
    [PATCH 2/7] staging: lustre: Rename a jump label for  ptlrpc_req_finished() calls SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-13 15:00 +0100
      Re: [PATCH 2/7] staging: lustre: Rename a jump label for  ptlrpc_req_finished() calls Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-14 08:00 +0100
        Re: [PATCH 2/7] staging: lustre: Rename a jump label for  ptlrpc_req_finished() calls SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-14 10:10 +0100
          Re: [PATCH 2/7] staging: lustre: Rename a jump label for  ptlrpc_req_finished() calls Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-14 10:40 +0100
            Re: staging: lustre: Rename a jump label for ptlrpc_req_finished()  calls SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-14 11:10 +0100
    [PATCH 1/7] staging: lustre: Delete unnecessary goto statements in  six functions SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-13 15:00 +0100
      Re: [PATCH 1/7] staging: lustre: Delete unnecessary goto statements  in six functions Joe Perches <joe@perches.com> - 2015-12-15 15:30 +0100
        Re: [PATCH 1/7] staging: lustre: Delete unnecessary goto statements  in six functions Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-15 15:50 +0100
          Re: [PATCH 1/7] staging: lustre: Delete unnecessary goto statements  in six functions Joe Perches <joe@perches.com> - 2015-12-15 16:10 +0100
            Re: [PATCH 1/7] staging: lustre: Delete unnecessary goto statements  in six functions Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-15 18:50 +0100
              Re: [PATCH 1/7] staging: lustre: Delete unnecessary goto statements  in six functions Joe Perches <joe@perches.com> - 2015-12-15 19:20 +0100
                Re: staging: lustre: Delete unnecessary goto statements in six  functions SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-15 19:30 +0100
                  Re: staging: lustre: Delete unnecessary goto statements in six  functions Joe Perches <joe@perches.com> - 2015-12-15 19:40 +0100
                    Re: staging: lustre: Delete unnecessary goto statements in six  functions SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-15 20:00 +0100
                      Re: staging: lustre: Delete unnecessary goto statements in six  functions Joe Perches <joe@perches.com> - 2015-12-15 20:00 +0100
            Re: staging: lustre: Delete unnecessary goto statements in six  functions SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-15 19:10 +0100
              Re: staging: lustre: Delete unnecessary goto statements in six  functions Joe Perches <joe@perches.com> - 2015-12-15 19:30 +0100
    [PATCH 3/7] staging: lustre: Rename a jump label for a kfree(key)  call SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-13 15:00 +0100
    [PATCH 6/7] staging: lustre: A few checks less in  mgc_process_recover_log() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-13 15:00 +0100
    [PATCH 5/7] staging: lustre: Less checks in mgc_process_recover_log()  after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-13 15:00 +0100
      Re: [PATCH 5/7] staging: lustre: Less checks in  mgc_process_recover_log() after error detection Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-14 12:10 +0100
        Re: [PATCH 5/7] staging: lustre: Less checks in  mgc_process_recover_log() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-14 13:10 +0100
          Re: [PATCH 5/7] staging: lustre: Less checks in  mgc_process_recover_log() after error detection Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-14 13:40 +0100
            Re: staging: lustre: Less checks in mgc_process_recover_log() after  error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-14 13:50 +0100
              Re: staging: lustre: Less checks in mgc_process_recover_log() after  error detection Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-14 15:00 +0100
                Re: staging: lustre: Less checks in mgc_process_recover_log() after  error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-14 18:50 +0100
                  Re: staging: lustre: Less checks in mgc_process_recover_log() after  error detection Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-15 12:50 +0100
                    Re: staging: lustre: Less checks in mgc_process_recover_log() after  error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-15 16:10 +0100
    [PATCH 4/7] staging: lustre: Delete an unnecessary variable  initialisation in mgc_process_recover_log() SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-13 15:00 +0100
    [PATCH 7/7] staging: lustre: Rename a jump label for module_put()  calls SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-13 15:10 +0100
    [PATCH v2 0/4] staging-Lustre: Fine-tuning for some function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-21 20:10 +0100
      [PATCH v2 1/4] staging: lustre: Delete unnecessary goto statements in  six functions SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-21 20:10 +0100
      [PATCH v2 4/4] staging: lustre: Fix a jump label position in  osc_get_info() SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-21 20:20 +0100
      [PATCH v2 3/4] staging: lustre: Less checks in  mgc_process_recover_log() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-21 20:20 +0100
        Re: [PATCH v2 3/4] staging: lustre: Less checks in  mgc_process_recover_log() after error detection Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-12-22 00:50 +0100
          Re: [PATCH v2 3/4] staging: lustre: Less checks in  mgc_process_recover_log() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-22 08:20 +0100
          Re: [PATCH v2 3/4] staging: lustre: Less checks in  mgc_process_recover_log() after error detection Dan Carpenter <dan.carpenter@oracle.com> - 2015-12-22 09:10 +0100
      [PATCH v2 2/4] staging: lustre: Delete an unnecessary variable  initialisation in mgc_process_recover_log() SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-21 20:20 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1290623 — [PATCH 5/7] staging: lustre: Less checks in mgc_process_recover_log() after error detection

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 13 Dec 2015 12:21:17 +0100

A few checks would be performed by the mgc_process_recover_log() function
even if it is known already that the passed variable "pages" contained
a null pointer.

* Let us return directly if a call of the kcalloc() function failed.

* Move assignments for the variables "eof" and "req" behind
  this memory allocation.

* Delete a sanity check then.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/lustre/lustre/mgc/mgc_request.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index da130f4..f3b4c30 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -1285,14 +1285,14 @@ static int mgc_apply_recover_logs(struct obd_device *mgc,
 static int mgc_process_recover_log(struct obd_device *obd,
 				   struct config_llog_data *cld)
 {
-	struct ptlrpc_request *req = NULL;
+	struct ptlrpc_request *req;
 	struct config_llog_instance *cfg = &cld->cld_cfg;
 	struct mgs_config_body *body;
 	struct mgs_config_res  *res;
 	struct ptlrpc_bulk_desc *desc;
 	struct page **pages;
 	int nrpages;
-	bool eof = true;
+	bool eof;
 	bool mne_swab;
 	int i;
 	int ealen;
@@ -1309,10 +1309,11 @@ static int mgc_process_recover_log(struct obd_device *obd,
 		nrpages = CONFIG_READ_NRPAGES_INIT;
 
 	pages = kcalloc(nrpages, sizeof(*pages), GFP_KERNEL);
-	if (pages == NULL) {
-		rc = -ENOMEM;
-		goto out;
-	}
+	if (!pages)
+		return -ENOMEM;
+
+	req = NULL;
+	eof = true;
 
 	for (i = 0; i < nrpages; i++) {
 		pages[i] = alloc_page(GFP_KERNEL);
@@ -1432,14 +1433,12 @@ out:
 	if (rc == 0 && !eof)
 		goto again;
 
-	if (pages) {
-		for (i = 0; i < nrpages; i++) {
-			if (pages[i] == NULL)
-				break;
-			__free_page(pages[i]);
-		}
-		kfree(pages);
+	for (i = 0; i < nrpages; i++) {
+		if (pages[i] == NULL)
+			break;
+		__free_page(pages[i]);
 	}
+	kfree(pages);
 	return rc;
 }
 
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291111 — Re: [PATCH 5/7] staging: lustre: Less checks in mgc_process_recover_log() after error detection

On Sun, Dec 13, 2015 at 02:57:38PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 13 Dec 2015 12:21:17 +0100
> 
> A few checks would be performed by the mgc_process_recover_log() function
> even if it is known already that the passed variable "pages" contained
> a null pointer.
> 
> * Let us return directly if a call of the kcalloc() function failed.
> 
> * Move assignments for the variables "eof" and "req" behind
>   this memory allocation.

Why?  Then in the next patch it moves again.  It's like cup shuffle to
read these patches sometimes.

regards,
dan carpenter


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291139 — Re: [PATCH 5/7] staging: lustre: Less checks in mgc_process_recover_log() after error detection

>> A few checks would be performed by the mgc_process_recover_log() function
>> even if it is known already that the passed variable "pages" contained
>> a null pointer.
>>
>> * Let us return directly if a call of the kcalloc() function failed.
>>
>> * Move assignments for the variables "eof" and "req" behind
>>   this memory allocation.
> 
> Why?

The positions of their initialisation depends on the selected exception 
handling implementation, doesn't it?

Can you accept the proposed changes around the affected memory allocations?


> Then in the next patch it moves again.

This detail is a matter of patch granularity.


> It's like cup shuffle to read these patches sometimes.

Do you prefer to stash any changes together for a bigger update step?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291152 — Re: [PATCH 5/7] staging: lustre: Less checks in mgc_process_recover_log() after error detection

On Mon, Dec 14, 2015 at 01:04:14PM +0100, SF Markus Elfring wrote:
> >> A few checks would be performed by the mgc_process_recover_log() function
> >> even if it is known already that the passed variable "pages" contained
> >> a null pointer.
> >>
> >> * Let us return directly if a call of the kcalloc() function failed.
> >>
> >> * Move assignments for the variables "eof" and "req" behind
> >>   this memory allocation.
> > 
> > Why?
> 
> The positions of their initialisation depends on the selected exception 
> handling implementation, doesn't it?
> 
> Can you accept the proposed changes around the affected memory allocations?
> 

Just leave it as-is if there is no reason.

> 
> > Then in the next patch it moves again.
> 
> This detail is a matter of patch granularity.
> 
> 
> > It's like cup shuffle to read these patches sometimes.
> 
> Do you prefer to stash any changes together for a bigger update step?

Yes.  Patches 5 and 6 would be easier to review if they were folded into
one patch.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291158 — Re: staging: lustre: Less checks in mgc_process_recover_log() after error detection

>> Can you accept the proposed changes around the affected memory allocations?
> 
> Just leave it as-is if there is no reason.

I suggest to make the implementation of the function "mgc_process_recover_log"
a bit more efficient.


>> Do you prefer to stash any changes together for a bigger update step?
> 
> Yes.  Patches 5 and 6 would be easier to review if they were folded into
> one patch.

I do not like patch squashing for my update suggestions here.

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291198 — Re: staging: lustre: Less checks in mgc_process_recover_log() after error detection

On Mon, Dec 14, 2015 at 01:45:11PM +0100, SF Markus Elfring wrote:
> >> Do you prefer to stash any changes together for a bigger update step?
> > 
> > Yes.  Patches 5 and 6 would be easier to review if they were folded into
> > one patch.
> 
> I do not like patch squashing for my update suggestions here.

I am a maintainer in drivers/staging.  I am telling you what you need to
do if you want us to apply your patch.  What you do with that
information is up to you.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291377 — Re: staging: lustre: Less checks in mgc_process_recover_log() after error detection

>> I do not like patch squashing for my update suggestions here.
> 
> I am a maintainer in drivers/staging.

Thanks for this information.


> I am telling you what you need to do if you want us to apply your patch.

I am still waiting for a bit more constructive feedback for this
patch series. How many days should I wait before I should send adjusted
update suggestions for this approach?


> What you do with that information is up to you.

Our software development dialogue seems to trigger special
challenges between us so far.
Are you generally willing to change the exception handling for
the memory allocations in the function "mgc_process_recover_log"
at all?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292071 — Re: staging: lustre: Less checks in mgc_process_recover_log() after error detection

On Mon, Dec 14, 2015 at 06:43:15PM +0100, SF Markus Elfring wrote:
> Our software development dialogue seems to trigger special
> challenges between us so far.

I try very hard to review patches mechanically and not be biased so that
after a while people know if their patches will be merged or not without
waiting for feedback.

In this case, I had asked you not to send patches renaming out labels
and then the next day you sent me a string of patches renaming out
labels.  If you were a lustre dev then I would accept these renames
definitely.  But I believe that for anyone else, I would ask them what
the point of doing these renames is.  I do not think I have been unfair
to you.  There was no element of surprise.

Part of the reason we have CodingStyle is so that we can tell people
"That's not in CodingStyle, that's just your own opinion so don't redo
code just because you have a different opinion from the maintainer."

> Are you generally willing to change the exception handling for
> the memory allocations in the function "mgc_process_recover_log"
> at all?

I like the first patch in this series.  I do not like the renames.  I
don't care too much about patches 5 and 6 except that they should be
folded together and you should not move "req" and "eof" around.

Mostly I wish you would just focus on fixing bugs instead of these sorts
of patches.  It is a lot of work for me to explain how to redo patches
but it is worth it for bugfixes.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292244 — Re: staging: lustre: Less checks in mgc_process_recover_log() after error detection

> If you were a lustre dev then I would accept these renames definitely.

I find this information interesting.
Would any more contributors like to share their opinion?


> I do not think I have been unfair to you.

This view is correct in principle.


> There was no element of surprise.

I am trying to discuss further "special" update suggestions
where the topic focus might evolve to new directions.
I got the impression that you had some difficulties already
with my previous proposals. So I am unsure about the general
change acceptance from you alone.

You pointed out that you are maintainer for this software area.
I was not so aware about this detail while I noticed that
you are very active Linux software developer.
(You are not mentioned in the file "MAINTAINERS" for example.)


> Part of the reason we have CodingStyle is so that we can tell people
> "That's not in CodingStyle, that's just your own opinion so don't redo
> code just because you have a different opinion from the maintainer."

I find this description reasonable.

But I see some challenges to improve the coding style specification.
I would appreciate if some items can become less ambiguous and imprecise.
I assume that a few recommendations from the script "checkpatch.pl"
should also be mentioned there.

>
>> Are you generally willing to change the exception handling for
>> the memory allocations in the function "mgc_process_recover_log"
>> at all?
> I like the first patch in this series.

Thanks for a bit of positive feedback.


> I do not like the renames.

I guess that this design aspect can be clarified a bit more.


> I don't care too much about patches 5 and 6 except that they should be
> folded together and you should not move "req" and "eof" around.

I can understand this concern better than your first response
for these update steps.

I might send an adjusted patch series a few days later.


> Mostly I wish you would just focus on fixing bugs instead of these sorts
> of patches.

How often are deviations from the coding style also just ordinary bugs?

It seems that changes for this area are occasionally not so attractive
in comparison to software improvements for components
which are more popular.

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1290624 — [PATCH 4/7] staging: lustre: Delete an unnecessary variable initialisation in mgc_process_recover_log()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 13 Dec 2015 12:00:32 +0100

The variable "mne_swab" will eventually be set to an appropriate value
from a call of the ptlrpc_rep_need_swab() function.
Thus let us omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/lustre/lustre/mgc/mgc_request.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index 2c48847..da130f4 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -1293,7 +1293,7 @@ static int mgc_process_recover_log(struct obd_device *obd,
 	struct page **pages;
 	int nrpages;
 	bool eof = true;
-	bool mne_swab = false;
+	bool mne_swab;
 	int i;
 	int ealen;
 	int rc;
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1290626 — [PATCH 7/7] staging: lustre: Rename a jump label for module_put() calls

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 13 Dec 2015 14:05:57 +0100

This issue was detected by using the Coccinelle software.

Choose a jump label according to the current Linux coding style convention.
I suggest to improve this implementation detail by the reuse of a script
like the following for the semantic patch language.

@rename_jump_label exists@
identifier target != put_module, work;
type return_type;
@@
 return_type work(...)
 {
 ... when any
 goto
-target
+put_module
 ;
 ... when any
-target
+put_module
 :
 module_put(...);
 ... when any
 }

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/lustre/lustre/llite/lloop.c     |  8 ++--
 drivers/staging/lustre/lustre/mdc/mdc_request.c | 52 ++++++++++++-------------
 drivers/staging/lustre/lustre/osc/osc_request.c | 24 ++++++------
 3 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/lloop.c b/drivers/staging/lustre/lustre/llite/lloop.c
index 420d391..ebeef3b 100644
--- a/drivers/staging/lustre/lustre/llite/lloop.c
+++ b/drivers/staging/lustre/lustre/llite/lloop.c
@@ -484,14 +484,14 @@ static int loop_set_fd(struct lloop_device *lo, struct file *unused,
 
 	error = -EBUSY;
 	if (lo->lo_state != LLOOP_UNBOUND)
-		goto out;
+		goto put_module;
 
 	mapping = file->f_mapping;
 	inode = mapping->host;
 
 	error = -EINVAL;
 	if (!S_ISREG(inode->i_mode) || inode->i_sb->s_magic != LL_SUPER_MAGIC)
-		goto out;
+		goto put_module;
 
 	if (!(file->f_mode & FMODE_WRITE))
 		lo_flags |= LO_FLAGS_READ_ONLY;
@@ -500,7 +500,7 @@ static int loop_set_fd(struct lloop_device *lo, struct file *unused,
 
 	if ((loff_t)(sector_t)size != size) {
 		error = -EFBIG;
-		goto out;
+		goto put_module;
 	}
 
 	/* remove all pages in cache so as dirty pages not to be existent. */
@@ -542,7 +542,7 @@ static int loop_set_fd(struct lloop_device *lo, struct file *unused,
 	down(&lo->lo_sem);
 	return 0;
 
-out:
+put_module:
 	/* This is safe: open() is still holding a reference. */
 	module_put(THIS_MODULE);
 	return error;
diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
index 2085ba6..eaeca9a 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
@@ -1734,7 +1734,7 @@ static int mdc_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
 	switch (cmd) {
 	case OBD_IOC_CHANGELOG_SEND:
 		rc = mdc_ioc_changelog_send(obd, karg);
-		goto out;
+		goto put_module;
 	case OBD_IOC_CHANGELOG_CLEAR: {
 		struct ioc_changelog *icc = karg;
 		struct changelog_setinfo cs = {
@@ -1745,47 +1745,47 @@ static int mdc_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
 		rc = obd_set_info_async(NULL, exp, strlen(KEY_CHANGELOG_CLEAR),
 					KEY_CHANGELOG_CLEAR, sizeof(cs), &cs,
 					NULL);
-		goto out;
+		goto put_module;
 	}
 	case OBD_IOC_FID2PATH:
 		rc = mdc_ioc_fid2path(exp, karg);
-		goto out;
+		goto put_module;
 	case LL_IOC_HSM_CT_START:
 		rc = mdc_ioc_hsm_ct_start(exp, karg);
 		/* ignore if it was already registered on this MDS. */
 		if (rc == -EEXIST)
 			rc = 0;
-		goto out;
+		goto put_module;
 	case LL_IOC_HSM_PROGRESS:
 		rc = mdc_ioc_hsm_progress(exp, karg);
-		goto out;
+		goto put_module;
 	case LL_IOC_HSM_STATE_GET:
 		rc = mdc_ioc_hsm_state_get(exp, karg);
-		goto out;
+		goto put_module;
 	case LL_IOC_HSM_STATE_SET:
 		rc = mdc_ioc_hsm_state_set(exp, karg);
-		goto out;
+		goto put_module;
 	case LL_IOC_HSM_ACTION:
 		rc = mdc_ioc_hsm_current_action(exp, karg);
-		goto out;
+		goto put_module;
 	case LL_IOC_HSM_REQUEST:
 		rc = mdc_ioc_hsm_request(exp, karg);
-		goto out;
+		goto put_module;
 	case OBD_IOC_CLIENT_RECOVER:
 		rc = ptlrpc_recover_import(imp, data->ioc_inlbuf1, 0);
 		if (rc < 0)
-			goto out;
+			goto put_module;
 		rc = 0;
-		goto out;
+		goto put_module;
 	case IOC_OSC_SET_ACTIVE:
 		rc = ptlrpc_set_import_active(imp, data->ioc_offset);
-		goto out;
+		goto put_module;
 	case OBD_IOC_POLL_QUOTACHECK:
 		rc = mdc_quota_poll_check(exp, (struct if_quotacheck *)karg);
-		goto out;
+		goto put_module;
 	case OBD_IOC_PING_TARGET:
 		rc = ptlrpc_obd_ping(obd);
-		goto out;
+		goto put_module;
 	/*
 	 * Normally IOC_OBD_STATFS, OBD_IOC_QUOTACTL iocontrol are handled by
 	 * LMV instead of MDC. But when the cluster is upgraded from 1.8,
@@ -1798,7 +1798,7 @@ static int mdc_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
 
 		if (*((__u32 *) data->ioc_inlbuf2) != 0) {
 			rc = -ENODEV;
-			goto out;
+			goto put_module;
 		}
 
 		/* copy UUID */
@@ -1806,24 +1806,24 @@ static int mdc_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
 				 min_t(size_t, data->ioc_plen2,
 					       sizeof(struct obd_uuid)))) {
 			rc = -EFAULT;
-			goto out;
+			goto put_module;
 		}
 
 		rc = mdc_statfs(NULL, obd->obd_self_export, &stat_buf,
 				cfs_time_shift_64(-OBD_STATFS_CACHE_SECONDS),
 				0);
 		if (rc != 0)
-			goto out;
+			goto put_module;
 
 		if (copy_to_user(data->ioc_pbuf1, &stat_buf,
 				 min_t(size_t, data->ioc_plen1,
 					       sizeof(stat_buf)))) {
 			rc = -EFAULT;
-			goto out;
+			goto put_module;
 		}
 
 		rc = 0;
-		goto out;
+		goto put_module;
 	}
 	case OBD_IOC_QUOTACTL: {
 		struct if_quotactl *qctl = karg;
@@ -1832,7 +1832,7 @@ static int mdc_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
 		oqctl = kzalloc(sizeof(*oqctl), GFP_NOFS);
 		if (!oqctl) {
 			rc = -ENOMEM;
-			goto out;
+			goto put_module;
 		}
 
 		QCTL_COPY(oqctl, qctl);
@@ -1844,26 +1844,26 @@ static int mdc_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
 		}
 
 		kfree(oqctl);
-		goto out;
+		goto put_module;
 	}
 	case LL_IOC_GET_CONNECT_FLAGS:
 		if (copy_to_user(uarg, exp_connect_flags_ptr(exp),
 				 sizeof(*exp_connect_flags_ptr(exp)))) {
 			rc = -EFAULT;
-			goto out;
+			goto put_module;
 		}
 
 		rc = 0;
-		goto out;
+		goto put_module;
 	case LL_IOC_LOV_SWAP_LAYOUTS:
 		rc = mdc_ioc_swap_layouts(exp, karg);
-		goto out;
+		goto put_module;
 	default:
 		CERROR("unrecognised ioctl: cmd = %#x\n", cmd);
 		rc = -ENOTTY;
-		goto out;
+		goto put_module;
 	}
-out:
+put_module:
 	module_put(THIS_MODULE);
 
 	return rc;
diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
index 3a56fb7..3ee1ff8 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -2611,7 +2611,7 @@ static int osc_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
 		len = 0;
 		if (obd_ioctl_getdata(&buf, &len, uarg)) {
 			err = -EINVAL;
-			goto out;
+			goto put_module;
 		}
 
 		data = (struct obd_ioctl_data *)buf;
@@ -2619,13 +2619,13 @@ static int osc_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
 		if (sizeof(*desc) > data->ioc_inllen1) {
 			obd_ioctl_freedata(buf, len);
 			err = -EINVAL;
-			goto out;
+			goto put_module;
 		}
 
 		if (data->ioc_inllen2 < sizeof(uuid)) {
 			obd_ioctl_freedata(buf, len);
 			err = -EINVAL;
-			goto out;
+			goto put_module;
 		}
 
 		desc = (struct lov_desc *)data->ioc_inlbuf1;
@@ -2643,39 +2643,39 @@ static int osc_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
 		if (err)
 			err = -EFAULT;
 		obd_ioctl_freedata(buf, len);
-		goto out;
+		goto put_module;
 	}
 	case LL_IOC_LOV_SETSTRIPE:
 		err = obd_alloc_memmd(exp, karg);
 		if (err > 0)
 			err = 0;
-		goto out;
+		goto put_module;
 	case LL_IOC_LOV_GETSTRIPE:
 		err = osc_getstripe(karg, uarg);
-		goto out;
+		goto put_module;
 	case OBD_IOC_CLIENT_RECOVER:
 		err = ptlrpc_recover_import(obd->u.cli.cl_import,
 					    data->ioc_inlbuf1, 0);
 		if (err > 0)
 			err = 0;
-		goto out;
+		goto put_module;
 	case IOC_OSC_SET_ACTIVE:
 		err = ptlrpc_set_import_active(obd->u.cli.cl_import,
 					       data->ioc_offset);
-		goto out;
+		goto put_module;
 	case OBD_IOC_POLL_QUOTACHECK:
 		err = osc_quota_poll_check(exp, karg);
-		goto out;
+		goto put_module;
 	case OBD_IOC_PING_TARGET:
 		err = ptlrpc_obd_ping(obd);
-		goto out;
+		goto put_module;
 	default:
 		CDEBUG(D_INODE, "unrecognised ioctl %#x by %s\n",
 		       cmd, current_comm());
 		err = -ENOTTY;
-		goto out;
+		goto put_module;
 	}
-out:
+put_module:
 	module_put(THIS_MODULE);
 	return err;
 }
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296151 — [PATCH v2 0/4] staging-Lustre: Fine-tuning for some function implementations

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 21 Dec 2015 20:00:02 +0100

Several update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
  Delete unnecessary goto statements in six functions
  Delete an unnecessary variable initialisation in mgc_process_recover_log()
  Less checks in mgc_process_recover_log() after error detection
  Fix a jump label position in osc_get_info()

 drivers/staging/lustre/lustre/llite/namei.c     |  1 -
 drivers/staging/lustre/lustre/mdc/mdc_request.c |  7 ----
 drivers/staging/lustre/lustre/mgc/mgc_request.c | 53 +++++++++++--------------
 drivers/staging/lustre/lustre/osc/osc_request.c |  2 +-
 4 files changed, 25 insertions(+), 38 deletions(-)

-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296153 — [PATCH v2 1/4] staging: lustre: Delete unnecessary goto statements in six functions

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 21 Dec 2015 18:15:45 +0100

Six goto statements referred to a source code position
directly behind them.
Thus omit such unnecessary jumps.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/lustre/lustre/llite/namei.c     | 1 -
 drivers/staging/lustre/lustre/mdc/mdc_request.c | 7 -------
 2 files changed, 8 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index 64db5e8..2113dd4 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -554,7 +554,6 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry,
 		retval = NULL;
 	else
 		retval = dentry;
-	goto out;
  out:
 	if (req)
 		ptlrpc_req_finished(req);
diff --git a/drivers/staging/lustre/lustre/mdc/mdc_request.c b/drivers/staging/lustre/lustre/mdc/mdc_request.c
index 294c050..920b1e9 100644
--- a/drivers/staging/lustre/lustre/mdc/mdc_request.c
+++ b/drivers/staging/lustre/lustre/mdc/mdc_request.c
@@ -1181,7 +1181,6 @@ static int mdc_ioc_hsm_progress(struct obd_export *exp,
 	ptlrpc_request_set_replen(req);
 
 	rc = mdc_queue_wait(req);
-	goto out;
 out:
 	ptlrpc_req_finished(req);
 	return rc;
@@ -1216,7 +1215,6 @@ static int mdc_ioc_hsm_ct_register(struct obd_import *imp, __u32 archives)
 	ptlrpc_request_set_replen(req);
 
 	rc = mdc_queue_wait(req);
-	goto out;
 out:
 	ptlrpc_req_finished(req);
 	return rc;
@@ -1282,7 +1280,6 @@ static int mdc_ioc_hsm_ct_unregister(struct obd_import *imp)
 	ptlrpc_request_set_replen(req);
 
 	rc = mdc_queue_wait(req);
-	goto out;
 out:
 	ptlrpc_req_finished(req);
 	return rc;
@@ -1362,8 +1359,6 @@ static int mdc_ioc_hsm_state_set(struct obd_export *exp,
 	ptlrpc_request_set_replen(req);
 
 	rc = mdc_queue_wait(req);
-	goto out;
-
 out:
 	ptlrpc_req_finished(req);
 	return rc;
@@ -1427,8 +1422,6 @@ static int mdc_ioc_hsm_request(struct obd_export *exp,
 	ptlrpc_request_set_replen(req);
 
 	rc = mdc_queue_wait(req);
-	goto out;
-
 out:
 	ptlrpc_req_finished(req);
 	return rc;
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296157 — [PATCH v2 4/4] staging: lustre: Fix a jump label position in osc_get_info()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 21 Dec 2015 19:30:42 +0100

The script "checkpatch.pl" pointed out that labels should not be indented.
Thus delete a horizontal tab before the jump label "out"
in the function "osc_get_info".

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/lustre/lustre/osc/osc_request.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
index d6c1447..85ab180 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -2727,7 +2727,7 @@ static int osc_get_info(const struct lu_env *env, struct obd_export *exp,
 		}
 
 		*((u64 *)val) = *reply;
-	out:
+out:
 		ptlrpc_req_finished(req);
 		return rc;
 	} else if (KEY_IS(KEY_FIEMAP)) {
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296160 — [PATCH v2 3/4] staging: lustre: Less checks in mgc_process_recover_log() after error detection

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 21 Dec 2015 18:58:51 +0100

A few checks would be performed by the mgc_process_recover_log() function
even though it was determined that the passed variable "pages" contained
a null pointer or a call of the alloc_page() function failed.

1. Let us return directly if a call of the kcalloc() function failed.

2. Corresponding implementation details could be improved by adjustments
   for jump targets according to the Linux coding style convention.

3. Delete sanity checks then.

4. Move an assignment for the variable "eof" behind memory allocations.

5. The variable "req" will eventually be set to an appropriate pointer
   from a call of the ptlrpc_request_alloc() function.
   Thus let us omit the explicit initialisation before.

6. Apply a recommendation from the script "checkpatch.pl".

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/lustre/lustre/mgc/mgc_request.c | 51 +++++++++++--------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index da130f4..5f581df 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -1285,14 +1285,14 @@ static int mgc_apply_recover_logs(struct obd_device *mgc,
 static int mgc_process_recover_log(struct obd_device *obd,
 				   struct config_llog_data *cld)
 {
-	struct ptlrpc_request *req = NULL;
+	struct ptlrpc_request *req;
 	struct config_llog_instance *cfg = &cld->cld_cfg;
 	struct mgs_config_body *body;
 	struct mgs_config_res  *res;
 	struct ptlrpc_bulk_desc *desc;
 	struct page **pages;
 	int nrpages;
-	bool eof = true;
+	bool eof;
 	bool mne_swab;
 	int i;
 	int ealen;
@@ -1309,19 +1309,18 @@ static int mgc_process_recover_log(struct obd_device *obd,
 		nrpages = CONFIG_READ_NRPAGES_INIT;
 
 	pages = kcalloc(nrpages, sizeof(*pages), GFP_KERNEL);
-	if (pages == NULL) {
-		rc = -ENOMEM;
-		goto out;
-	}
+	if (!pages)
+		return -ENOMEM;
 
 	for (i = 0; i < nrpages; i++) {
 		pages[i] = alloc_page(GFP_KERNEL);
 		if (pages[i] == NULL) {
 			rc = -ENOMEM;
-			goto out;
+			goto free_pages;
 		}
 	}
 
+	eof = true;
 again:
 	LASSERT(cld_is_recover(cld));
 	LASSERT(mutex_is_locked(&cld->cld_lock));
@@ -1329,12 +1328,12 @@ again:
 				   &RQF_MGS_CONFIG_READ);
 	if (req == NULL) {
 		rc = -ENOMEM;
-		goto out;
+		goto free_pages;
 	}
 
 	rc = ptlrpc_request_pack(req, LUSTRE_MGS_VERSION, MGS_CONFIG_READ);
 	if (rc)
-		goto out;
+		goto finish_request;
 
 	/* pack request */
 	body = req_capsule_client_get(&req->rq_pill, &RMF_MGS_CONFIG_BODY);
@@ -1343,7 +1342,7 @@ again:
 	if (strlcpy(body->mcb_name, cld->cld_logname, sizeof(body->mcb_name))
 	    >= sizeof(body->mcb_name)) {
 		rc = -E2BIG;
-		goto out;
+		goto finish_request;
 	}
 	body->mcb_offset = cfg->cfg_last_idx + 1;
 	body->mcb_type   = cld->cld_type;
@@ -1355,7 +1354,7 @@ again:
 				    MGS_BULK_PORTAL);
 	if (desc == NULL) {
 		rc = -ENOMEM;
-		goto out;
+		goto finish_request;
 	}
 
 	for (i = 0; i < nrpages; i++)
@@ -1364,12 +1363,12 @@ again:
 	ptlrpc_request_set_replen(req);
 	rc = ptlrpc_queue_wait(req);
 	if (rc)
-		goto out;
+		goto finish_request;
 
 	res = req_capsule_server_get(&req->rq_pill, &RMF_MGS_CONFIG_RES);
 	if (res->mcr_size < res->mcr_offset) {
 		rc = -EINVAL;
-		goto out;
+		goto finish_request;
 	}
 
 	/* always update the index even though it might have errors with
@@ -1383,18 +1382,18 @@ again:
 	ealen = sptlrpc_cli_unwrap_bulk_read(req, req->rq_bulk, 0);
 	if (ealen < 0) {
 		rc = ealen;
-		goto out;
+		goto finish_request;
 	}
 
 	if (ealen > nrpages << PAGE_CACHE_SHIFT) {
 		rc = -EINVAL;
-		goto out;
+		goto finish_request;
 	}
 
 	if (ealen == 0) { /* no logs transferred */
 		if (!eof)
 			rc = -EINVAL;
-		goto out;
+		goto finish_request;
 	}
 
 	mne_swab = !!ptlrpc_rep_need_swab(req);
@@ -1424,22 +1423,18 @@ again:
 
 		ealen -= PAGE_CACHE_SIZE;
 	}
-
-out:
-	if (req)
-		ptlrpc_req_finished(req);
+finish_request:
+	ptlrpc_req_finished(req);
 
 	if (rc == 0 && !eof)
 		goto again;
-
-	if (pages) {
-		for (i = 0; i < nrpages; i++) {
-			if (pages[i] == NULL)
-				break;
-			__free_page(pages[i]);
-		}
-		kfree(pages);
+free_pages:
+	for (i = 0; i < nrpages; i++) {
+		if (!(pages[i]))
+			break;
+		__free_page(pages[i]);
 	}
+	kfree(pages);
 	return rc;
 }
 
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296315 — Re: [PATCH v2 3/4] staging: lustre: Less checks in mgc_process_recover_log() after error detection

On Mon, Dec 21, 2015 at 08:12:12PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 21 Dec 2015 18:58:51 +0100
> 
> A few checks would be performed by the mgc_process_recover_log() function
> even though it was determined that the passed variable "pages" contained
> a null pointer or a call of the alloc_page() function failed.
> 
> 1. Let us return directly if a call of the kcalloc() function failed.
> 
> 2. Corresponding implementation details could be improved by adjustments
>    for jump targets according to the Linux coding style convention.
> 
> 3. Delete sanity checks then.
> 
> 4. Move an assignment for the variable "eof" behind memory allocations.
> 
> 5. The variable "req" will eventually be set to an appropriate pointer
>    from a call of the ptlrpc_request_alloc() function.
>    Thus let us omit the explicit initialisation before.
> 
> 6. Apply a recommendation from the script "checkpatch.pl".

That's 6 different things, shouldn't this be 6 different patches?

please redo.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296586 — Re: [PATCH v2 3/4] staging: lustre: Less checks in mgc_process_recover_log() after error detection

>> 6. Apply a recommendation from the script "checkpatch.pl".
> 
> That's 6 different things, shouldn't this be 6 different patches?
> 
> please redo.

Dan Carpenter requested to squash the previous update steps 5 and 6
into a single patch for better source code review.
Now I see further software development challenges to increase
the patch granularity even more as you suggest.

Which route would Lustre developers like to follow?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296608 — Re: [PATCH v2 3/4] staging: lustre: Less checks in mgc_process_recover_log() after error detection

On Mon, Dec 21, 2015 at 03:48:57PM -0800, Greg Kroah-Hartman wrote:
> 
> That's 6 different things, shouldn't this be 6 different patches?
> 

Not really.  The patch could be described as just "change from using one
exit label to using several."  Markus has sent a number of these patches
and I am CC'd on them because of kernel-janitors, it's really painful to
review when he breaks them up into tiny patches where he changes one
label at a time.  It's like trying to put coleslaw back together into a
head of cabbage.

> On Mon, Dec 21, 2015 at 08:12:12PM +0100, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Mon, 21 Dec 2015 18:58:51 +0100
> > 
> > A few checks would be performed by the mgc_process_recover_log() function
> > even though it was determined that the passed variable "pages" contained
> > a null pointer or a call of the alloc_page() function failed.
> > 
> > 1. Let us return directly if a call of the kcalloc() function failed.
> > 
> > 2. Corresponding implementation details could be improved by adjustments
> >    for jump targets according to the Linux coding style convention.
> > 
> > 3. Delete sanity checks then.

These are not sanity checks, of course.  They were required because of a
common exit path.

> > 
> > 4. Move an assignment for the variable "eof" behind memory allocations.

I had asked Markus not to do this.  It is unrelated.

> > 
> > 5. The variable "req" will eventually be set to an appropriate pointer
> >    from a call of the ptlrpc_request_alloc() function.
> >    Thus let us omit the explicit initialisation before.

Now that we use multiple labels it isn't necessary to initialize "req".

> > 
> > 6. Apply a recommendation from the script "checkpatch.pl".

This is where he changed pages[i] == NULL to !(pages[i]).  It's not
strictly related but it's minor and he was changing those lines anyway.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1296161 — [PATCH v2 2/4] staging: lustre: Delete an unnecessary variable initialisation in mgc_process_recover_log()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 21 Dec 2015 18:24:45 +0100

The variable "mne_swab" will eventually be set to an appropriate value
from a call of the ptlrpc_rep_need_swab() function.
Thus let us omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/lustre/lustre/mgc/mgc_request.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index 2c48847..da130f4 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -1293,7 +1293,7 @@ static int mgc_process_recover_log(struct obd_device *obd,
 	struct page **pages;
 	int nrpages;
 	bool eof = true;
-	bool mne_swab = false;
+	bool mne_swab;
 	int i;
 	int ealen;
 	int rc;
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web