From: Dan Carpenter <dan.carpenter@oracle.com>
To: lustre-devel@lists.lustre.org
Subject: [lustre-devel] [PATCH 2/3] staging:lustre: remove kernel defines in userland headers
Date: Fri, 22 May 2015 16:00:19 +0300 [thread overview]
Message-ID: <20150522130019.GN4150@mwanda> (raw)
In-Reply-To: <1432248378-28912-3-git-send-email-jsimmons@infradead.org>
This patch seems fine but it would also be better if it were broken up
into easy to review patches.
[patch 1] delete LNET_USE_LIB_FREELIST code
This is theortetically what this patch does except it does tons
of other things as well.
Patches which delete whole functions, #defines or from #ifdef to #endif
are normally easy to review. When there are + lines it is hard.
[patch 2] move things around (i guess)?
[patch 3] white space fixes
[patch 4] get rid of lnet_eq_free_locked() and macro friends.
[patch 5] rename CamelCase functions
[patch 6] clean up lnet_msg_container_setup()
> typedef struct lnet_peer {
> - struct list_head lp_hashlist; /* chain on peer hash */
> - struct list_head lp_txq; /* messages blocking for tx credits */
> - struct list_head lp_rtrq; /* messages blocking for router credits */
> - struct list_head lp_rtr_list; /* chain on router list */
> - int lp_txcredits; /* # tx credits available */
> - int lp_mintxcredits; /* low water mark */
> - int lp_rtrcredits; /* # router credits */
> - int lp_minrtrcredits; /* low water mark */
> - unsigned int lp_alive:1; /* alive/dead? */
> - unsigned int lp_notify:1; /* notification outstanding? */
> - unsigned int lp_notifylnd:1; /* outstanding notification for LND? */
> - unsigned int lp_notifying:1; /* some thread is handling notification */
> - unsigned int lp_ping_notsent; /* SEND event outstanding from ping */
> - int lp_alive_count; /* # times router went dead<->alive */
> - long lp_txqnob; /* bytes queued for sending */
> - unsigned long lp_timestamp; /* time of last aliveness news */
> - unsigned long lp_ping_timestamp; /* time of last ping attempt */
> - unsigned long lp_ping_deadline; /* != 0 if ping reply expected */
> - unsigned long lp_last_alive; /* when I was last alive */
> - unsigned long lp_last_query; /* when lp_ni was queried last time */
> - lnet_ni_t *lp_ni; /* interface peer is on */
> - lnet_nid_t lp_nid; /* peer's NID */
> - int lp_refcount; /* # refs */
> - int lp_cpt; /* CPT this peer attached on */
> + /* chain on peer hash */
> + struct list_head lp_hashlist;
> + /* messages blocking for tx credits */
> + struct list_head lp_txq;
> + /* messages blocking for router credits */
> + struct list_head lp_rtrq;
> + /* chain on router list */
> + struct list_head lp_rtr_list;
> + /* # tx credits available */
> + int lp_txcredits;
> + /* low water mark */
> + int lp_mintxcredits;
> + /* # router credits */
> + int lp_rtrcredits;
> + /* low water mark */
> + int lp_minrtrcredits;
> + /* alive/dead? */
> + unsigned int lp_alive:1;
> + /* notification outstanding? */
> + unsigned int lp_notify:1;
> + /* outstanding notification for LND? */
> + unsigned int lp_notifylnd:1;
> + /* some thread is handling notification */
> + unsigned int lp_notifying:1;
> + /* SEND event outstanding from ping */
> + unsigned int lp_ping_notsent;
> + /* # times router went dead<->alive */
> + int lp_alive_count;
> + /* bytes queued for sending */
> + long lp_txqnob;
> + /* time of last aliveness news */
> + unsigned long lp_timestamp;
> + /* time of last ping attempt */
> + unsigned long lp_ping_timestamp;
> + /* != 0 if ping reply expected */
> + unsigned long lp_ping_deadline;
> + /* when I was last alive */
> + unsigned long lp_last_alive;
> + /* when lp_ni was queried last time */
> + unsigned long lp_last_query;
> + /* interface peer is on */
> + lnet_ni_t *lp_ni;
> + /* peer's NID */
> + lnet_nid_t lp_nid;
> + /* # refs */
> + int lp_refcount;
> + /* CPT this peer attached on */
> + int lp_cpt;
This new block of declarations is uglier than the original. Don't make
things uglier.
regards,
dan carpenter
WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: James Simmons <jsimmons@infradead.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
devel@driverdev.osuosl.org, Oleg Drokin <oleg.drokin@intel.com>,
Andreas Dilger <andreas.dilger@intel.com>,
HPDD-discuss@ml01.01.org, James Simmons <uja.ornl@gmail.com>,
James Simmons <uja.ornl@yahoo.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
lustre-devel@lists.lustre.org
Subject: Re: [PATCH 2/3] staging:lustre: remove kernel defines in userland headers
Date: Fri, 22 May 2015 16:00:19 +0300 [thread overview]
Message-ID: <20150522130019.GN4150@mwanda> (raw)
In-Reply-To: <1432248378-28912-3-git-send-email-jsimmons@infradead.org>
This patch seems fine but it would also be better if it were broken up
into easy to review patches.
[patch 1] delete LNET_USE_LIB_FREELIST code
This is theortetically what this patch does except it does tons
of other things as well.
Patches which delete whole functions, #defines or from #ifdef to #endif
are normally easy to review. When there are + lines it is hard.
[patch 2] move things around (i guess)?
[patch 3] white space fixes
[patch 4] get rid of lnet_eq_free_locked() and macro friends.
[patch 5] rename CamelCase functions
[patch 6] clean up lnet_msg_container_setup()
> typedef struct lnet_peer {
> - struct list_head lp_hashlist; /* chain on peer hash */
> - struct list_head lp_txq; /* messages blocking for tx credits */
> - struct list_head lp_rtrq; /* messages blocking for router credits */
> - struct list_head lp_rtr_list; /* chain on router list */
> - int lp_txcredits; /* # tx credits available */
> - int lp_mintxcredits; /* low water mark */
> - int lp_rtrcredits; /* # router credits */
> - int lp_minrtrcredits; /* low water mark */
> - unsigned int lp_alive:1; /* alive/dead? */
> - unsigned int lp_notify:1; /* notification outstanding? */
> - unsigned int lp_notifylnd:1; /* outstanding notification for LND? */
> - unsigned int lp_notifying:1; /* some thread is handling notification */
> - unsigned int lp_ping_notsent; /* SEND event outstanding from ping */
> - int lp_alive_count; /* # times router went dead<->alive */
> - long lp_txqnob; /* bytes queued for sending */
> - unsigned long lp_timestamp; /* time of last aliveness news */
> - unsigned long lp_ping_timestamp; /* time of last ping attempt */
> - unsigned long lp_ping_deadline; /* != 0 if ping reply expected */
> - unsigned long lp_last_alive; /* when I was last alive */
> - unsigned long lp_last_query; /* when lp_ni was queried last time */
> - lnet_ni_t *lp_ni; /* interface peer is on */
> - lnet_nid_t lp_nid; /* peer's NID */
> - int lp_refcount; /* # refs */
> - int lp_cpt; /* CPT this peer attached on */
> + /* chain on peer hash */
> + struct list_head lp_hashlist;
> + /* messages blocking for tx credits */
> + struct list_head lp_txq;
> + /* messages blocking for router credits */
> + struct list_head lp_rtrq;
> + /* chain on router list */
> + struct list_head lp_rtr_list;
> + /* # tx credits available */
> + int lp_txcredits;
> + /* low water mark */
> + int lp_mintxcredits;
> + /* # router credits */
> + int lp_rtrcredits;
> + /* low water mark */
> + int lp_minrtrcredits;
> + /* alive/dead? */
> + unsigned int lp_alive:1;
> + /* notification outstanding? */
> + unsigned int lp_notify:1;
> + /* outstanding notification for LND? */
> + unsigned int lp_notifylnd:1;
> + /* some thread is handling notification */
> + unsigned int lp_notifying:1;
> + /* SEND event outstanding from ping */
> + unsigned int lp_ping_notsent;
> + /* # times router went dead<->alive */
> + int lp_alive_count;
> + /* bytes queued for sending */
> + long lp_txqnob;
> + /* time of last aliveness news */
> + unsigned long lp_timestamp;
> + /* time of last ping attempt */
> + unsigned long lp_ping_timestamp;
> + /* != 0 if ping reply expected */
> + unsigned long lp_ping_deadline;
> + /* when I was last alive */
> + unsigned long lp_last_alive;
> + /* when lp_ni was queried last time */
> + unsigned long lp_last_query;
> + /* interface peer is on */
> + lnet_ni_t *lp_ni;
> + /* peer's NID */
> + lnet_nid_t lp_nid;
> + /* # refs */
> + int lp_refcount;
> + /* CPT this peer attached on */
> + int lp_cpt;
This new block of declarations is uglier than the original. Don't make
things uglier.
regards,
dan carpenter
next prev parent reply other threads:[~2015-05-22 13:00 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <uja.ornl@gmail.com>
2015-05-21 22:46 ` [lustre-devel] [PATCH 0/3] First set of Intel branch merger for libcfs/lnet James Simmons
2015-05-21 22:46 ` James Simmons
2015-05-21 22:46 ` [lustre-devel] [PATCH 1/3] staging:lustre: remove tcpip abstraction from libcfs James Simmons
2015-05-21 22:46 ` James Simmons
2015-05-22 11:15 ` [lustre-devel] " Dan Carpenter
2015-05-22 11:15 ` Dan Carpenter
2015-05-22 15:08 ` [lustre-devel] " Simmons, James A.
2015-05-22 15:08 ` Simmons, James A.
2015-05-22 15:39 ` Dan Carpenter
2015-05-22 15:39 ` Dan Carpenter
2015-05-22 21:40 ` Greg Kroah-Hartman
2015-05-22 21:40 ` Greg Kroah-Hartman
2015-05-21 22:46 ` [lustre-devel] [PATCH 2/3] staging:lustre: remove kernel defines in userland headers James Simmons
2015-05-21 22:46 ` James Simmons
2015-05-22 13:00 ` Dan Carpenter [this message]
2015-05-22 13:00 ` Dan Carpenter
2015-05-22 15:12 ` [lustre-devel] " Simmons, James A.
2015-05-22 15:12 ` Simmons, James A.
2015-05-22 15:24 ` Joe Perches
2015-05-22 15:24 ` Joe Perches
2015-05-21 22:46 ` [lustre-devel] [PATCH 3/3] staging:lustre: cleanup libcfs lock handling James Simmons
2015-05-21 22:46 ` James Simmons
2015-05-22 18:32 ` [lustre-devel] [PATCH 0/6] staging:lustre: remove tcpip abstraction from libcfs James Simmons
2015-05-22 18:32 ` James Simmons
2015-05-22 18:32 ` [lustre-devel] [PATCH 1/6] staging:lustre:remove useless libcfs_sock_release James Simmons
2015-05-22 18:32 ` James Simmons
2015-05-22 18:32 ` [lustre-devel] [PATCH 2/6] staging:lustre:remove useless libcfs_sock_abort_accept James Simmons
2015-05-22 18:32 ` James Simmons
2015-05-22 18:32 ` [lustre-devel] [PATCH 3/6] staging:lustre: rename tcpip handling functions to lnet_* prefix James Simmons
2015-05-22 18:32 ` James Simmons
2015-05-22 18:32 ` [lustre-devel] [PATCH 4/6] staging:lustre: use available kernel wrappers in lib-socket.c James Simmons
2015-05-22 18:32 ` James Simmons
2015-05-22 18:32 ` [lustre-devel] [PATCH 5/6] staging:lustre: style cleanups for lib-socket.c James Simmons
2015-05-22 18:32 ` James Simmons
2015-05-25 9:37 ` [lustre-devel] " Dan Carpenter
2015-05-25 9:37 ` Dan Carpenter
2015-05-27 15:01 ` [lustre-devel] " Simmons, James A.
2015-05-27 15:01 ` Simmons, James A.
2015-05-27 15:24 ` Dan Carpenter
2015-05-27 15:24 ` Dan Carpenter
2015-05-27 21:04 ` Simmons, James A.
2015-05-27 21:04 ` Simmons, James A.
2015-05-22 18:32 ` [lustre-devel] [PATCH 6/6] staging:lustre: Update license and copyright " James Simmons
2015-05-22 18:32 ` 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=20150522130019.GN4150@mwanda \
--to=dan.carpenter@oracle.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.