[lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[James Simmons]

Latest testing fails when using ko2iblnd. It was tracked down
to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.

This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
---
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
index 89f9390..0d32e65 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
@@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net, __u32 *cpts, int ncpts)
 	 */
 
 	net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
-					   sizeof(*net->ibn_fmr_ps));
+					   sizeof(kib_fmr_poolset_t));
 	if (!net->ibn_fmr_ps) {
 		CERROR("Failed to allocate FMR pool array\n");
 		rc = -ENOMEM;
@@ -1992,7 +1992,7 @@ static int kiblnd_net_init_pools(kib_net_t *net, __u32 *cpts, int ncpts)
 
  create_tx_pool:
 	net->ibn_tx_ps = cfs_percpt_alloc(lnet_cpt_table(),
-					  sizeof(*net->ibn_tx_ps));
+					  sizeof(kib_tx_poolset_t));
 	if (!net->ibn_tx_ps) {
 		CERROR("Failed to allocate tx pool array\n");
 		rc = -ENOMEM;
-- 
1.7.1

[PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[James Simmons]

Latest testing fails when using ko2iblnd. It was tracked down
to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.

This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
---
 .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
index 89f9390..0d32e65 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
@@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net, __u32 *cpts, int ncpts)
 	 */
 
 	net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
-					   sizeof(*net->ibn_fmr_ps));
+					   sizeof(kib_fmr_poolset_t));
 	if (!net->ibn_fmr_ps) {
 		CERROR("Failed to allocate FMR pool array\n");
 		rc = -ENOMEM;
@@ -1992,7 +1992,7 @@ static int kiblnd_net_init_pools(kib_net_t *net, __u32 *cpts, int ncpts)
 
  create_tx_pool:
 	net->ibn_tx_ps = cfs_percpt_alloc(lnet_cpt_table(),
-					  sizeof(*net->ibn_tx_ps));
+					  sizeof(kib_tx_poolset_t));
 	if (!net->ibn_tx_ps) {
 		CERROR("Failed to allocate tx pool array\n");
 		rc = -ENOMEM;
-- 
1.7.1

[lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Greg Kroah-Hartman]

On Tue, Mar 22, 2016 at 06:21:04PM -0400, James Simmons wrote:
> Latest testing fails when using ko2iblnd. It was tracked down
> to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
> 
> This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
> ---
>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> index 89f9390..0d32e65 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> @@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net, __u32 *cpts, int ncpts)
>  	 */
>  
>  	net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
> -					   sizeof(*net->ibn_fmr_ps));
> +					   sizeof(kib_fmr_poolset_t));

Ok, why is this revert needed?  Please give me a big huge comment about
why this is not the same size of the variable being assigned to it,
otherwise someone else is going to come along and make the exact same
change again.

>  	if (!net->ibn_fmr_ps) {
>  		CERROR("Failed to allocate FMR pool array\n");
>  		rc = -ENOMEM;
> @@ -1992,7 +1992,7 @@ static int kiblnd_net_init_pools(kib_net_t *net, __u32 *cpts, int ncpts)
>  
>   create_tx_pool:
>  	net->ibn_tx_ps = cfs_percpt_alloc(lnet_cpt_table(),
> -					  sizeof(*net->ibn_tx_ps));
> +					  sizeof(kib_tx_poolset_t));

Same here, why is this code wrong?

thanks,

greg k-h

Re: [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Greg Kroah-Hartman]

On Tue, Mar 22, 2016 at 06:21:04PM -0400, James Simmons wrote:
> Latest testing fails when using ko2iblnd. It was tracked down
> to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
> 
> This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
> ---
>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> index 89f9390..0d32e65 100644
> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> @@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net, __u32 *cpts, int ncpts)
>  	 */
>  
>  	net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
> -					   sizeof(*net->ibn_fmr_ps));
> +					   sizeof(kib_fmr_poolset_t));

Ok, why is this revert needed?  Please give me a big huge comment about
why this is not the same size of the variable being assigned to it,
otherwise someone else is going to come along and make the exact same
change again.

>  	if (!net->ibn_fmr_ps) {
>  		CERROR("Failed to allocate FMR pool array\n");
>  		rc = -ENOMEM;
> @@ -1992,7 +1992,7 @@ static int kiblnd_net_init_pools(kib_net_t *net, __u32 *cpts, int ncpts)
>  
>   create_tx_pool:
>  	net->ibn_tx_ps = cfs_percpt_alloc(lnet_cpt_table(),
> -					  sizeof(*net->ibn_tx_ps));
> +					  sizeof(kib_tx_poolset_t));

Same here, why is this code wrong?

thanks,

greg k-h

[lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Dilger, Andreas]

On 2016/03/22, 19:49, "lustre-devel on behalf of Greg Kroah-Hartman"
<lustre-devel-bounces at lists.lustre.org on behalf of
gregkh@linuxfoundation.org> wrote:

>On Tue, Mar 22, 2016 at 06:21:04PM -0400, James Simmons wrote:
>> Latest testing fails when using ko2iblnd. It was tracked down
>> to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
>> 
>> This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
>> ---
>>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>>b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> index 89f9390..0d32e65 100644
>> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> @@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
>>__u32 *cpts, int ncpts)
>>  	 */
>>  
>>  	net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
>> -					   sizeof(*net->ibn_fmr_ps));
>> +					   sizeof(kib_fmr_poolset_t));
>
>Ok, why is this revert needed?  Please give me a big huge comment about
>why this is not the same size of the variable being assigned to it,
>otherwise someone else is going to come along and make the exact same
>change again.
>
>>  	if (!net->ibn_fmr_ps) {
>>  		CERROR("Failed to allocate FMR pool array\n");
>>  		rc = -ENOMEM;
>> @@ -1992,7 +1992,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
>>__u32 *cpts, int ncpts)
>>  
>>   create_tx_pool:
>>  	net->ibn_tx_ps = cfs_percpt_alloc(lnet_cpt_table(),
>> -					  sizeof(*net->ibn_tx_ps));
>> +					  sizeof(kib_tx_poolset_t));
>
>Same here, why is this code wrong?

Looks like the declarations are:

        kib_tx_poolset_t	**ibn_tx_ps;	/* tx pool-set */
	kib_fmr_poolset_t	**ibn_fmr_ps;	/* fmr pool-set */



so the right code should be:

        sizeof(**net->ibn_tx_ps);


and the same for sizeof(**net->ibn_fmr_ps)

Cheers, Andreas
-- 
Andreas Dilger

Lustre Principal Architect
Intel High Performance Data Division

Re: [lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Dilger, Andreas]

On 2016/03/22, 19:49, "lustre-devel on behalf of Greg Kroah-Hartman"
<lustre-devel-bounces@lists.lustre.org on behalf of
gregkh@linuxfoundation.org> wrote:

>On Tue, Mar 22, 2016 at 06:21:04PM -0400, James Simmons wrote:
>> Latest testing fails when using ko2iblnd. It was tracked down
>> to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
>> 
>> This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
>> ---
>>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>>b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> index 89f9390..0d32e65 100644
>> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> @@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
>>__u32 *cpts, int ncpts)
>>  	 */
>>  
>>  	net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
>> -					   sizeof(*net->ibn_fmr_ps));
>> +					   sizeof(kib_fmr_poolset_t));
>
>Ok, why is this revert needed?  Please give me a big huge comment about
>why this is not the same size of the variable being assigned to it,
>otherwise someone else is going to come along and make the exact same
>change again.
>
>>  	if (!net->ibn_fmr_ps) {
>>  		CERROR("Failed to allocate FMR pool array\n");
>>  		rc = -ENOMEM;
>> @@ -1992,7 +1992,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
>>__u32 *cpts, int ncpts)
>>  
>>   create_tx_pool:
>>  	net->ibn_tx_ps = cfs_percpt_alloc(lnet_cpt_table(),
>> -					  sizeof(*net->ibn_tx_ps));
>> +					  sizeof(kib_tx_poolset_t));
>
>Same here, why is this code wrong?

Looks like the declarations are:

        kib_tx_poolset_t	**ibn_tx_ps;	/* tx pool-set */
	kib_fmr_poolset_t	**ibn_fmr_ps;	/* fmr pool-set */



so the right code should be:

        sizeof(**net->ibn_tx_ps);


and the same for sizeof(**net->ibn_fmr_ps)

Cheers, Andreas
-- 
Andreas Dilger

Lustre Principal Architect
Intel High Performance Data Division

[lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Oucharek, Doug S]

Yes, those are ?per-CPT allocations?.  So the allocator ends up allocating an array of pointers to the given data type.

Doug

On Mar 22, 2016, at 10:39 PM, Dilger, Andreas <andreas.dilger at intel.com<mailto:andreas.dilger@intel.com>> wrote:

On 2016/03/22, 19:49, "lustre-devel on behalf of Greg Kroah-Hartman"
<lustre-devel-bounces at lists.lustre.org<mailto:lustre-devel-bounces@lists.lustre.org> on behalf of
gregkh at linuxfoundation.org<mailto:gregkh@linuxfoundation.org>> wrote:

On Tue, Mar 22, 2016 at 06:21:04PM -0400, James Simmons wrote:
Latest testing fails when using ko2iblnd. It was tracked down
to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.

This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
---
.../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
index 89f9390..0d32e65 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
@@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
__u32 *cpts, int ncpts)
 */

net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
-    sizeof(*net->ibn_fmr_ps));
+    sizeof(kib_fmr_poolset_t));

Ok, why is this revert needed?  Please give me a big huge comment about
why this is not the same size of the variable being assigned to it,
otherwise someone else is going to come along and make the exact same
change again.

if (!net->ibn_fmr_ps) {
CERROR("Failed to allocate FMR pool array\n");
rc = -ENOMEM;
@@ -1992,7 +1992,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
__u32 *cpts, int ncpts)

 create_tx_pool:
net->ibn_tx_ps = cfs_percpt_alloc(lnet_cpt_table(),
-   sizeof(*net->ibn_tx_ps));
+   sizeof(kib_tx_poolset_t));

Same here, why is this code wrong?

Looks like the declarations are:

       kib_tx_poolset_t **ibn_tx_ps; /* tx pool-set */
kib_fmr_poolset_t **ibn_fmr_ps; /* fmr pool-set */



so the right code should be:

       sizeof(**net->ibn_tx_ps);


and the same for sizeof(**net->ibn_fmr_ps)

Cheers, Andreas
--
Andreas Dilger

Lustre Principal Architect
Intel High Performance Data Division


_______________________________________________
lustre-devel mailing list
lustre-devel at lists.lustre.org<mailto:lustre-devel@lists.lustre.org>
http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

Re: [lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Oucharek, Doug S]

Yes, those are “per-CPT allocations”.  So the allocator ends up allocating an array of pointers to the given data type.

Doug

On Mar 22, 2016, at 10:39 PM, Dilger, Andreas <andreas.dilger@intel.com<mailto:andreas.dilger@intel.com>> wrote:

On 2016/03/22, 19:49, "lustre-devel on behalf of Greg Kroah-Hartman"
<lustre-devel-bounces@lists.lustre.org<mailto:lustre-devel-bounces@lists.lustre.org> on behalf of
gregkh@linuxfoundation.org<mailto:gregkh@linuxfoundation.org>> wrote:

On Tue, Mar 22, 2016 at 06:21:04PM -0400, James Simmons wrote:
Latest testing fails when using ko2iblnd. It was tracked down
to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.

This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
---
.../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
index 89f9390..0d32e65 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
@@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
__u32 *cpts, int ncpts)
 */

net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
-    sizeof(*net->ibn_fmr_ps));
+    sizeof(kib_fmr_poolset_t));

Ok, why is this revert needed?  Please give me a big huge comment about
why this is not the same size of the variable being assigned to it,
otherwise someone else is going to come along and make the exact same
change again.

if (!net->ibn_fmr_ps) {
CERROR("Failed to allocate FMR pool array\n");
rc = -ENOMEM;
@@ -1992,7 +1992,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
__u32 *cpts, int ncpts)

 create_tx_pool:
net->ibn_tx_ps = cfs_percpt_alloc(lnet_cpt_table(),
-   sizeof(*net->ibn_tx_ps));
+   sizeof(kib_tx_poolset_t));

Same here, why is this code wrong?

Looks like the declarations are:

       kib_tx_poolset_t **ibn_tx_ps; /* tx pool-set */
kib_fmr_poolset_t **ibn_fmr_ps; /* fmr pool-set */



so the right code should be:

       sizeof(**net->ibn_tx_ps);


and the same for sizeof(**net->ibn_fmr_ps)

Cheers, Andreas
--
Andreas Dilger

Lustre Principal Architect
Intel High Performance Data Division


_______________________________________________
lustre-devel mailing list
lustre-devel@lists.lustre.org<mailto:lustre-devel@lists.lustre.org>
http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

[lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Greg Kroah-Hartman]

On Wed, Mar 23, 2016 at 05:39:36AM +0000, Dilger, Andreas wrote:
> On 2016/03/22, 19:49, "lustre-devel on behalf of Greg Kroah-Hartman"
> <lustre-devel-bounces@lists.lustre.org on behalf of
> gregkh at linuxfoundation.org> wrote:
> 
> >On Tue, Mar 22, 2016 at 06:21:04PM -0400, James Simmons wrote:
> >> Latest testing fails when using ko2iblnd. It was tracked down
> >> to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
> >> 
> >> This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
> >> ---
> >>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    4 ++--
> >>  1 files changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> >>b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> >> index 89f9390..0d32e65 100644
> >> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> >> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> >> @@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
> >>__u32 *cpts, int ncpts)
> >>  	 */
> >>  
> >>  	net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
> >> -					   sizeof(*net->ibn_fmr_ps));
> >> +					   sizeof(kib_fmr_poolset_t));
> >
> >Ok, why is this revert needed?  Please give me a big huge comment about
> >why this is not the same size of the variable being assigned to it,
> >otherwise someone else is going to come along and make the exact same
> >change again.
> >
> >>  	if (!net->ibn_fmr_ps) {
> >>  		CERROR("Failed to allocate FMR pool array\n");
> >>  		rc = -ENOMEM;
> >> @@ -1992,7 +1992,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
> >>__u32 *cpts, int ncpts)
> >>  
> >>   create_tx_pool:
> >>  	net->ibn_tx_ps = cfs_percpt_alloc(lnet_cpt_table(),
> >> -					  sizeof(*net->ibn_tx_ps));
> >> +					  sizeof(kib_tx_poolset_t));
> >
> >Same here, why is this code wrong?
> 
> Looks like the declarations are:
> 
>         kib_tx_poolset_t	**ibn_tx_ps;	/* tx pool-set */
> 	kib_fmr_poolset_t	**ibn_fmr_ps;	/* fmr pool-set */
> 
> 
> 
> so the right code should be:
> 
>         sizeof(**net->ibn_tx_ps);
> 
> 
> and the same for sizeof(**net->ibn_fmr_ps)

That's a mess, isn't there some other way to fix this up to be more
"obvious"?

thanks,

greg k-h

Re: [lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Greg Kroah-Hartman]

On Wed, Mar 23, 2016 at 05:39:36AM +0000, Dilger, Andreas wrote:
> On 2016/03/22, 19:49, "lustre-devel on behalf of Greg Kroah-Hartman"
> <lustre-devel-bounces@lists.lustre.org on behalf of
> gregkh@linuxfoundation.org> wrote:
> 
> >On Tue, Mar 22, 2016 at 06:21:04PM -0400, James Simmons wrote:
> >> Latest testing fails when using ko2iblnd. It was tracked down
> >> to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
> >> 
> >> This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
> >> ---
> >>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    4 ++--
> >>  1 files changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> >>b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> >> index 89f9390..0d32e65 100644
> >> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> >> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> >> @@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
> >>__u32 *cpts, int ncpts)
> >>  	 */
> >>  
> >>  	net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
> >> -					   sizeof(*net->ibn_fmr_ps));
> >> +					   sizeof(kib_fmr_poolset_t));
> >
> >Ok, why is this revert needed?  Please give me a big huge comment about
> >why this is not the same size of the variable being assigned to it,
> >otherwise someone else is going to come along and make the exact same
> >change again.
> >
> >>  	if (!net->ibn_fmr_ps) {
> >>  		CERROR("Failed to allocate FMR pool array\n");
> >>  		rc = -ENOMEM;
> >> @@ -1992,7 +1992,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
> >>__u32 *cpts, int ncpts)
> >>  
> >>   create_tx_pool:
> >>  	net->ibn_tx_ps = cfs_percpt_alloc(lnet_cpt_table(),
> >> -					  sizeof(*net->ibn_tx_ps));
> >> +					  sizeof(kib_tx_poolset_t));
> >
> >Same here, why is this code wrong?
> 
> Looks like the declarations are:
> 
>         kib_tx_poolset_t	**ibn_tx_ps;	/* tx pool-set */
> 	kib_fmr_poolset_t	**ibn_fmr_ps;	/* fmr pool-set */
> 
> 
> 
> so the right code should be:
> 
>         sizeof(**net->ibn_tx_ps);
> 
> 
> and the same for sizeof(**net->ibn_fmr_ps)

That's a mess, isn't there some other way to fix this up to be more
"obvious"?

thanks,

greg k-h

[lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Simmons, James A.]

>On Wed, Mar 23, 2016 at 05:39:36AM +0000, Dilger, Andreas wrote:
>> On 2016/03/22, 19:49, "lustre-devel on behalf of Greg Kroah-Hartman"
>> <lustre-devel-bounces@lists.lustre.org on behalf of
>> gregkh at linuxfoundation.org> wrote:
>> 
>> >On Tue, Mar 22, 2016 at 06:21:04PM -0400, James Simmons wrote:
>> >> Latest testing fails when using ko2iblnd. It was tracked down
>> >> to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
>> >> 
>> >> This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
>> >> ---
>> >>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    4 ++--
>> >>  1 files changed, 2 insertions(+), 2 deletions(-)
>> >> 
>> >> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> >>b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> >> index 89f9390..0d32e65 100644
>> >> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> >> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> >> @@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
>> >>__u32 *cpts, int ncpts)
>> >>  	 */
>> >>  
>> >>  	net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
>> >> -					   sizeof(*net->ibn_fmr_ps));
>> >> +					   sizeof(kib_fmr_poolset_t));
>> >
>> >Ok, why is this revert needed?  Please give me a big huge comment about
>> >why this is not the same size of the variable being assigned to it,
>> >otherwise someone else is going to come along and make the exact same
>> >change again.
>> >
>> >>  	if (!net->ibn_fmr_ps) {
>> >>  		CERROR("Failed to allocate FMR pool array\n");
>> >>  		rc = -ENOMEM;
>> >> @@ -1992,7 +1992,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
>> >>__u32 *cpts, int ncpts)
>> >>  
>> >>   create_tx_pool:
>> >>  	net->ibn_tx_ps = cfs_percpt_alloc(lnet_cpt_table(),
>> >> -					  sizeof(*net->ibn_tx_ps));
>> >> +					  sizeof(kib_tx_poolset_t));
>> >
>> >Same here, why is this code wrong?
>> 
>> Looks like the declarations are:
>> 
>>         kib_tx_poolset_t	**ibn_tx_ps;	/* tx pool-set */
>> 	kib_fmr_poolset_t	**ibn_fmr_ps;	/* fmr pool-set */
>> 
>> 
>> 
>> so the right code should be:
>> 
>>         sizeof(**net->ibn_tx_ps);
>> 
>> 
>> and the same for sizeof(**net->ibn_fmr_ps)
>
>That's a mess, isn't there some other way to fix this up to be more
>"obvious"?

This must have been encountered in the past. How was it handle in those
other cases?

RE: [lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Simmons, James A.]

>On Wed, Mar 23, 2016 at 05:39:36AM +0000, Dilger, Andreas wrote:
>> On 2016/03/22, 19:49, "lustre-devel on behalf of Greg Kroah-Hartman"
>> <lustre-devel-bounces@lists.lustre.org on behalf of
>> gregkh@linuxfoundation.org> wrote:
>> 
>> >On Tue, Mar 22, 2016 at 06:21:04PM -0400, James Simmons wrote:
>> >> Latest testing fails when using ko2iblnd. It was tracked down
>> >> to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
>> >> 
>> >> This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
>> >> ---
>> >>  .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c    |    4 ++--
>> >>  1 files changed, 2 insertions(+), 2 deletions(-)
>> >> 
>> >> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> >>b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> >> index 89f9390..0d32e65 100644
>> >> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> >> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
>> >> @@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
>> >>__u32 *cpts, int ncpts)
>> >>  	 */
>> >>  
>> >>  	net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
>> >> -					   sizeof(*net->ibn_fmr_ps));
>> >> +					   sizeof(kib_fmr_poolset_t));
>> >
>> >Ok, why is this revert needed?  Please give me a big huge comment about
>> >why this is not the same size of the variable being assigned to it,
>> >otherwise someone else is going to come along and make the exact same
>> >change again.
>> >
>> >>  	if (!net->ibn_fmr_ps) {
>> >>  		CERROR("Failed to allocate FMR pool array\n");
>> >>  		rc = -ENOMEM;
>> >> @@ -1992,7 +1992,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
>> >>__u32 *cpts, int ncpts)
>> >>  
>> >>   create_tx_pool:
>> >>  	net->ibn_tx_ps = cfs_percpt_alloc(lnet_cpt_table(),
>> >> -					  sizeof(*net->ibn_tx_ps));
>> >> +					  sizeof(kib_tx_poolset_t));
>> >
>> >Same here, why is this code wrong?
>> 
>> Looks like the declarations are:
>> 
>>         kib_tx_poolset_t	**ibn_tx_ps;	/* tx pool-set */
>> 	kib_fmr_poolset_t	**ibn_fmr_ps;	/* fmr pool-set */
>> 
>> 
>> 
>> so the right code should be:
>> 
>>         sizeof(**net->ibn_tx_ps);
>> 
>> 
>> and the same for sizeof(**net->ibn_fmr_ps)
>
>That's a mess, isn't there some other way to fix this up to be more
>"obvious"?

This must have been encountered in the past. How was it handle in those
other cases?

[lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Joe Perches]

On Wed, 2016-03-23 at 18:31 +0000, Simmons, James A. wrote:
> > On Wed, Mar 23, 2016 at 05:39:36AM +0000, Dilger, Andreas wrote:
> > > On 2016/03/22, 19:49, "lustre-devel on behalf of Greg Kroah-Hartman"
> > > <lustre-devel-bounces@lists.lustre.org on behalf of
> > > gregkh at linuxfoundation.org> wrote:
> > > > On Tue, Mar 22, 2016 at 06:21:04PM -0400, James Simmons wrote:
> > > > > 
> > > > > Latest testing fails when using ko2iblnd. It was tracked down
> > > > > to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
> > > > > 
> > > > > This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
[]
> > > > > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
[]
> > > > > @@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
> > > > > __u32 *cpts, int ncpts)
> > > > > ?	?*/
> > > > > ?
> > > > > ?	net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
> > > > > -					???sizeof(*net->ibn_fmr_ps));
> > > > > +					???sizeof(kib_fmr_poolset_t));
> > > > Ok, why is this revert needed???Please give me a big huge comment about
> > > > why this is not the same size of the variable being assigned to it,
> > > > otherwise someone else is going to come along and make the exact same
> > > > change again.
[]
> > > Looks like the declarations are:
> > > 
> > > ????????kib_tx_poolset_t	**ibn_tx_ps;	/* tx pool-set */
> > > 	kib_fmr_poolset_t	**ibn_fmr_ps;	/* fmr pool-set */
[]
> > > so the right code should be:
> > > 
> > > ????????sizeof(**net->ibn_tx_ps);
> > > and the same for sizeof(**net->ibn_fmr_ps)
> > That's a mess, isn't there some other way to fix this up to be more
> > "obvious"?
> This must have been encountered in the past. How was it handle in those
> other cases?

I fail to see why it's a mess.  It's just **
and someone making a mistake.

Removing the "typedef struct" uses from lustre
would probably make a lot of this clearer though.

$ git grep -En "typedef\s+struct" drivers/staging/lustre/
drivers/staging/lustre/include/linux/libcfs/libcfs_workitem.h:76:typedef struct cfs_workitem {
drivers/staging/lustre/include/linux/lnet/lib-types.h:56:typedef struct lnet_msg {
drivers/staging/lustre/include/linux/lnet/lib-types.h:110:typedef struct lnet_libhandle {
drivers/staging/lustre/include/linux/lnet/lib-types.h:118:typedef struct lnet_eq {
drivers/staging/lustre/include/linux/lnet/lib-types.h:129:typedef struct lnet_me {
drivers/staging/lustre/include/linux/lnet/lib-types.h:141:typedef struct lnet_libmd {
drivers/staging/lustre/include/linux/lnet/lib-types.h:166:typedef struct {
drivers/staging/lustre/include/linux/lnet/lib-types.h:181:typedef struct lnet_lnd {
drivers/staging/lustre/include/linux/lnet/lib-types.h:256:typedef struct lnet_ni {
drivers/staging/lustre/include/linux/lnet/lib-types.h:297:typedef struct {
drivers/staging/lustre/include/linux/lnet/lib-types.h:305:typedef struct lnet_peer {
drivers/staging/lustre/include/linux/lnet/lib-types.h:369:typedef struct {
drivers/staging/lustre/include/linux/lnet/lib-types.h:384:typedef struct {
drivers/staging/lustre/include/linux/lnet/lib-types.h:396:typedef struct {
drivers/staging/lustre/include/linux/lnet/lib-types.h:410:typedef struct {
drivers/staging/lustre/include/linux/lnet/lib-types.h:497:typedef struct lnet_portal {
drivers/staging/lustre/include/linux/lnet/lib-types.h:545:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:71:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:78:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:88:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:93:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:101:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:108:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:115:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:127:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:140:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:242:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:254:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:265:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:275:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:296:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:302:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:313:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:325:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:337:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:344:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:357:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:363:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:369:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:379:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:389:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:402:typedef struct {

Re: [lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Joe Perches]

On Wed, 2016-03-23 at 18:31 +0000, Simmons, James A. wrote:
> > On Wed, Mar 23, 2016 at 05:39:36AM +0000, Dilger, Andreas wrote:
> > > On 2016/03/22, 19:49, "lustre-devel on behalf of Greg Kroah-Hartman"
> > > <lustre-devel-bounces@lists.lustre.org on behalf of
> > > gregkh@linuxfoundation.org> wrote:
> > > > On Tue, Mar 22, 2016 at 06:21:04PM -0400, James Simmons wrote:
> > > > > 
> > > > > Latest testing fails when using ko2iblnd. It was tracked down
> > > > > to commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
> > > > > 
> > > > > This reverts commit 4671a026616df26000f7d8ad2f2ea4b6de79263c.
[]
> > > > > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
[]
> > > > > @@ -1968,7 +1968,7 @@ static int kiblnd_net_init_pools(kib_net_t *net,
> > > > > __u32 *cpts, int ncpts)
> > > > >  	 */
> > > > >  
> > > > >  	net->ibn_fmr_ps = cfs_percpt_alloc(lnet_cpt_table(),
> > > > > -					   sizeof(*net->ibn_fmr_ps));
> > > > > +					   sizeof(kib_fmr_poolset_t));
> > > > Ok, why is this revert needed?  Please give me a big huge comment about
> > > > why this is not the same size of the variable being assigned to it,
> > > > otherwise someone else is going to come along and make the exact same
> > > > change again.
[]
> > > Looks like the declarations are:
> > > 
> > >         kib_tx_poolset_t	**ibn_tx_ps;	/* tx pool-set */
> > > 	kib_fmr_poolset_t	**ibn_fmr_ps;	/* fmr pool-set */
[]
> > > so the right code should be:
> > > 
> > >         sizeof(**net->ibn_tx_ps);
> > > and the same for sizeof(**net->ibn_fmr_ps)
> > That's a mess, isn't there some other way to fix this up to be more
> > "obvious"?
> This must have been encountered in the past. How was it handle in those
> other cases?

I fail to see why it's a mess.  It's just **
and someone making a mistake.

Removing the "typedef struct" uses from lustre
would probably make a lot of this clearer though.

$ git grep -En "typedef\s+struct" drivers/staging/lustre/
drivers/staging/lustre/include/linux/libcfs/libcfs_workitem.h:76:typedef struct cfs_workitem {
drivers/staging/lustre/include/linux/lnet/lib-types.h:56:typedef struct lnet_msg {
drivers/staging/lustre/include/linux/lnet/lib-types.h:110:typedef struct lnet_libhandle {
drivers/staging/lustre/include/linux/lnet/lib-types.h:118:typedef struct lnet_eq {
drivers/staging/lustre/include/linux/lnet/lib-types.h:129:typedef struct lnet_me {
drivers/staging/lustre/include/linux/lnet/lib-types.h:141:typedef struct lnet_libmd {
drivers/staging/lustre/include/linux/lnet/lib-types.h:166:typedef struct {
drivers/staging/lustre/include/linux/lnet/lib-types.h:181:typedef struct lnet_lnd {
drivers/staging/lustre/include/linux/lnet/lib-types.h:256:typedef struct lnet_ni {
drivers/staging/lustre/include/linux/lnet/lib-types.h:297:typedef struct {
drivers/staging/lustre/include/linux/lnet/lib-types.h:305:typedef struct lnet_peer {
drivers/staging/lustre/include/linux/lnet/lib-types.h:369:typedef struct {
drivers/staging/lustre/include/linux/lnet/lib-types.h:384:typedef struct {
drivers/staging/lustre/include/linux/lnet/lib-types.h:396:typedef struct {
drivers/staging/lustre/include/linux/lnet/lib-types.h:410:typedef struct {
drivers/staging/lustre/include/linux/lnet/lib-types.h:497:typedef struct lnet_portal {
drivers/staging/lustre/include/linux/lnet/lib-types.h:545:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:71:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:78:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:88:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:93:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:101:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:108:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:115:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:127:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:140:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:242:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:254:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:265:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:275:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:296:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:302:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:313:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:325:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:337:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:344:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:357:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:363:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:369:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:379:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:389:typedef struct {
drivers/staging/lustre/include/linux/lnet/lnetst.h:402:typedef struct {

[lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Simmons, James A.]

>> > > so the right code should be:
>> > > 
>> > > ????????sizeof(**net->ibn_tx_ps);
>> > > and the same for sizeof(**net->ibn_fmr_ps)
>> > That's a mess, isn't there some other way to fix this up to be more
>> > "obvious"?
>> This must have been encountered in the past. How was it handle in those
>> other cases?
>
>I fail to see why it's a mess.  It's just **
>and someone making a mistake.

I have no trouble with **.  If we revert it someone else will come along and
do the same mistake so I think we are stuck with the change to **.

>Removing the "typedef struct" uses from lustre
>would probably make a lot of this clearer though.

That along with a few hundred more patches heading Greg's way :-)

RE: [lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

[Simmons, James A.]

>> > > so the right code should be:
>> > > 
>> > >         sizeof(**net->ibn_tx_ps);
>> > > and the same for sizeof(**net->ibn_fmr_ps)
>> > That's a mess, isn't there some other way to fix this up to be more
>> > "obvious"?
>> This must have been encountered in the past. How was it handle in those
>> other cases?
>
>I fail to see why it's a mess.  It's just **
>and someone making a mistake.

I have no trouble with **.  If we revert it someone else will come along and
do the same mistake so I think we are stuck with the change to **.

>Removing the "typedef struct" uses from lustre
>would probably make a lot of this clearer though.

That along with a few hundred more patches heading Greg's way :-)

[lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

['Greg Kroah-Hartman']

On Wed, Mar 23, 2016 at 08:29:49PM +0000, Simmons, James A. wrote:
> 
> >> > > so the right code should be:
> >> > > 
> >> > > ????????sizeof(**net->ibn_tx_ps);
> >> > > and the same for sizeof(**net->ibn_fmr_ps)
> >> > That's a mess, isn't there some other way to fix this up to be more
> >> > "obvious"?
> >> This must have been encountered in the past. How was it handle in those
> >> other cases?
> >
> >I fail to see why it's a mess.  It's just **
> >and someone making a mistake.
> 
> I have no trouble with **.  If we revert it someone else will come along and
> do the same mistake so I think we are stuck with the change to **.

That's fine, but if you do that, just comment it well so that no one
tries to change it again.  That's why I can't accept the original patch
in this email thread.

thanks,

greg k-h

Re: [lustre-devel] [PATCH] Revert "Staging: lustre: o2iblnd: Use sizeof type *pointer instead of sizeof type."

['Greg Kroah-Hartman']

On Wed, Mar 23, 2016 at 08:29:49PM +0000, Simmons, James A. wrote:
> 
> >> > > so the right code should be:
> >> > > 
> >> > >         sizeof(**net->ibn_tx_ps);
> >> > > and the same for sizeof(**net->ibn_fmr_ps)
> >> > That's a mess, isn't there some other way to fix this up to be more
> >> > "obvious"?
> >> This must have been encountered in the past. How was it handle in those
> >> other cases?
> >
> >I fail to see why it's a mess.  It's just **
> >and someone making a mistake.
> 
> I have no trouble with **.  If we revert it someone else will come along and
> do the same mistake so I think we are stuck with the change to **.

That's fine, but if you do that, just comment it well so that no one
tries to change it again.  That's why I can't accept the original patch
in this email thread.

thanks,

greg k-h