* Re: ate_resource->lowest_free_index signed or unsigned?
2009-04-20 16:45 ate_resource->lowest_free_index signed or unsigned? Roel Kluin
@ 2009-04-21 6:46 ` Jes Sorensen
2009-04-21 11:21 ` Roel Kluin
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Jes Sorensen @ 2009-04-21 6:46 UTC (permalink / raw)
To: linux-ia64
Roel Kluin wrote:
> In arch/ia64/include/asm/sn/pcibr_provider.h:94
>
> struct ate_resource{
> u64 *ate;
> u64 num_ate;
> u64 lowest_free_index;
> };
>
> as you see lowest_free_index is unsigned,
>
> but in alloc_ate_resource() and free_ate_resource() it is
> treated as signed:
Hi Roel,
I don't thing it's a big issue to be honest, it just means the tests for
lowest_free_index < 0 are no-ops. I would be fine with a patch that
changes it to signed.
Cheers,
Jes
> static inline void free_ate_resource(struct ate_resource *ate_resource,
> int start)
> {
> mark_ate(ate_resource, start, ate_resource->ate[start], 0);
> if ((ate_resource->lowest_free_index > start) ||
> (ate_resource->lowest_free_index < 0))
> ate_resource->lowest_free_index = start;
> }
>
> /*
> * alloc_ate_resource: Allocate the requested number of ATEs.
> */
> static inline int alloc_ate_resource(struct ate_resource *ate_resource,
> int ate_needed)
> {
> int start_index;
>
> /*
> * Check for ate exhaustion.
> */
> if (ate_resource->lowest_free_index < 0)
> return -1;
>
> /*
> * Find the required number of free consecutive ates.
> */
> start_index > find_free_ate(ate_resource, ate_resource->lowest_free_index,
> ate_needed);
> if (start_index >= 0)
> mark_ate(ate_resource, start_index, ate_needed, ate_needed);
>
> ate_resource->lowest_free_index > find_free_ate(ate_resource, ate_resource->lowest_free_index, 1);
>
> return start_index;
> }
>
> should it be signed instead?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: ate_resource->lowest_free_index signed or unsigned?
2009-04-20 16:45 ate_resource->lowest_free_index signed or unsigned? Roel Kluin
2009-04-21 6:46 ` Jes Sorensen
@ 2009-04-21 11:21 ` Roel Kluin
2009-04-21 11:31 ` Jes Sorensen
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Roel Kluin @ 2009-04-21 11:21 UTC (permalink / raw)
To: linux-ia64
Jes Sorensen wrote:
> Roel Kluin wrote:
>> lowest_free_index is unsigned, but in alloc_ate_resource() and
>> free_ate_resource() it is treated as signed
>
> I don't thing it's a big issue to be honest, it just means the tests for
> lowest_free_index < 0 are no-ops. I would be fine with a patch that
> changes it to signed.
Ok, How's this? If my changelog is not clear feel free to amend it.
------------------------------>8-------------8<---------------------------------
unsigned lowest_free_index was set in alloc_ate_resource() with the return
value of find_free_ate(), which is -1 when no free ate is found. Since
unsigned, a lowest_free_index of -1, `ate exhaustion', would then not be noticed
in a subsequent call to alloc_ate_resource().
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/arch/ia64/include/asm/sn/pcibr_provider.h b/arch/ia64/include/asm/sn/pcibr_provider.h
index da205b7..b3133ef 100644
--- a/arch/ia64/include/asm/sn/pcibr_provider.h
+++ b/arch/ia64/include/asm/sn/pcibr_provider.h
@@ -94,7 +94,7 @@
struct ate_resource{
u64 *ate;
u64 num_ate;
- u64 lowest_free_index;
+ int lowest_free_index;
};
struct pcibus_info {
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: ate_resource->lowest_free_index signed or unsigned?
2009-04-20 16:45 ate_resource->lowest_free_index signed or unsigned? Roel Kluin
2009-04-21 6:46 ` Jes Sorensen
2009-04-21 11:21 ` Roel Kluin
@ 2009-04-21 11:31 ` Jes Sorensen
2009-04-21 12:15 ` Roel Kluin
2009-04-23 11:35 ` Jes Sorensen
4 siblings, 0 replies; 6+ messages in thread
From: Jes Sorensen @ 2009-04-21 11:31 UTC (permalink / raw)
To: linux-ia64
Roel Kluin wrote:
> Jes Sorensen wrote:
>> Roel Kluin wrote:
>>> lowest_free_index is unsigned, but in alloc_ate_resource() and
>>> free_ate_resource() it is treated as signed
>> I don't thing it's a big issue to be honest, it just means the tests for
>> lowest_free_index < 0 are no-ops. I would be fine with a patch that
>> changes it to signed.
>
> Ok, How's this? If my changelog is not clear feel free to amend it.
Hi Roel,
Please make it 'long' or 's64' to keep the size in the struct.
Cheers,
Jes
> ------------------------------>8-------------8<---------------------------------
> unsigned lowest_free_index was set in alloc_ate_resource() with the return
> value of find_free_ate(), which is -1 when no free ate is found. Since
> unsigned, a lowest_free_index of -1, `ate exhaustion', would then not be noticed
> in a subsequent call to alloc_ate_resource().
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> diff --git a/arch/ia64/include/asm/sn/pcibr_provider.h b/arch/ia64/include/asm/sn/pcibr_provider.h
> index da205b7..b3133ef 100644
> --- a/arch/ia64/include/asm/sn/pcibr_provider.h
> +++ b/arch/ia64/include/asm/sn/pcibr_provider.h
> @@ -94,7 +94,7 @@
> struct ate_resource{
> u64 *ate;
> u64 num_ate;
> - u64 lowest_free_index;
> + int lowest_free_index;
> };
>
> struct pcibus_info {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: ate_resource->lowest_free_index signed or unsigned?
2009-04-20 16:45 ate_resource->lowest_free_index signed or unsigned? Roel Kluin
` (2 preceding siblings ...)
2009-04-21 11:31 ` Jes Sorensen
@ 2009-04-21 12:15 ` Roel Kluin
2009-04-23 11:35 ` Jes Sorensen
4 siblings, 0 replies; 6+ messages in thread
From: Roel Kluin @ 2009-04-21 12:15 UTC (permalink / raw)
To: linux-ia64
Jes Sorensen wrote:
> Roel Kluin wrote:
>> Jes Sorensen wrote:
>>> Roel Kluin wrote:
>>>> lowest_free_index is unsigned, but in alloc_ate_resource() and
>>>> free_ate_resource() it is treated as signed
>>> I don't thing it's a big issue to be honest, it just means the tests for
>>> lowest_free_index < 0 are no-ops. I would be fine with a patch that
>>> changes it to signed.
> Please make it 'long' or 's64' to keep the size in the struct.
------------------------------>8-------------8<---------------------------------
unsigned lowest_free_index was set in alloc_ate_resource() with the return
value of find_free_ate(), which is -1 when no free ate is found. Since
unsigned, a lowest_free_index of -1, `ate exhaustion', would then not be noticed
in a subsequent call to alloc_ate_resource().
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/arch/ia64/include/asm/sn/pcibr_provider.h b/arch/ia64/include/asm/sn/pcibr_provider.h
index da205b7..636d914 100644
--- a/arch/ia64/include/asm/sn/pcibr_provider.h
+++ b/arch/ia64/include/asm/sn/pcibr_provider.h
@@ -94,7 +94,7 @@
struct ate_resource{
u64 *ate;
u64 num_ate;
- u64 lowest_free_index;
+ s64 lowest_free_index;
};
struct pcibus_info {
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: ate_resource->lowest_free_index signed or unsigned?
2009-04-20 16:45 ate_resource->lowest_free_index signed or unsigned? Roel Kluin
` (3 preceding siblings ...)
2009-04-21 12:15 ` Roel Kluin
@ 2009-04-23 11:35 ` Jes Sorensen
4 siblings, 0 replies; 6+ messages in thread
From: Jes Sorensen @ 2009-04-23 11:35 UTC (permalink / raw)
To: linux-ia64
Roel Kluin wrote:
> Jes Sorensen wrote:
>> Roel Kluin wrote:
>>> Jes Sorensen wrote:
>>>> Roel Kluin wrote:
>>>>> lowest_free_index is unsigned, but in alloc_ate_resource() and
>>>>> free_ate_resource() it is treated as signed
>>>> I don't thing it's a big issue to be honest, it just means the tests for
>>>> lowest_free_index < 0 are no-ops. I would be fine with a patch that
>>>> changes it to signed.
>
>> Please make it 'long' or 's64' to keep the size in the struct.
Hi Roel,
I ran some basic tests, seems fine.
Acked-by: Jes Sorensen <jes@sgi.com>
Cheers,
Jes
> ------------------------------>8-------------8<---------------------------------
> unsigned lowest_free_index was set in alloc_ate_resource() with the return
> value of find_free_ate(), which is -1 when no free ate is found. Since
> unsigned, a lowest_free_index of -1, `ate exhaustion', would then not be noticed
> in a subsequent call to alloc_ate_resource().
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> diff --git a/arch/ia64/include/asm/sn/pcibr_provider.h b/arch/ia64/include/asm/sn/pcibr_provider.h
> index da205b7..636d914 100644
> --- a/arch/ia64/include/asm/sn/pcibr_provider.h
> +++ b/arch/ia64/include/asm/sn/pcibr_provider.h
> @@ -94,7 +94,7 @@
> struct ate_resource{
> u64 *ate;
> u64 num_ate;
> - u64 lowest_free_index;
> + s64 lowest_free_index;
> };
>
> struct pcibus_info {
^ permalink raw reply [flat|nested] 6+ messages in thread