[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <745be03a-2ef6-f59e-2f05-6e9858c133fd@gmail.com>
Date: Tue, 22 Mar 2022 20:29:01 +0530
From: Sathish Kumar <skumark1902@...il.com>
To: Greg KH <gregkh@...uxfoundation.org>,
"Fabio M. De Francesco" <fmdefrancesco@...il.com>
Cc: Larry.Finger@...inger.net, florian.c.schilhabel@...glemail.com,
linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] staging: rtl8712: Fix CamelCase warnings
On 22/03/22 5:01 pm, Greg KH wrote:
> On Tue, Mar 22, 2022 at 11:42:21AM +0100, Fabio M. De Francesco wrote:
>> On martedì 22 marzo 2022 05:30:29 CET Sathish Kumar wrote:
>>> On 18/03/22 4:58 pm, Greg KH wrote:
>>>> On Fri, Mar 18, 2022 at 03:44:40PM +0530, Sathish Kumar wrote:
>>>>> This patch fixes the checkpatch.pl warnings like:
>>>>> CHECK: Avoid CamelCase: <blnEnableRxFF0Filter>
>>>>> + u8 blnEnableRxFF0Filter;
>>>>>
>>>>> Signed-off-by: Sathish Kumar <skumark1902@...il.com>
>>>>> ---
>>>>> Changes in v2:
>>>>> - Remove the "bln" prefix
>>>>> ---
>>>>> drivers/staging/rtl8712/drv_types.h | 2 +-
>>>>> drivers/staging/rtl8712/rtl871x_cmd.c | 2 +-
>>>>> drivers/staging/rtl8712/xmit_linux.c | 4 ++--
>>>>> 3 files changed, 4 insertions(+), 4 deletions(-)
>>>>>
>>>>> [...]
>>>>>
>>>>> do {
>>>>> msleep(100);
>>>>> - } while (adapter->blnEnableRxFF0Filter == 1);
>>>>> + } while (adapter->enable_rx_ff0_filter == 1);
>>>> Ah, that's funny. It's amazing it works at all and that the compiler
>>>> doesn't optimize this away. This isn't a good pattern to use in kernel
>>> Do you mean the following code is not a good pattern in kernel?
>>> do {
>>> msleep();
>>> } while(condition);
>> Exactly, this is not a pattern that works as you expect :)
>>
>> I was waiting for Greg to detail something more about this subject but,
>> since it looks like he has no time yet to respond, I'll try to interpret
>> his words.
>>
>> (@Greg, please forgive me if I saying something different from what you
>> intended to convey :)).
>>
>> The reason why this pattern does not work as expected is too long to be
>> explained here. However, I think that Greg is suggesting to you to research
>> and use what are called "Condition variables".
> Kind of. The problem is that "condition" here is just looking at a
> random variable. There is no sort of assurance that the variable will
> actually change or that that compiler even has to read from memory for
> it. It could cache the value the first time it is read and then never
> update it for the whole loop logic.
>
> Please read Documentation/memory-barriers.txt for how to fix this all up
> and do it properly.
>
> Again, it's amazing that the current code even works at all. So maybe
> it doesn't! :)
>
> thansks,
>
> greg k-h
Thank you Greg and Fabio for your inputs. Will check and fix this up.
Regards,
Sathish
Powered by blists - more mailing lists