Lustre-devel archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Simmons, James A. <simmonsja@ornl.gov>
To: lustre-devel@lists.lustre.org
Subject: [lustre-devel] [PATCH 21/40] staging: lustre: improve LNet clean up code and API
Date: Tue, 15 Dec 2015 17:10:39 +0000	[thread overview]
Message-ID: <05bc05cb164b4d249f74683aef9c9d72@EXCHCS32.ornl.gov> (raw)
In-Reply-To: <20151202125440.GS18797@mwanda>

>Actually we're going to have to redo so much code that it's not worth it
>for me to review the rest of these patches.  

Sorry I didn't get back to you sooner but I was on vacation.  Thanks for 
reviewing this work. Especially since this is the first major bug fixing merge
for the lustre client which means a lot of pain involved to iron out how to
do this. I have been pondering if pushing bug fixes before style cleanups
is the right thing to do. I pushed a bunch of bug fixes earlier and none got
merged which either means Greg is just backed up and hasn't the time to
merge them or  style issues are higher priority. Assuming these bug fixes are
in scope of the staging tree. Should I continue to push this work first?
Well either way I should update this patch series so it ready to merge at some
point.

>Please just look over everything again:
>
> BAD:	return -1;
>GOOD:	return -EINVAL;
>
> BAD:  failed0:
>GOOD:  free_something:
>
> BAD:	if (rc != 0)
>GOOD:	if (rc)
>
>Do one thing per patch.
>Do not introduce a bug and then fix it in a later patch.
>Check ioc_len more carefully.
>Don't make the code look ugly just to please checkpatch.pl.
>Do error handling not success handling.
>Try to avoid indenting a far to the right.

Okay. Will start to do the patch cleanup.

  parent reply	other threads:[~2015-12-15 17:10 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-20 23:35 [lustre-devel] [PATCH 00/40] Sync upstream lustre client LNet core James Simmons
2015-11-20 23:35 ` [lustre-devel] [PATCH 01/40] staging: lustre: drop *_t from end of struct lnet_text_buf James Simmons
2015-11-20 23:35 ` [lustre-devel] [PATCH 02/40] staging: lustre: fix 'NULL pointer dereference' errors for LNet James Simmons
2015-12-02  7:46   ` Dan Carpenter
2015-12-15 18:08     ` Simmons, James A.
2015-11-20 23:35 ` [lustre-devel] [PATCH 03/40] staging: lustre: reflect down routes in /proc/sys/lnet/routes James Simmons
2015-12-02  7:54   ` Dan Carpenter
2015-11-20 23:35 ` [lustre-devel] [PATCH 04/40] staging: lustre: fix failure handle of create reply James Simmons
2015-11-20 23:35 ` [lustre-devel] [PATCH 05/40] staging: lustre: eliminate obsolete Cray SeaStar support James Simmons
2015-11-20 23:35 ` [lustre-devel] [PATCH 06/40] staging: lustre: remove uses of IS_ERR_VALUE() James Simmons
2015-11-21 18:45   ` Dan Carpenter
2015-11-20 23:35 ` [lustre-devel] [PATCH 07/40] staging: lustre: return +ve for blocked lnet message James Simmons
2015-11-20 23:35 ` [lustre-devel] [PATCH 08/40] staging: lustre: do not memset after LIBCFS_ALLOC James Simmons
2015-11-20 23:35 ` [lustre-devel] [PATCH 09/40] staging: lustre: Dynamic LNet Configuration (DLC) James Simmons
2015-11-20 23:35 ` [lustre-devel] [PATCH 10/40] staging: lustre: Dynamic LNet Configuration (DLC) dynamic routing James Simmons
2015-11-20 23:35 ` [lustre-devel] [PATCH 11/40] staging: lustre: DLC Feature dynamic net config James Simmons
2015-12-02  9:23   ` Dan Carpenter
2015-11-20 23:35 ` [lustre-devel] [PATCH 12/40] staging: lustre: Dynamic LNet Configuration (DLC) IOCTL changes James Simmons
2015-12-02  9:48   ` Dan Carpenter
2015-11-20 23:35 ` [lustre-devel] [PATCH 13/40] staging: lustre: Dynamic LNet Configuration (DLC) show command James Simmons
2015-12-02 11:20   ` Dan Carpenter
2015-12-15 18:14     ` Simmons, James A.
2015-12-15 18:19       ` Dan Carpenter
2015-12-15 18:39         ` Simmons, James A.
2015-12-15 18:48       ` Greg Kroah-Hartman
2015-12-15 19:48         ` Simmons, James A.
2015-12-15 19:55           ` 'Greg Kroah-Hartman'
2015-12-02 12:00   ` Dan Carpenter
2015-11-20 23:35 ` [lustre-devel] [PATCH 14/40] staging: lustre: fix crash due to NULL networks string James Simmons
2015-12-02 11:27   ` Dan Carpenter
2015-11-20 23:35 ` [lustre-devel] [PATCH 15/40] staging: lustre: DLC user/kernel space glue code James Simmons
2015-12-02 12:11   ` Dan Carpenter
2015-11-20 23:35 ` [lustre-devel] [PATCH 16/40] staging: lustre: make local functions static for LNet ni James Simmons
2015-11-20 23:35 ` [lustre-devel] [PATCH 17/40] staging: lustre: add sparse annotation __user wherever needed for lnet James Simmons
2015-11-20 23:35 ` [lustre-devel] [PATCH 18/40] staging: lustre: remove LUSTRE_{, SRV_}LNET_PID James Simmons
2015-11-20 23:35 ` [lustre-devel] [PATCH 19/40] staging: lustre: copy out libcfs ioctl inline buffer James Simmons
2015-12-02 12:34   ` Dan Carpenter
2015-11-20 23:35 ` [lustre-devel] [PATCH 20/40] staging: lustre: fix kernel crash when network failed to start James Simmons
2015-12-02 12:44   ` Dan Carpenter
2015-11-20 23:35 ` [lustre-devel] [PATCH 21/40] staging: lustre: improve LNet clean up code and API James Simmons
2015-12-02 12:59   ` Dan Carpenter
2015-12-02 13:20     ` Alexander Zarochentsev
2015-12-02 13:59       ` Dan Carpenter
2015-12-15 17:10     ` Simmons, James A. [this message]
2015-12-15 17:41       ` Dan Carpenter
2015-11-20 23:35 ` [lustre-devel] [PATCH 22/40] staging: lustre: Fixes to make lnetctl function as expected James Simmons
2015-11-20 23:35 ` [lustre-devel] [PATCH 23/40] staging: lustre: return appropriate errno when adding route James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 24/40] staging: lustre: make some lnet functions static James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 25/40] staging: lustre: missed a few cases of using NULL instead of 0 James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 26/40] staging: lustre: startup lnet acceptor thread dynamically James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 27/40] staging: lustre: reject invalid net configuration for lnet James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 28/40] staging: lustre: return -EEXIST if NI is not unique James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 29/40] staging: lustre: handle lnet_check_routes() errors James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 30/40] staging: lustre: improvement to router checker James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 31/40] staging: lustre: assume a kernel build James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 32/40] staging: lustre: prevent assert on LNet module unload James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 33/40] staging: lustre: remove messages from lazy portal on NI shutdown James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 34/40] staging: lustre: remove unnecessary EXPORT_SYMBOL from lnet layer James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 35/40] staging: lustre: avoid race during lnet acceptor thread termination James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 36/40] staging: lustre: test for sk_sleep presence in compact-2.6.h James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 37/40] staging: lustre: remove unnecessary NULL check in IOC_LIBCFS_GET_NET James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 38/40] staging: lustre: Allocate the correct number of rtr buffers James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 39/40] staging: lustre: Use lnet_is_route_alive for router aliveness James Simmons
2015-11-20 23:36 ` [lustre-devel] [PATCH 40/40] staging: lustre: Remove LASSERTS from router checker James Simmons

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=05bc05cb164b4d249f74683aef9c9d72@EXCHCS32.ornl.gov \
    --to=simmonsja@ornl.gov \
    --cc=lustre-devel@lists.lustre.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox