diff mbox series

RDMA/irdma: Move variable into switch case

Message ID 20230201012823.105150-1-jack.vogel@oracle.com (mailing list archive)
State Rejected
Headers show
Series RDMA/irdma: Move variable into switch case | expand

Commit Message

Jack Vogel Feb. 1, 2023, 1:28 a.m. UTC
Fix build warnings when CONFIG_INIT_STACK_ALL_ZERO is enabled.

Signed-off-by: Jack Vogel <jack.vogel@oracle.com>
---
 drivers/infiniband/hw/irdma/hw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Leon Romanovsky Feb. 1, 2023, 10:11 a.m. UTC | #1
On Tue, Jan 31, 2023 at 05:28:23PM -0800, Jack Vogel wrote:
> Fix build warnings when CONFIG_INIT_STACK_ALL_ZERO is enabled.

Which warnings do you see? What is you compiler version?

The code is perfectly fine.

> 
> Signed-off-by: Jack Vogel <jack.vogel@oracle.com>
> ---
>  drivers/infiniband/hw/irdma/hw.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/irdma/hw.c b/drivers/infiniband/hw/irdma/hw.c
> index ab246447520b..e3c639a0d920 100644
> --- a/drivers/infiniband/hw/irdma/hw.c
> +++ b/drivers/infiniband/hw/irdma/hw.c
> @@ -272,8 +272,8 @@ static void irdma_process_aeq(struct irdma_pci_f *rf)
>  		}
>  
>  		switch (info->ae_id) {
> +		case IRDMA_AE_LLP_CONNECTION_ESTABLISHED: {
>  			struct irdma_cm_node *cm_node;
> -		case IRDMA_AE_LLP_CONNECTION_ESTABLISHED:
>  			cm_node = iwqp->cm_node;
>  			if (cm_node->accept_pend) {
>  				atomic_dec(&cm_node->listener->pend_accepts_cnt);
> @@ -281,7 +281,7 @@ static void irdma_process_aeq(struct irdma_pci_f *rf)
>  			}
>  			iwqp->rts_ae_rcvd = 1;
>  			wake_up_interruptible(&iwqp->waitq);
> -			break;
> +		} break;
>  		case IRDMA_AE_LLP_FIN_RECEIVED:
>  		case IRDMA_AE_RDMAP_ROE_BAD_LLP_CLOSE:
>  			if (qp->term_flags)
> -- 
> 2.39.1
>
Jack Vogel Feb. 2, 2023, 12:09 a.m. UTC | #2
Hey Leon,

Oracle switched to GCC11 in our UEK7/OL9 releases recently, leading up to that release we added the ALL_ZERO config option, and then ran into some warnings, our build treats warnings as errors and would fail. For instance this thread: 

https://lkml.iu.edu/hypermail/linux/kernel/2202.1/05558.html

A number of changes were made in the mainline code by Kees Cook and even made it into the linux-5.15.y branch, but a couple of them we have carried as specials for the past year, I was recently prodded about the matter again by an internal group, so I thought I would submit these patches upstream. 

I must apologize though, for unbeknownst to me, our tools team actually back ported the fix from gcc12 regarding these warnings and forgot to tell the UEK group about it :) It wasn’t until you asked about the warnings, I reverted the commits and did a build to capture them, then I discovered they no longer occur. So, sorry about the noise. I will be reverting our own changes as unnecessary now.

Regards,

Jack

> On Feb 1, 2023, at 2:11 AM, Leon Romanovsky <leon@kernel.org> wrote:
> 
> On Tue, Jan 31, 2023 at 05:28:23PM -0800, Jack Vogel wrote:
>> Fix build warnings when CONFIG_INIT_STACK_ALL_ZERO is enabled.
> 
> Which warnings do you see? What is you compiler version?
> 
> The code is perfectly fine.
> 
>> 
>> Signed-off-by: Jack Vogel <jack.vogel@oracle.com>
>> ---
>> drivers/infiniband/hw/irdma/hw.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/infiniband/hw/irdma/hw.c b/drivers/infiniband/hw/irdma/hw.c
>> index ab246447520b..e3c639a0d920 100644
>> --- a/drivers/infiniband/hw/irdma/hw.c
>> +++ b/drivers/infiniband/hw/irdma/hw.c
>> @@ -272,8 +272,8 @@ static void irdma_process_aeq(struct irdma_pci_f *rf)
>> 		}
>> 
>> 		switch (info->ae_id) {
>> +		case IRDMA_AE_LLP_CONNECTION_ESTABLISHED: {
>> 			struct irdma_cm_node *cm_node;
>> -		case IRDMA_AE_LLP_CONNECTION_ESTABLISHED:
>> 			cm_node = iwqp->cm_node;
>> 			if (cm_node->accept_pend) {
>> 				atomic_dec(&cm_node->listener->pend_accepts_cnt);
>> @@ -281,7 +281,7 @@ static void irdma_process_aeq(struct irdma_pci_f *rf)
>> 			}
>> 			iwqp->rts_ae_rcvd = 1;
>> 			wake_up_interruptible(&iwqp->waitq);
>> -			break;
>> +		} break;
>> 		case IRDMA_AE_LLP_FIN_RECEIVED:
>> 		case IRDMA_AE_RDMAP_ROE_BAD_LLP_CLOSE:
>> 			if (qp->term_flags)
>> -- 
>> 2.39.1
>>
Leon Romanovsky Feb. 2, 2023, 7:58 a.m. UTC | #3
On Thu, Feb 02, 2023 at 12:09:50AM +0000, Jack Vogel wrote:
> Hey Leon,
> 
> Oracle switched to GCC11 in our UEK7/OL9 releases recently, leading up to that release we added the ALL_ZERO config option, and then ran into some warnings, our build treats warnings as errors and would fail. For instance this thread: 
> 
> https://lkml.iu.edu/hypermail/linux/kernel/2202.1/05558.html
> 
> A number of changes were made in the mainline code by Kees Cook and even made it into the linux-5.15.y branch, but a couple of them we have carried as specials for the past year, I was recently prodded about the matter again by an internal group, so I thought I would submit these patches upstream. 
> 
> I must apologize though, for unbeknownst to me, our tools team actually back ported the fix from gcc12 regarding these warnings and forgot to tell the UEK group about it :) It wasn’t until you asked about the warnings, I reverted the commits and did a build to capture them, then I discovered they no longer occur. So, sorry about the noise. I will be reverting our own changes as unnecessary now.

Glad to hear.

Thanks
Jason Gunthorpe Feb. 3, 2023, 2:44 p.m. UTC | #4
On Thu, Feb 02, 2023 at 09:58:47AM +0200, Leon Romanovsky wrote:
> On Thu, Feb 02, 2023 at 12:09:50AM +0000, Jack Vogel wrote:
> > Hey Leon,
> > 
> > Oracle switched to GCC11 in our UEK7/OL9 releases recently, leading up to that release we added the ALL_ZERO config option, and then ran into some warnings, our build treats warnings as errors and would fail. For instance this thread: 
> > 
> > https://lkml.iu.edu/hypermail/linux/kernel/2202.1/05558.html
> > 
> > A number of changes were made in the mainline code by Kees Cook and even made it into the linux-5.15.y branch, but a couple of them we have carried as specials for the past year, I was recently prodded about the matter again by an internal group, so I thought I would submit these patches upstream. 
> > 
> > I must apologize though, for unbeknownst to me, our tools team actually back ported the fix from gcc12 regarding these warnings and forgot to tell the UEK group about it :) It wasn’t until you asked about the warnings, I reverted the commits and did a build to capture them, then I discovered they no longer occur. So, sorry about the noise. I will be reverting our own changes as unnecessary now.
> 
> Glad to hear.

Regardless, it is really weird coding style to have a variable
block immediately after the switch statement

Jason
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/irdma/hw.c b/drivers/infiniband/hw/irdma/hw.c
index ab246447520b..e3c639a0d920 100644
--- a/drivers/infiniband/hw/irdma/hw.c
+++ b/drivers/infiniband/hw/irdma/hw.c
@@ -272,8 +272,8 @@  static void irdma_process_aeq(struct irdma_pci_f *rf)
 		}
 
 		switch (info->ae_id) {
+		case IRDMA_AE_LLP_CONNECTION_ESTABLISHED: {
 			struct irdma_cm_node *cm_node;
-		case IRDMA_AE_LLP_CONNECTION_ESTABLISHED:
 			cm_node = iwqp->cm_node;
 			if (cm_node->accept_pend) {
 				atomic_dec(&cm_node->listener->pend_accepts_cnt);
@@ -281,7 +281,7 @@  static void irdma_process_aeq(struct irdma_pci_f *rf)
 			}
 			iwqp->rts_ae_rcvd = 1;
 			wake_up_interruptible(&iwqp->waitq);
-			break;
+		} break;
 		case IRDMA_AE_LLP_FIN_RECEIVED:
 		case IRDMA_AE_RDMAP_ROE_BAD_LLP_CLOSE:
 			if (qp->term_flags)