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

Groups > linux.kernel > #1330538 > unrolled thread

Re: [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values

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/6] ibmvscsi: Add and use enums for valid CRQ header  values Manoj Kumar <manoj@linux.vnet.ibm.com> - 2016-02-09 18:50 +0100
    Re: [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header  values Tyrel Datwyler <tyreld@linux.vnet.ibm.com> - 2016-02-09 19:30 +0100

#1330538 — Re: [PATCH 2/6] ibmvscsi: Add and use enums for valid CRQ header values

> Yeah, I can see how that is confusing. Since, all three possible valid
> crq message types have the first bit set I think this was originally a
> cute hack to grab anything that was likely valid. Then in
> ibmvscsi_handle_crq() we explicitly match the full header value in a
> switch statement logging anything that turned out actually invalid.
>
>>
>> If 'valid' will only have one of these four enums defined, would
>> this be better written as:
>>
>> 	if (crq->valid != VIOSRP_CRQ_FREE)
>
> This definitely would make the logic easier to read and follow. Also,
> this would make sure any crq with an invalid header that doesn't have
> its first bit set will also be logged by the ibmvscsi_handle_crq()
> switch statement default block and not silently ignored.
>
> -Tyrel

Sounds good, Tyrel. Does this mean I should expect a v2 of this patch
series?

- Manoj N. Kumar

[toc] | [next] | [standalone]

#1330573

On 02/09/2016 09:41 AM, Manoj Kumar wrote:
>> Yeah, I can see how that is confusing. Since, all three possible valid
>> crq message types have the first bit set I think this was originally a
>> cute hack to grab anything that was likely valid. Then in
>> ibmvscsi_handle_crq() we explicitly match the full header value in a
>> switch statement logging anything that turned out actually invalid.
>>
>>>
>>> If 'valid' will only have one of these four enums defined, would
>>> this be better written as:
>>>
>>> 	if (crq->valid != VIOSRP_CRQ_FREE)
>>
>> This definitely would make the logic easier to read and follow. Also,
>> this would make sure any crq with an invalid header that doesn't have
>> its first bit set will also be logged by the ibmvscsi_handle_crq()
>> switch statement default block and not silently ignored.
>>
>> -Tyrel
> 
> Sounds good, Tyrel. Does this mean I should expect a v2 of this patch
> series?
> 
> - Manoj N. Kumar

Haven't had a chance to clean up and resubmit, but yes there will be a
v2 coming along soon.

-Tyrel

> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web