[PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs
From: Yong Zhao
Avoid intermediate negative numbers when doing calculations with a mix
of signed and unsigned variables where implicit conversions can lead
to unexpected results.
When kernel queue buffer wraps around to 0, we need to check that rptr
won't be overwritten by the new packet.
Signed-off-by: Yong Zhao
Signed-off-by: Felix Kuehling
---
drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
index 9ebb4c1..1c66334 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
@@ -210,6 +210,11 @@ static int acquire_packet_buffer(struct kernel_queue *kq,
uint32_t wptr, rptr;
unsigned int *queue_address;
+ /* When rptr == wptr, the buffer is empty.
+* When rptr == wptr + 1, the buffer is full.
+* It is always rptr that advances to the position of wptr, rather than
+* the opposite. So we can only use up to queue_size_dwords - 1 dwords.
+*/
rptr = *kq->rptr_kernel;
wptr = *kq->wptr_kernel;
queue_address = (unsigned int *)kq->pq_kernel_addr;
@@ -219,11 +224,10 @@ static int acquire_packet_buffer(struct kernel_queue *kq,
pr_debug("wptr: %d\n", wptr);
pr_debug("queue_address 0x%p\n", queue_address);
- available_size = (rptr - 1 - wptr + queue_size_dwords) %
+ available_size = (rptr + queue_size_dwords - 1 - wptr) %
queue_size_dwords;
- if (packet_size_in_dwords >= queue_size_dwords ||
- packet_size_in_dwords >= available_size) {
+ if (packet_size_in_dwords > available_size) {
/*
* make sure calling functions know
* acquire_packet_buffer() failed
@@ -233,6 +237,14 @@ static int acquire_packet_buffer(struct kernel_queue *kq,
}
if (wptr + packet_size_in_dwords >= queue_size_dwords) {
+ /* make sure after rolling back to position 0, there is
+* still enough space.
+*/
+ if (packet_size_in_dwords >= rptr) {
+ *buffer_ptr = NULL;
+ return -ENOMEM;
+ }
+ /* fill nops, roll back and start at position 0 */
while (wptr > 0) {
queue_address[wptr] = kq->nop_packet;
wptr = (wptr + 1) % queue_size_dwords;
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs
On Sat, Sep 16, 2017 at 2:43 AM, Felix Kuehling wrote:
> From: Yong Zhao
>
> Avoid intermediate negative numbers when doing calculations with a mix
> of signed and unsigned variables where implicit conversions can lead
> to unexpected results.
>
> When kernel queue buffer wraps around to 0, we need to check that rptr
> won't be overwritten by the new packet.
>
> Signed-off-by: Yong Zhao
> Signed-off-by: Felix Kuehling
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 18 +++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
> index 9ebb4c1..1c66334 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
> @@ -210,6 +210,11 @@ static int acquire_packet_buffer(struct kernel_queue *kq,
> uint32_t wptr, rptr;
> unsigned int *queue_address;
>
> + /* When rptr == wptr, the buffer is empty.
Start comment text in a new line. First line should be just /*
> +* When rptr == wptr + 1, the buffer is full.
> +* It is always rptr that advances to the position of wptr, rather
> than
> +* the opposite. So we can only use up to queue_size_dwords - 1
> dwords.
> +*/
> rptr = *kq->rptr_kernel;
> wptr = *kq->wptr_kernel;
> queue_address = (unsigned int *)kq->pq_kernel_addr;
> @@ -219,11 +224,10 @@ static int acquire_packet_buffer(struct kernel_queue
> *kq,
> pr_debug("wptr: %d\n", wptr);
> pr_debug("queue_address 0x%p\n", queue_address);
>
> - available_size = (rptr - 1 - wptr + queue_size_dwords) %
> + available_size = (rptr + queue_size_dwords - 1 - wptr) %
> queue_size_dwords;
>
> - if (packet_size_in_dwords >= queue_size_dwords ||
> - packet_size_in_dwords >= available_size) {
> + if (packet_size_in_dwords > available_size) {
> /*
> * make sure calling functions know
> * acquire_packet_buffer() failed
> @@ -233,6 +237,14 @@ static int acquire_packet_buffer(struct kernel_queue *kq,
> }
>
> if (wptr + packet_size_in_dwords >= queue_size_dwords) {
> + /* make sure after rolling back to position 0, there is
> +* still enough space.
> +*/
> + if (packet_size_in_dwords >= rptr) {
> + *buffer_ptr = NULL;
> + return -ENOMEM;
> + }
I don't think the condition is correct.
Suppose, queue_size_dwords == 100, wptr == rptr == 50 (queue is empty)
and we have a new packet with size of 70.
Now, wptr + size is 120, which is >= 100
However, 70 >= rptr (50) which will give us -ENOMEM, but this is not
correct condition, because the packet *does* have enough room in the
queue.
I think the condition should be:
if (packet_size_in_dwords - (queue_size_dwords - wptr) >= rptr)
but please check this.
> + /* fill nops, roll back and start at position 0 */
> while (wptr > 0) {
> queue_address[wptr] = kq->nop_packet;
> wptr = (wptr + 1) % queue_size_dwords;
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs
On 2017-09-17 08:03 AM, Oded Gabbay wrote:
> On Sat, Sep 16, 2017 at 2:43 AM, Felix Kuehling
> wrote:
>> From: Yong Zhao
>>
>> Avoid intermediate negative numbers when doing calculations with a mix
>> of signed and unsigned variables where implicit conversions can lead
>> to unexpected results.
>>
>> When kernel queue buffer wraps around to 0, we need to check that rptr
>> won't be overwritten by the new packet.
>>
>> Signed-off-by: Yong Zhao
>> Signed-off-by: Felix Kuehling
>> ---
>> drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 18 +++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> index 9ebb4c1..1c66334 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> @@ -210,6 +210,11 @@ static int acquire_packet_buffer(struct kernel_queue
>> *kq,
>> uint32_t wptr, rptr;
>> unsigned int *queue_address;
>>
>> + /* When rptr == wptr, the buffer is empty.
> Start comment text in a new line. First line should be just /*
>
>> +* When rptr == wptr + 1, the buffer is full.
>> +* It is always rptr that advances to the position of wptr, rather
>> than
>> +* the opposite. So we can only use up to queue_size_dwords - 1
>> dwords.
>> +*/
>> rptr = *kq->rptr_kernel;
>> wptr = *kq->wptr_kernel;
>> queue_address = (unsigned int *)kq->pq_kernel_addr;
>> @@ -219,11 +224,10 @@ static int acquire_packet_buffer(struct kernel_queue
>> *kq,
>> pr_debug("wptr: %d\n", wptr);
>> pr_debug("queue_address 0x%p\n", queue_address);
>>
>> - available_size = (rptr - 1 - wptr + queue_size_dwords) %
>> + available_size = (rptr + queue_size_dwords - 1 - wptr) %
>> queue_size_dwords;
>>
>> - if (packet_size_in_dwords >= queue_size_dwords ||
>> - packet_size_in_dwords >= available_size) {
>> + if (packet_size_in_dwords > available_size) {
>> /*
>> * make sure calling functions know
>> * acquire_packet_buffer() failed
>> @@ -233,6 +237,14 @@ static int acquire_packet_buffer(struct kernel_queue
>> *kq,
>> }
>>
>> if (wptr + packet_size_in_dwords >= queue_size_dwords) {
>> + /* make sure after rolling back to position 0, there is
>> +* still enough space.
>> +*/
>> + if (packet_size_in_dwords >= rptr) {
>> + *buffer_ptr = NULL;
>> + return -ENOMEM;
>> + }
> I don't think the condition is correct.
> Suppose, queue_size_dwords == 100, wptr == rptr == 50 (queue is empty)
> and we have a new packet with size of 70.
> Now, wptr + size is 120, which is >= 100
> However, 70 >= rptr (50) which will give us -ENOMEM, but this is not
> correct condition, because the packet *does* have enough room in the
> queue.
Not really. We need 70 consecutive dwords. So the last 50 dwords of the
queue are not enough. Therefore we decide to wrap around to the
beginning of the queue and fill the last 50 dwords with NOPs. That means
we really need 70+50 dwords, including those NOPs. Now we only have 50
dwords left in the queue for the actual packet, which is not enough. So
we have no choice but to fail with ENOMEM.
I think the consequence is, that we can't guarantee any successful
allocations from the queue that need more than half of the queue.
Regards,
Felix
>
> I think the condition should be:
> if (packet_size_in_dwords - (queue_size_dwords - wptr) >= rptr)
> but please check this.
>
>> + /* fill nops, roll back and start at position 0 */
>> while (wptr > 0) {
>> queue_address[wptr] = kq->nop_packet;
>> wptr = (wptr + 1) % queue_size_dwords;
>> --
>> 2.7.4
>>
>> ___
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs
On 2017-09-17 08:03 AM, Oded Gabbay wrote:
> On Sat, Sep 16, 2017 at 2:43 AM, Felix Kuehling
> wrote:
>> From: Yong Zhao
>>
>> Avoid intermediate negative numbers when doing calculations with a mix
>> of signed and unsigned variables where implicit conversions can lead
>> to unexpected results.
>>
>> When kernel queue buffer wraps around to 0, we need to check that rptr
>> won't be overwritten by the new packet.
>>
>> Signed-off-by: Yong Zhao
>> Signed-off-by: Felix Kuehling
>> ---
>> drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 18 +++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> index 9ebb4c1..1c66334 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> @@ -210,6 +210,11 @@ static int acquire_packet_buffer(struct kernel_queue
>> *kq,
>> uint32_t wptr, rptr;
>> unsigned int *queue_address;
>>
>> + /* When rptr == wptr, the buffer is empty.
> Start comment text in a new line. First line should be just /*
That's a new rule to me, and it seems a bit arbitrary. checkpatch.pl
doesn't complain about this. checkpatch.pl does complain about the
closing */ not being on it's own line, but checkpatch has always been OK
with multiline comments starting on the same line as the opening /*.
There are also plenty of examples of multiline comments starting on the
same line as /*. For example run this grep on the include/linux:
grep -A3 '/\*[^*]\+[^/]$' *.h
Regards,
Felix
>
>> +* When rptr == wptr + 1, the buffer is full.
>> +* It is always rptr that advances to the position of wptr, rather
>> than
>> +* the opposite. So we can only use up to queue_size_dwords - 1
>> dwords.
>> +*/
>> rptr = *kq->rptr_kernel;
>> wptr = *kq->wptr_kernel;
>> queue_address = (unsigned int *)kq->pq_kernel_addr;
>> @@ -219,11 +224,10 @@ static int acquire_packet_buffer(struct kernel_queue
>> *kq,
>> pr_debug("wptr: %d\n", wptr);
>> pr_debug("queue_address 0x%p\n", queue_address);
>>
>> - available_size = (rptr - 1 - wptr + queue_size_dwords) %
>> + available_size = (rptr + queue_size_dwords - 1 - wptr) %
>> queue_size_dwords;
>>
>> - if (packet_size_in_dwords >= queue_size_dwords ||
>> - packet_size_in_dwords >= available_size) {
>> + if (packet_size_in_dwords > available_size) {
>> /*
>> * make sure calling functions know
>> * acquire_packet_buffer() failed
>> @@ -233,6 +237,14 @@ static int acquire_packet_buffer(struct kernel_queue
>> *kq,
>> }
>>
>> if (wptr + packet_size_in_dwords >= queue_size_dwords) {
>> + /* make sure after rolling back to position 0, there is
>> +* still enough space.
>> +*/
>> + if (packet_size_in_dwords >= rptr) {
>> + *buffer_ptr = NULL;
>> + return -ENOMEM;
>> + }
> I don't think the condition is correct.
> Suppose, queue_size_dwords == 100, wptr == rptr == 50 (queue is empty)
> and we have a new packet with size of 70.
> Now, wptr + size is 120, which is >= 100
> However, 70 >= rptr (50) which will give us -ENOMEM, but this is not
> correct condition, because the packet *does* have enough room in the
> queue.
>
> I think the condition should be:
> if (packet_size_in_dwords - (queue_size_dwords - wptr) >= rptr)
> but please check this.
>
>> + /* fill nops, roll back and start at position 0 */
>> while (wptr > 0) {
>> queue_address[wptr] = kq->nop_packet;
>> wptr = (wptr + 1) % queue_size_dwords;
>> --
>> 2.7.4
>>
>> ___
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs
Correct (coming from the guy who did all of the checkpatch cleanup for KFD).
For multi-line comments, /* Can be on its own, or on the same line as the
comment. */ has to be on its own.
https://github.com/torvalds/linux/blob/master/scripts/checkpatch.pl#L3042
Kent
-Original Message-
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of Felix
Kuehling
Sent: Monday, September 18, 2017 11:22 AM
To: Oded Gabbay
Cc: Zhao, Yong; amd-gfx list
Subject: Re: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs
On 2017-09-17 08:03 AM, Oded Gabbay wrote:
> On Sat, Sep 16, 2017 at 2:43 AM, Felix Kuehling
> wrote:
>> From: Yong Zhao
>>
>> Avoid intermediate negative numbers when doing calculations with a
>> mix of signed and unsigned variables where implicit conversions can
>> lead to unexpected results.
>>
>> When kernel queue buffer wraps around to 0, we need to check that
>> rptr won't be overwritten by the new packet.
>>
>> Signed-off-by: Yong Zhao
>> Signed-off-by: Felix Kuehling
>> ---
>> drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 18
>> +++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> index 9ebb4c1..1c66334 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> @@ -210,6 +210,11 @@ static int acquire_packet_buffer(struct kernel_queue
>> *kq,
>> uint32_t wptr, rptr;
>> unsigned int *queue_address;
>>
>> + /* When rptr == wptr, the buffer is empty.
> Start comment text in a new line. First line should be just /*
That's a new rule to me, and it seems a bit arbitrary. checkpatch.pl doesn't
complain about this. checkpatch.pl does complain about the closing */ not being
on it's own line, but checkpatch has always been OK with multiline comments
starting on the same line as the opening /*.
There are also plenty of examples of multiline comments starting on the same
line as /*. For example run this grep on the include/linux:
grep -A3 '/\*[^*]\+[^/]$' *.h
Regards,
Felix
>
>> +* When rptr == wptr + 1, the buffer is full.
>> +* It is always rptr that advances to the position of wptr, rather
>> than
>> +* the opposite. So we can only use up to queue_size_dwords - 1
>> dwords.
>> +*/
>> rptr = *kq->rptr_kernel;
>> wptr = *kq->wptr_kernel;
>> queue_address = (unsigned int *)kq->pq_kernel_addr; @@
>> -219,11 +224,10 @@ static int acquire_packet_buffer(struct kernel_queue *kq,
>> pr_debug("wptr: %d\n", wptr);
>> pr_debug("queue_address 0x%p\n", queue_address);
>>
>> - available_size = (rptr - 1 - wptr + queue_size_dwords) %
>> + available_size = (rptr + queue_size_dwords - 1 - wptr) %
>>
>> queue_size_dwords;
>>
>> - if (packet_size_in_dwords >= queue_size_dwords ||
>> - packet_size_in_dwords >= available_size) {
>> + if (packet_size_in_dwords > available_size) {
>> /*
>> * make sure calling functions know
>> * acquire_packet_buffer() failed @@ -233,6 +237,14
>> @@ static int acquire_packet_buffer(struct kernel_queue *kq,
>> }
>>
>> if (wptr + packet_size_in_dwords >= queue_size_dwords) {
>> + /* make sure after rolling back to position 0, there is
>> +* still enough space.
>> +*/
>> + if (packet_size_in_dwords >= rptr) {
>> + *buffer_ptr = NULL;
>> + return -ENOMEM;
>> + }
> I don't think the condition is correct.
> Suppose, queue_size_dwords == 100, wptr == rptr == 50 (queue is empty)
> and we have a new packet with size of 70.
> Now, wptr + size is 120, which is >= 100 However, 70 >= rptr (50)
> which will give us -ENOMEM, but this is not correct condition, because
> the packet *does* have enough room in the queue.
>
> I think the condition should be:
> if (packet_size_in_dwords - (queue_size_dwords - wptr) >= rptr) but
> please check this.
>
>> + /* fill nops, roll back and start at position 0 */
>> while (wptr > 0) {
>> queue_address[wptr] = kq->nop_packet;
>> wptr = (wptr + 1) % queue_size_dwords;
>> --
>> 2.7.4
>>
>> ___
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs
Though to be fair, we should probably consolidate the comment style so that
it's actually consistent through the KFD.
Kent
-Original Message-
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of
Russell, Kent
Sent: Monday, September 18, 2017 11:28 AM
To: Kuehling, Felix; Oded Gabbay
Cc: Zhao, Yong; amd-gfx list
Subject: RE: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs
Correct (coming from the guy who did all of the checkpatch cleanup for KFD).
For multi-line comments, /* Can be on its own, or on the same line as the
comment. */ has to be on its own.
https://github.com/torvalds/linux/blob/master/scripts/checkpatch.pl#L3042
Kent
-Original Message-
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of Felix
Kuehling
Sent: Monday, September 18, 2017 11:22 AM
To: Oded Gabbay
Cc: Zhao, Yong; amd-gfx list
Subject: Re: [PATCH 09/11] drm/amdkfd: Fix kernel-queue wrapping bugs
On 2017-09-17 08:03 AM, Oded Gabbay wrote:
> On Sat, Sep 16, 2017 at 2:43 AM, Felix Kuehling
> wrote:
>> From: Yong Zhao
>>
>> Avoid intermediate negative numbers when doing calculations with a
>> mix of signed and unsigned variables where implicit conversions can
>> lead to unexpected results.
>>
>> When kernel queue buffer wraps around to 0, we need to check that
>> rptr won't be overwritten by the new packet.
>>
>> Signed-off-by: Yong Zhao
>> Signed-off-by: Felix Kuehling
>> ---
>> drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 18
>> +++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> index 9ebb4c1..1c66334 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> @@ -210,6 +210,11 @@ static int acquire_packet_buffer(struct kernel_queue
>> *kq,
>> uint32_t wptr, rptr;
>> unsigned int *queue_address;
>>
>> + /* When rptr == wptr, the buffer is empty.
> Start comment text in a new line. First line should be just /*
That's a new rule to me, and it seems a bit arbitrary. checkpatch.pl doesn't
complain about this. checkpatch.pl does complain about the closing */ not being
on it's own line, but checkpatch has always been OK with multiline comments
starting on the same line as the opening /*.
There are also plenty of examples of multiline comments starting on the same
line as /*. For example run this grep on the include/linux:
grep -A3 '/\*[^*]\+[^/]$' *.h
Regards,
Felix
>
>> +* When rptr == wptr + 1, the buffer is full.
>> +* It is always rptr that advances to the position of wptr, rather
>> than
>> +* the opposite. So we can only use up to queue_size_dwords - 1
>> dwords.
>> +*/
>> rptr = *kq->rptr_kernel;
>> wptr = *kq->wptr_kernel;
>> queue_address = (unsigned int *)kq->pq_kernel_addr; @@
>> -219,11 +224,10 @@ static int acquire_packet_buffer(struct kernel_queue *kq,
>> pr_debug("wptr: %d\n", wptr);
>> pr_debug("queue_address 0x%p\n", queue_address);
>>
>> - available_size = (rptr - 1 - wptr + queue_size_dwords) %
>> + available_size = (rptr + queue_size_dwords - 1 - wptr) %
>>
>> queue_size_dwords;
>>
>> - if (packet_size_in_dwords >= queue_size_dwords ||
>> - packet_size_in_dwords >= available_size) {
>> + if (packet_size_in_dwords > available_size) {
>> /*
>> * make sure calling functions know
>> * acquire_packet_buffer() failed @@ -233,6 +237,14
>> @@ static int acquire_packet_buffer(struct kernel_queue *kq,
>> }
>>
>> if (wptr + packet_size_in_dwords >= queue_size_dwords) {
>> + /* make sure after rolling back to position 0, there is
>> +* still enough space.
>> +*/
>> + if (packet_size_in_dwords >= rptr) {
>> + *buffer_ptr = NULL;
>> + return -ENOMEM;
>> + }
> I don't think the condition is correct.
> Suppose, queue_size_dwords == 100, wptr == rptr == 50 (queue is empty)
> and we have a new packet with size of 70.
> Now, wptr + size is 120, which is >= 100 However, 70 >= rptr (50)
> which will give us -ENOMEM, but this is not correct condition, because
> the packet *do
