Re: [PATCH] Staging: lustre: o2iblnd.c: Finished cleaning code style

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 02, 2015 at 07:52:07PM +0200, Guillaume Matheron wrote:
> I fixed lines over 80 characters and unnecessary returns
> 
> Signed-off-by: Guillaume Matheron <guillaume.matheron@xxxxxx>
> ---
>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    | 53 +++++++++++++---------
>  1 file changed, 31 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> index ae4069e..14c1d2b 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> @@ -559,8 +559,9 @@ static kib_conn_t *kiblnd_get_conn_by_idx(lnet_ni_t *ni, int index)
>  				conn = list_entry(ctmp, kib_conn_t,
>  						      ibc_list);
>  				kiblnd_conn_addref(conn);
> -				read_unlock_irqrestore(&kiblnd_data.kib_global_lock,
> -						       flags);
> +				read_unlock_irqrestore(
> +					&kiblnd_data.kib_global_lock,
> +					flags);

The original was basically fine and the new one is also ok.

>  				return conn;
>  			}
>  		}
> @@ -879,7 +880,8 @@ void kiblnd_destroy_conn(kib_conn_t *conn)
>  
>  	if (conn->ibc_rxs != NULL) {
>  		LIBCFS_FREE(conn->ibc_rxs,
> -			    IBLND_RX_MSGS(conn->ibc_version) * sizeof(kib_rx_t));
> +			    IBLND_RX_MSGS(conn->ibc_version)
> +			      * sizeof(kib_rx_t));

Actually, could you move the * to the first line and align it like.

		LIBCFS_FREE(conn->ibc_rxs,
			    IBLND_RX_MSGS(conn->ibc_version) *
			    sizeof(kib_rx_t));

[tab][tab][tab][space][space][space][space]

>  	}
>  
>  	if (conn->ibc_connvars != NULL)
> @@ -936,7 +938,8 @@ int kiblnd_close_stale_conns_locked(kib_peer_t *peer,
>  		    conn->ibc_incarnation == incarnation)
>  			continue;
>  
> -		CDEBUG(D_NET, "Closing stale conn -> %s version: %x, incarnation:%#llx(%x, %#llx)\n",
> +		CDEBUG(D_NET,
> +		       "Closing stale conn -> %s version: %x, incarnation:%#llx(%x, %#llx)\n",
>  		       libcfs_nid2str(peer->ibp_nid),
>  		       conn->ibc_version, conn->ibc_incarnation,
>  		       version, incarnation);
> @@ -1079,7 +1082,6 @@ void kiblnd_query(lnet_ni_t *ni, lnet_nid_t nid, unsigned long *when)
>  	CDEBUG(D_NET, "Peer %s %p, alive %ld secs ago\n",
>  	       libcfs_nid2str(nid), peer,
>  	       last_alive ? cfs_duration_sec(now - last_alive) : -1);
> -	return;
>  }
>  
>  void kiblnd_free_pages(kib_pages_t *p)
> @@ -1166,7 +1168,8 @@ void kiblnd_map_rx_descs(kib_conn_t *conn)
>  		rx->rx_msg = (kib_msg_t *)(((char *)page_address(pg)) + pg_off);
>  
>  		rx->rx_msgaddr = kiblnd_dma_map_single(conn->ibc_hdev->ibh_ibdev,
> -						       rx->rx_msg, IBLND_MSG_SIZE,
> +						       rx->rx_msg,
> +						       IBLND_MSG_SIZE,
>  						       DMA_FROM_DEVICE);
>  		LASSERT(!kiblnd_dma_mapping_error(conn->ibc_hdev->ibh_ibdev,
>  						   rx->rx_msgaddr));
> @@ -1446,8 +1449,9 @@ static void kiblnd_fini_fmr_poolset(kib_fmr_poolset_t *fps)
>  	}
>  }
>  
> -static int kiblnd_init_fmr_poolset(kib_fmr_poolset_t *fps, int cpt, kib_net_t *net,
> -				   int pool_size, int flush_trigger)
> +static int kiblnd_init_fmr_poolset(kib_fmr_poolset_t *fps, int cpt,
> +				   kib_net_t *net, int pool_size,
> +				   int flush_trigger)
>  {
>  	kib_fmr_pool_t *fpo;
>  	int	     rc;
> @@ -1557,7 +1561,8 @@ int kiblnd_fmr_pool_map(kib_fmr_poolset_t *fps, __u64 *pages, int npages,
>  
>  	if (fps->fps_increasing) {
>  		spin_unlock(&fps->fps_lock);
> -		CDEBUG(D_NET, "Another thread is allocating new FMR pool, waiting for her to complete\n");
> +		CDEBUG(D_NET,
> +			"Another thread is allocating new FMR pool, waiting for her to complete\n");


Slightly better to align it like:

		CDEBUG(D_NET,
		       "Another thread is allocating new FMR pool, waiting for her to complete\n");


[tab][tab][space][space][space][space][space][space][space]"Another

> @@ -2808,7 +2813,8 @@ static int kiblnd_base_startup(void)
>  	LASSERT(kiblnd_data.kib_init == IBLND_INIT_NOTHING);
>  
>  	try_module_get(THIS_MODULE);
> -	memset(&kiblnd_data, 0, sizeof(kiblnd_data)); /* zero pointers, flags etc */
> +	/* zero pointers, flags etc */

These comments are obvious.  Just delete them.

> +	memset(&kiblnd_data, 0, sizeof(kiblnd_data));
>  
>  	rwlock_init(&kiblnd_data.kib_global_lock);
>  
> @@ -3089,10 +3096,12 @@ static int __init kiblnd_module_init(void)
>  	int    rc;
>  
>  	CLASSERT(sizeof(kib_msg_t) <= IBLND_MSG_SIZE);
> -	CLASSERT(offsetof(kib_msg_t, ibm_u.get.ibgm_rd.rd_frags[IBLND_MAX_RDMA_FRAGS])
> -		  <= IBLND_MSG_SIZE);
> -	CLASSERT(offsetof(kib_msg_t, ibm_u.putack.ibpam_rd.rd_frags[IBLND_MAX_RDMA_FRAGS])
> -		  <= IBLND_MSG_SIZE);
> +	CLASSERT(offsetof(kib_msg_t,
> +		ibm_u.get.ibgm_rd.rd_frags[IBLND_MAX_RDMA_FRAGS])
> +		<= IBLND_MSG_SIZE);
> +	CLASSERT(offsetof(kib_msg_t,
> +		ibm_u.putack.ibpam_rd.rd_frags[IBLND_MAX_RDMA_FRAGS])
> +		<= IBLND_MSG_SIZE);

Put the <= on the line before.

>  
>  	rc = kiblnd_tunables_init();
>  	if (rc != 0)

You're also mixing a bunch of different types of cleanups together and
some weren't mentioned in the changelog.  Probably better to split it
into multiple patches.

regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux