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


Groups > linux.kernel > #1413784 > unrolled thread

Re: [PATCH 2/3] ntb_transport: Check the number of spads the hardware supports

Started byJon Mason <jdmason@kudzu.us>
First post2016-06-04 17:50 +0200
Last post2016-06-09 17:00 +0200
Articles 4 — 3 participants

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

  Re: [PATCH 2/3] ntb_transport: Check the number of spads the  hardware supports Jon Mason <jdmason@kudzu.us> - 2016-06-04 17:50 +0200
    Re: [PATCH 2/3] ntb_transport: Check the number of spads the hardware  supports Logan Gunthorpe <logang@deltatee.com> - 2016-06-07 19:20 +0200
      Re: [PATCH v2 2/3] ntb_transport: Check the number of spads the  hardware supports "Jiang, Dave" <dave.jiang@intel.com> - 2016-06-07 19:30 +0200
        Re: [PATCH v2 2/3] ntb_transport: Check the number of spads the  hardware supports Jon Mason <jdmason@kudzu.us> - 2016-06-09 17:00 +0200

#1413784 — Re: [PATCH 2/3] ntb_transport: Check the number of spads the hardware supports

FromJon Mason <jdmason@kudzu.us>
Date2016-06-04 17:50 +0200
SubjectRe: [PATCH 2/3] ntb_transport: Check the number of spads the hardware supports
Message-ID<rGlZL-2V7-9@gated-at.bofh.it>
On Fri, Jun 03, 2016 at 02:50:32PM -0600, Logan Gunthorpe wrote:
> I'm working on hardware that currently has a limited number of
> scratchpad registers and ntb_ndev fails with no clue as to why. I
> feel it is better to fail early and provide a reasonable error message
> then to fail later on..
> 
> The same is done to ntb_perf, but it doesn't currently require enough
> spads to actually fail.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---
>  drivers/ntb/ntb_transport.c | 9 +++++++--
>  drivers/ntb/test/ntb_perf.c | 8 ++++++--
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 2ef9d913..34e7a7a 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1037,6 +1037,13 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
>  	int node;
>  	int rc, i;
>  
> +	mw_count = ntb_mw_count(ndev);
> +	if (ntb_spad_count(ndev) < (NUM_MWS + 1 + mw_count*2)) {

Nit, please add spaces around '*' (per checkpatch)

> +		dev_err(&ndev->dev, "Not enough scratch pad registers for %s",
> +			NTB_TRANSPORT_NAME);
> +		return -EIO;
> +	}
> +
>  	if (ntb_db_is_unsafe(ndev))
>  		dev_dbg(&ndev->dev,
>  			"doorbell is unsafe, proceed anyway...\n");
> @@ -1052,8 +1059,6 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
>  
>  	nt->ndev = ndev;
>  
> -	mw_count = ntb_mw_count(ndev);
> -
>  	nt->mw_count = mw_count;
>  
>  	nt->mw_vec = kzalloc_node(mw_count * sizeof(*nt->mw_vec),
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> index 30635c8..1815592 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -143,8 +143,6 @@ enum {
>  	VERSION = 0,
>  	MW_SZ_HIGH,
>  	MW_SZ_LOW,
> -	SPAD_MSG,
> -	SPAD_ACK,

Please explicitly point out that this is being modified in the commit
message.  I don't see them being used, so probably not a big deal
(unless Dave Jiang has something queued that will use it).

>  	MAX_SPAD
>  };
>  
> @@ -698,6 +696,12 @@ static int perf_probe(struct ntb_client *client, struct ntb_dev *ntb)
>  
>  	node = dev_to_node(&pdev->dev);
>  
> +	if (ntb_spad_count(ntb) < MAX_SPAD) {
> +		dev_err(&ntb->dev, "Not enough scratch pad registers for %s",
> +			DRIVER_NAME);
> +		return -EIO;
> +	}

Move this check above the dev_to_node assignment above.

Thanks,
Jon

> +
>  	perf = kzalloc_node(sizeof(*perf), GFP_KERNEL, node);
>  	if (!perf) {
>  		rc = -ENOMEM;
> -- 
> 2.1.4
> 

[toc] | [next] | [standalone]


#1416431 — Re: [PATCH 2/3] ntb_transport: Check the number of spads the hardware supports

FromLogan Gunthorpe <logang@deltatee.com>
Date2016-06-07 19:20 +0200
SubjectRe: [PATCH 2/3] ntb_transport: Check the number of spads the hardware supports
Message-ID<rHsPv-5vs-3@gated-at.bofh.it>
In reply to#1413784
Hi Jon,

Thanks for the feedback. I'll send an updated patch in a moment.

On 04/06/16 09:40 AM, Jon Mason wrote:
> Nit, please add spaces around '*' (per checkpatch)

I'll change this, but I did run it through checkpatch and it did not
warn about this.

> Please explicitly point out that this is being modified in the commit
> message.  I don't see them being used, so probably not a big deal
> (unless Dave Jiang has something queued that will use it).

Done. I feel like he can always add them back in when he adds the
functionality. This way, when he does, MAX_SPAD will be updated and the
check will still be correct.

> Move this check above the dev_to_node assignment above.

Done.


Logan

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


#1416435 — Re: [PATCH v2 2/3] ntb_transport: Check the number of spads the hardware supports

From"Jiang, Dave" <dave.jiang@intel.com>
Date2016-06-07 19:30 +0200
SubjectRe: [PATCH v2 2/3] ntb_transport: Check the number of spads the hardware supports
Message-ID<rHsZb-5yF-7@gated-at.bofh.it>
In reply to#1416431
On Tue, 2016-06-07 at 11:20 -0600, Logan Gunthorpe wrote:
> I'm working on hardware that currently has a limited number of
> scratchpad registers and ntb_ndev fails with no clue as to why. I
> feel it is better to fail early and provide a reasonable error
> message
> then to fail later on.
> 
> The same is done to ntb_perf, but it doesn't currently require enough
> spads to actually fail. I've also removed the unused SPAD_MSG and
> SPAD_ACK enums so that MAX_SPAD accurately reflects the number of
> spads used.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>

Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/ntb/ntb_transport.c | 9 +++++++--
>  drivers/ntb/test/ntb_perf.c | 8 ++++++--
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c
> b/drivers/ntb/ntb_transport.c
> index 2ef9d913..6d9940a 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1037,6 +1037,13 @@ static int ntb_transport_probe(struct
> ntb_client *self, struct ntb_dev *ndev)
>  	int node;
>  	int rc, i;
>  
> +	mw_count = ntb_mw_count(ndev);
> +	if (ntb_spad_count(ndev) < (NUM_MWS + 1 + mw_count * 2)) {
> +		dev_err(&ndev->dev, "Not enough scratch pad
> registers for %s",
> +			NTB_TRANSPORT_NAME);
> +		return -EIO;
> +	}
> +
>  	if (ntb_db_is_unsafe(ndev))
>  		dev_dbg(&ndev->dev,
>  			"doorbell is unsafe, proceed anyway...\n");
> @@ -1052,8 +1059,6 @@ static int ntb_transport_probe(struct
> ntb_client *self, struct ntb_dev *ndev)
>  
>  	nt->ndev = ndev;
>  
> -	mw_count = ntb_mw_count(ndev);
> -
>  	nt->mw_count = mw_count;
>  
>  	nt->mw_vec = kzalloc_node(mw_count * sizeof(*nt->mw_vec),
> diff --git a/drivers/ntb/test/ntb_perf.c
> b/drivers/ntb/test/ntb_perf.c
> index 30635c8..4368519 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -143,8 +143,6 @@ enum {
>  	VERSION = 0,
>  	MW_SZ_HIGH,
>  	MW_SZ_LOW,
> -	SPAD_MSG,
> -	SPAD_ACK,
>  	MAX_SPAD
>  };
>  
> @@ -696,6 +694,12 @@ static int perf_probe(struct ntb_client *client,
> struct ntb_dev *ntb)
>  	int node;
>  	int rc = 0;
>  
> +	if (ntb_spad_count(ntb) < MAX_SPAD) {
> +		dev_err(&ntb->dev, "Not enough scratch pad registers
> for %s",
> +			DRIVER_NAME);
> +		return -EIO;
> +	}
> +
>  	node = dev_to_node(&pdev->dev);
>  
>  	perf = kzalloc_node(sizeof(*perf), GFP_KERNEL, node);
> -- 
> 2.1.4
> 

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


#1418406 — Re: [PATCH v2 2/3] ntb_transport: Check the number of spads the hardware supports

FromJon Mason <jdmason@kudzu.us>
Date2016-06-09 17:00 +0200
SubjectRe: [PATCH v2 2/3] ntb_transport: Check the number of spads the hardware supports
Message-ID<rI9B8-7JN-13@gated-at.bofh.it>
In reply to#1416435
On Tue, Jun 7, 2016 at 1:24 PM, Jiang, Dave <dave.jiang@intel.com> wrote:
> On Tue, 2016-06-07 at 11:20 -0600, Logan Gunthorpe wrote:
>> I'm working on hardware that currently has a limited number of
>> scratchpad registers and ntb_ndev fails with no clue as to why. I
>> feel it is better to fail early and provide a reasonable error
>> message
>> then to fail later on.
>>
>> The same is done to ntb_perf, but it doesn't currently require enough
>> spads to actually fail. I've also removed the unused SPAD_MSG and
>> SPAD_ACK enums so that MAX_SPAD accurately reflects the number of
>> spads used.
>>
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>
> Acked-by: Dave Jiang <dave.jiang@intel.com>

Added to the ntb-next branch.

thanks,
Jon

>> ---
>>  drivers/ntb/ntb_transport.c | 9 +++++++--
>>  drivers/ntb/test/ntb_perf.c | 8 ++++++--
>>  2 files changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/ntb/ntb_transport.c
>> b/drivers/ntb/ntb_transport.c
>> index 2ef9d913..6d9940a 100644
>> --- a/drivers/ntb/ntb_transport.c
>> +++ b/drivers/ntb/ntb_transport.c
>> @@ -1037,6 +1037,13 @@ static int ntb_transport_probe(struct
>> ntb_client *self, struct ntb_dev *ndev)
>>       int node;
>>       int rc, i;
>>
>> +     mw_count = ntb_mw_count(ndev);
>> +     if (ntb_spad_count(ndev) < (NUM_MWS + 1 + mw_count * 2)) {
>> +             dev_err(&ndev->dev, "Not enough scratch pad
>> registers for %s",
>> +                     NTB_TRANSPORT_NAME);
>> +             return -EIO;
>> +     }
>> +
>>       if (ntb_db_is_unsafe(ndev))
>>               dev_dbg(&ndev->dev,
>>                       "doorbell is unsafe, proceed anyway...\n");
>> @@ -1052,8 +1059,6 @@ static int ntb_transport_probe(struct
>> ntb_client *self, struct ntb_dev *ndev)
>>
>>       nt->ndev = ndev;
>>
>> -     mw_count = ntb_mw_count(ndev);
>> -
>>       nt->mw_count = mw_count;
>>
>>       nt->mw_vec = kzalloc_node(mw_count * sizeof(*nt->mw_vec),
>> diff --git a/drivers/ntb/test/ntb_perf.c
>> b/drivers/ntb/test/ntb_perf.c
>> index 30635c8..4368519 100644
>> --- a/drivers/ntb/test/ntb_perf.c
>> +++ b/drivers/ntb/test/ntb_perf.c
>> @@ -143,8 +143,6 @@ enum {
>>       VERSION = 0,
>>       MW_SZ_HIGH,
>>       MW_SZ_LOW,
>> -     SPAD_MSG,
>> -     SPAD_ACK,
>>       MAX_SPAD
>>  };
>>
>> @@ -696,6 +694,12 @@ static int perf_probe(struct ntb_client *client,
>> struct ntb_dev *ntb)
>>       int node;
>>       int rc = 0;
>>
>> +     if (ntb_spad_count(ntb) < MAX_SPAD) {
>> +             dev_err(&ntb->dev, "Not enough scratch pad registers
>> for %s",
>> +                     DRIVER_NAME);
>> +             return -EIO;
>> +     }
>> +
>>       node = dev_to_node(&pdev->dev);
>>
>>       perf = kzalloc_node(sizeof(*perf), GFP_KERNEL, node);
>> --
>> 2.1.4
>>
>
> --
> You received this message because you are subscribed to the Google Groups "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+unsubscribe@googlegroups.com.
> To post to this group, send email to linux-ntb@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/1465320253.16234.176.camel%40intel.com.
> For more options, visit https://groups.google.com/d/optout.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web