Re: [PATCH] Revert "staging:r8188eu: use lib80211 CCMP decrypt"

2019-01-01 Thread Larry Finger

On 1/1/19 1:31 PM, Michael Straube wrote:


I've tested your patch and it solved the issue. No freezes and dmesg looks good.

I noticed that try_then_request_module() is also used in rtw_wep_encrypt() and
rtw_wep_decrypt(). I guess that also could cause problems?


Yes, I believe it would if anyone is still using WEP. My plan is to get rid of 
the try_then_request_module() there as well, and for completeness, I plan to 
restore usage of the lib80211 routines for TKIP as well.


Once I get a chance to test the TKIP and WEP changes, I plan to have a set of 4 
patches to switch the driver to using lib80211 routines for all 
decryption/encryption.


Larry


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] Revert "staging:r8188eu: use lib80211 CCMP decrypt"

2019-01-01 Thread Larry Finger

On 1/1/19 3:02 AM, Ivan Safonov wrote:
I suggested a patch for loading modules from interruptible mode, but this patch 
remained unclaimed ( 
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-August/124851.html 
).


For some reason I thought that this patch had been removed and did not track the 
fate of this code ( 
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-August/124573.html 
).


That patch was quite extensive, but had only a minimal commit message. I'm 
surprised that it did not get a bad review, but I can see why it was ignored.


Where did you submit that patch? It certainly was never in my mailbox.

Larry
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] Revert "staging:r8188eu: use lib80211 CCMP decrypt"

2019-01-01 Thread Michael Straube


On 1/1/19 10:02 AM, Ivan Safonov wrote:

I suggested a patch for loading modules from interruptible mode, but this patch 
remained unclaimed ( 
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-August/124851.html
 ).



So with these changes try_then_request_module() would work properly?


For some reason I thought that this patch had been removed and did not track 
the fate of this code ( 
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-August/124573.html
 ).



I reverted that patch (there are conflicts meanwhile) and removed
try_then_request_module() in rtw_aes_encrypt() and it looks good.

Perhaps the same applies for the reverted TKIP changes?

Michael


On 1/1/19 5:17 AM, Larry Finger wrote:

On 12/30/18 12:39 PM, Michael Straube wrote:

Commit 6bd082af7e36 ("staging:r8188eu: use lib80211 CCMP decrypt")
is causing hardfreeze whenever the driver tries to connect to my wifi
network. That makes the driver unusable on my system. Reverting the
commit fixes the issue and the driver works properly.

Dec 29 19:21:17 gentoo kernel: BUG: scheduling while atomic: 
swapper/6/0/0x0100


Michael,

I have verified the freezes that you see. Although I have not been able to 
capture the console dump, I think we are likely seeing the same problem.

I do have a work-around in that I have not gotten any freezes when I force 
module lib80211_crypt_ccmp to be loaded before I load module r8188eu. This clue 
was used in finding what seems to be a good fix.

I do not know anything about demand loading of modules using 
try_then_request_module(); however, I noticed that the macro actually calls 
__request_module(), which has the following comment:

  * Load a module using the user mode module loader. The function returns
  * zero on success or a negative errno code or positive exit code from
  * "modprobe" on failure. Note that a successful module load does not mean
  * the module did not then unload and exit on an error of its own. Callers
  * must check that the service they requested is now available not blindly
  * invoke it.

I note that it says "user mode module loader". Routine rtw_aes_decrypt() is likely inside 
some sort of locking, which leads to the "scheduling while atomic" bug that you see. As a 
result, I suspect that the module is not loaded, and that leads to the NULL dereference when the 
module is accessed. Please try the one-line patch attached, which forces lib80211 to load when 
r8188eu is loaded. With this patch, I have been connected to an AES-encrypted AP for nearly 3 hours 
with no problems.

Larry



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] Revert "staging:r8188eu: use lib80211 CCMP decrypt"

2019-01-01 Thread Michael Straube

On 1/1/19 3:17 AM, Larry Finger wrote:

On 12/30/18 12:39 PM, Michael Straube wrote:

Commit 6bd082af7e36 ("staging:r8188eu: use lib80211 CCMP decrypt")
is causing hardfreeze whenever the driver tries to connect to my wifi
network. That makes the driver unusable on my system. Reverting the
commit fixes the issue and the driver works properly.

Dec 29 19:21:17 gentoo kernel: BUG: scheduling while atomic: 
swapper/6/0/0x0100


Michael,

I have verified the freezes that you see. Although I have not been able to 
capture the console dump, I think we are likely seeing the same problem.

I do have a work-around in that I have not gotten any freezes when I force 
module lib80211_crypt_ccmp to be loaded before I load module r8188eu. This clue 
was used in finding what seems to be a good fix.

I do not know anything about demand loading of modules using 
try_then_request_module(); however, I noticed that the macro actually calls 
__request_module(), which has the following comment:

  * Load a module using the user mode module loader. The function returns
  * zero on success or a negative errno code or positive exit code from
  * "modprobe" on failure. Note that a successful module load does not mean
  * the module did not then unload and exit on an error of its own. Callers
  * must check that the service they requested is now available not blindly
  * invoke it.

I note that it says "user mode module loader". Routine rtw_aes_decrypt() is likely inside 
some sort of locking, which leads to the "scheduling while atomic" bug that you see. As a 
result, I suspect that the module is not loaded, and that leads to the NULL dereference when the 
module is accessed. Please try the one-line patch attached, which forces lib80211 to load when 
r8188eu is loaded. With this patch, I have been connected to an AES-encrypted AP for nearly 3 hours 
with no problems.

Larry




I've tested your patch and it solved the issue. No freezes and dmesg looks good.

I noticed that try_then_request_module() is also used in rtw_wep_encrypt() and
rtw_wep_decrypt(). I guess that also could cause problems?

Michael


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2] ARM: staging: bcm2835-audio: interpolate audio delay

2019-01-01 Thread Mike Brady
Apologies for the delay coming back to this.

Having had a long look at the further implications of this proposed change, I 
would like to withdraw the suggested patch.

I agree that is would seem excessive to have to change the PCM core to 
accommodate it. Furthermore, the idea only works if is is legitimate to assume 
that frames are smoothly consumed in the interpolation interval. If that 
assumption were not to hold for some reason, then the delay reported would be 
wrong.

Thanks to everyone for their comments and inputs.

Regards
Mike



> On 13 Nov 2018, at 16:50, Takashi Iwai  wrote:
> 
> On Sun, 11 Nov 2018 19:21:29 +0100,
> Mike Brady wrote:
>> 
>> /* hardware definition */
>> static const struct snd_pcm_hardware snd_bcm2835_playback_hw = {
>>  .info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
>>   SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
>> - SNDRV_PCM_INFO_DRAIN_TRIGGER | SNDRV_PCM_INFO_SYNC_APPLPTR),
>> + SNDRV_PCM_INFO_BATCH | SNDRV_PCM_INFO_DRAIN_TRIGGER |
>> + SNDRV_PCM_INFO_SYNC_APPLPTR),
> 
> As already mentioned, the addition of SNDRV_PCM_INFO_BATCH should be
> irrelevant with this change.  Please create another patch to add this
> and clarify it in the changelog.
> 
>> diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
>> index 4e6110d778bd..574df7d7a1fa 100644
>> --- a/sound/core/pcm_lib.c
>> +++ b/sound/core/pcm_lib.c
>> @@ -229,19 +229,38 @@ static void update_audio_tstamp(struct 
>> snd_pcm_substream *substream,
>>  (runtime->audio_tstamp_report.actual_type ==
>>  SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT)) {
>> 
>> -/*
>> - * provide audio timestamp derived from pointer position
>> - * add delay only if requested
>> - */
>> +// provide audio timestamp derived from pointer position
>> 
>>  audio_frames = runtime->hw_ptr_wrap + runtime->status->hw_ptr;
>> 
>> -if (runtime->audio_tstamp_config.report_delay) {
>> +/*
>> + * If the runtime->delay is greater than zero, it's a
>> + * genuine delay, e.g. a delay due to a hardware fifo.
>> + *
>> + * But if the runtime->delay is less than zero, it's an
>> + * interpolated estimate of the number of frames output
>> + * since the hardware pointer was last updated.
>> + *
>> + * It would be calculated in the pointer callback.
>> + * For example, for the bcm_2835 driver, it is calculated in
>> + * snd_bcm2835_pcm_pointer().
>> + */
>> +
>> +if (runtime->delay < 0) {
>> +// The delay is an interpolated estimate...
>>  if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>> -audio_frames -=  runtime->delay;
>> -else
>> -audio_frames +=  runtime->delay;
>> +audio_frames += runtime->delay;
>> +} else {
>> +// The delay is a real delay. Add it if requested.
>> +if (runtime->audio_tstamp_config.report_delay) {
>> +if (substream->stream ==
>> +SNDRV_PCM_STREAM_PLAYBACK)
>> +audio_frames -=  runtime->delay;
>> +else
>> +audio_frames +=  runtime->delay;
>> +}
>>  }
> 
> Well, honestly speaking, I'm really not fond of changing the PCM core
> just for this.
> 
> Can we make bcm audio driver providing the finer pointer update
> instead?  If we have a module option to disable that behavior, it's an
> enough excuse in case anyone really cares about the accuracy.
> 
> 
> thanks,
> 
> Takashi

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] Revert "staging:r8188eu: use lib80211 CCMP decrypt"

2019-01-01 Thread Ivan Safonov
I suggested a patch for loading modules from interruptible mode, but 
this patch remained unclaimed ( 
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-August/124851.html 
).


For some reason I thought that this patch had been removed and did not 
track the fate of this code ( 
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-August/124573.html 
).


On 1/1/19 5:17 AM, Larry Finger wrote:

On 12/30/18 12:39 PM, Michael Straube wrote:

Commit 6bd082af7e36 ("staging:r8188eu: use lib80211 CCMP decrypt")
is causing hardfreeze whenever the driver tries to connect to my wifi
network. That makes the driver unusable on my system. Reverting the
commit fixes the issue and the driver works properly.

Dec 29 19:21:17 gentoo kernel: BUG: scheduling while atomic: 
swapper/6/0/0x0100


Michael,

I have verified the freezes that you see. Although I have not been able 
to capture the console dump, I think we are likely seeing the same problem.


I do have a work-around in that I have not gotten any freezes when I 
force module lib80211_crypt_ccmp to be loaded before I load module 
r8188eu. This clue was used in finding what seems to be a good fix.


I do not know anything about demand loading of modules using 
try_then_request_module(); however, I noticed that the macro actually 
calls __request_module(), which has the following comment:


  * Load a module using the user mode module loader. The function returns
  * zero on success or a negative errno code or positive exit code from
  * "modprobe" on failure. Note that a successful module load does not mean
  * the module did not then unload and exit on an error of its own. Callers
  * must check that the service they requested is now available not blindly
  * invoke it.

I note that it says "user mode module loader". Routine rtw_aes_decrypt() 
is likely inside some sort of locking, which leads to the "scheduling 
while atomic" bug that you see. As a result, I suspect that the module 
is not loaded, and that leads to the NULL dereference when the module is 
accessed. Please try the one-line patch attached, which forces lib80211 
to load when r8188eu is loaded. With this patch, I have been connected 
to an AES-encrypted AP for nearly 3 hours with no problems.


Larry



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel