Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

wrong calculation of divf #28

Open
maximus21 opened this issue Dec 17, 2013 · 5 comments
Open

wrong calculation of divf #28

maximus21 opened this issue Dec 17, 2013 · 5 comments

Comments

@maximus21
Copy link

too large shift of dividend happens before division by target frequency. this leads to divf >=1024 in certain cases, for instance when target frequency is 1024000. divf should be calculated as a remainder from division by 1024 as it was done in the old version.
@koalo
Copy link
Owner

koalo commented Dec 17, 2013

You are right that this has changed, but intentionally because there was a bug in the old version - the factor 1024 was wrong and should have been 4096:
The goal is to do this calculation (in your example case):
500000000 / 1024000 = 488.28125
The binary representation is this: 111101000.01001
We have 12 bits in the fractional part, therefore the correct value of the register (without the leading password and bits) would be
111101000010010000000

Old version:
dividend = 500000000
dividend = 500000000*1024 = 512000000000
dividend = 512000000000 / 1024000 = 500000
divi = 500000 / 1024 = 488
divf = 500000 % 1024 = 288
Value of register:
111101000000100100000

New version:
dividend = 500000000
dividend = 500000000 << 12 = 2048000000000
dividend = 2048000000000 / 1024000 = 2000000
divi = 2000000 >> 12 = 488
divf = 2000000 & 0xFFF = 1152
Value of register:
111101000010010000000

@maximus21
Copy link
Author

Florian, the formula for average frequency in mash module is source/(divi+divf/1024). The fact that 12 bits are used to store the value doesn't mean that all 12 bits are used. What is your source of information regarding division by 4096?
It might be invisible in most setups, but in my setup codec and soc don't have connection for bit clock and that's where the difference is visible. 3.8 works perfectly, the new driver fails to produce 1.024Mhz.

koalo notifications@github.com wrote:

You are right that this has changed, but intentionally because there
was a bug in the old version - the factor 1024 was wrong and should
have been 4096:
The goal is to do this calculation (in your example case):
500000000 / 1024000 = 488.28125
The binary representation is this: 111101000.01001
We have 12 bits in the fractional part, therefore the correct value of
the register (without the leading password and bits) would be
111101000010010000000

Old version:
dividend = 500000000
dividend = 500000000*1024 = 512000000000
dividend = 512000000000 / 1024000 = 500000
divi = 500000 / 1024 = 488
divf = 500000 % 1024 = 288
Value of register:
111101000000100100000

New version:
dividend = 500000000
dividend = 500000000 << 12 = 2048000000000
dividend = 2048000000000 / 1024000 = 2000000
divi = 2000000 >> 12 = 488
divf = 2000000 & 0xFFF = 1152
Value of register:
111101000010010000000

Reply to this email directly or view it on GitHub:
#28 (comment)

@koalo
Copy link
Owner

koalo commented Dec 17, 2013

It is kind of strange to store a 1023 max value in 12 bits, but ok - I attached more importance to the bit field than to the formula.
What is the frequency you measure now?

@maximus21
Copy link
Author

I can't measure the frequency reliably. Maybe we should request clarification from the foundation people.

koalo notifications@github.com wrote:

It is kind of strange to store a 1023 max value in 12 bits, but ok - I
attached more importance to the bit field than to the formula.
What is the frequency you measure now?

Reply to this email directly or view it on GitHub:
#28 (comment)

@koalo
Copy link
Owner

koalo commented Dec 17, 2013

If we assume the 1024 is right, the frequency is now: 1022234 Hz
If we assume the 4096 is right, the frequency was before: 1024443 Hz
How have you noticed that?

koalo pushed a commit that referenced this issue Aug 4, 2014
@jtlayton @smfrench
cifs: ensure that srv_mutex is held when dealing with ssocket pointer
73e216a 
Oleksii reported that he had seen an oops similar to this:

BUG: unable to handle kernel NULL pointer dereference at 0000000000000088
IP: [<ffffffff814dcc13>] sock_sendmsg+0x93/0xd0
PGD 0
Oops: 0000 [#1] PREEMPT SMP
Modules linked in: ipt_MASQUERADE xt_REDIRECT xt_tcpudp iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack ip_tables x_tables carl9170 ath usb_storage f2fs nfnetlink_log nfnetlink md4 cifs dns_resolver hid_generic usbhid hid af_packet uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_core videodev rfcomm btusb bnep bluetooth qmi_wwan qcserial cdc_wdm usb_wwan usbnet usbserial mii snd_hda_codec_hdmi snd_hda_codec_realtek iwldvm mac80211 coretemp intel_powerclamp kvm_intel kvm iwlwifi snd_hda_intel cfg80211 snd_hda_codec xhci_hcd e1000e ehci_pci snd_hwdep sdhci_pci snd_pcm ehci_hcd microcode psmouse sdhci thinkpad_acpi mmc_core i2c_i801 pcspkr usbcore hwmon snd_timer snd_page_alloc snd ptp rfkill pps_core soundcore evdev usb_common vboxnetflt(O) vboxdrv(O)Oops#2 Part8
 loop tun binfmt_misc fuse msr acpi_call(O) ipv6 autofs4
CPU: 0 PID: 21612 Comm: kworker/0:1 Tainted: G        W  O 3.10.1SIGN #28
Hardware name: LENOVO 2306CTO/2306CTO, BIOS G2ET92WW (2.52 ) 02/22/2013
Workqueue: cifsiod cifs_echo_request [cifs]
task: ffff8801e1f416f0 ti: ffff880148744000 task.ti: ffff880148744000
RIP: 0010:[<ffffffff814dcc13>]  [<ffffffff814dcc13>] sock_sendmsg+0x93/0xd0
RSP: 0000:ffff880148745b00  EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff880148745b78 RCX: 0000000000000048
RDX: ffff880148745c90 RSI: ffff880181864a00 RDI: ffff880148745b78
RBP: ffff880148745c48 R08: 0000000000000048 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff880181864a00
R13: ffff880148745c90 R14: 0000000000000048 R15: 0000000000000048
FS:  0000000000000000(0000) GS:ffff88021e200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000088 CR3: 000000020c42c000 CR4: 00000000001407b0
Oops#2 Part7
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Stack:
 ffff880148745b30 ffffffff810c4af9 0000004848745b30 ffff880181864a00
 ffffffff81ffbc40 0000000000000000 ffff880148745c90 ffffffff810a5aab
 ffff880148745bc0 ffffffff81ffbc40 ffff880148745b60 ffffffff815a9fb8
Call Trace:
 [<ffffffff810c4af9>] ? finish_task_switch+0x49/0xe0
 [<ffffffff810a5aab>] ? lock_timer_base.isra.36+0x2b/0x50
 [<ffffffff815a9fb8>] ? _raw_spin_unlock_irqrestore+0x18/0x40
 [<ffffffff810a673f>] ? try_to_del_timer_sync+0x4f/0x70
 [<ffffffff815aa38f>] ? _raw_spin_unlock_bh+0x1f/0x30
 [<ffffffff814dcc87>] kernel_sendmsg+0x37/0x50
 [<ffffffffa081a0e0>] smb_send_kvec+0xd0/0x1d0 [cifs]
 [<ffffffffa081a263>] smb_send_rqst+0x83/0x1f0 [cifs]
 [<ffffffffa081ab6c>] cifs_call_async+0xec/0x1b0 [cifs]
 [<ffffffffa08245e0>] ? free_rsp_buf+0x40/0x40 [cifs]
Oops#2 Part6
 [<ffffffffa082606e>] SMB2_echo+0x8e/0xb0 [cifs]
 [<ffffffffa0808789>] cifs_echo_request+0x79/0xa0 [cifs]
 [<ffffffff810b45b3>] process_one_work+0x173/0x4a0
 [<ffffffff810b52a1>] worker_thread+0x121/0x3a0
 [<ffffffff810b5180>] ? manage_workers.isra.27+0x2b0/0x2b0
 [<ffffffff810bae00>] kthread+0xc0/0xd0
 [<ffffffff810bad40>] ? kthread_create_on_node+0x120/0x120
 [<ffffffff815b199c>] ret_from_fork+0x7c/0xb0
 [<ffffffff810bad40>] ? kthread_create_on_node+0x120/0x120
Code: 84 24 b8 00 00 00 4c 89 f1 4c 89 ea 4c 89 e6 48 89 df 4c 89 60 18 48 c7 40 28 00 00 00 00 4c 89 68 30 44 89 70 14 49 8b 44 24 28 <ff> 90 88 00 00 00 3d ef fd ff ff 74 10 48 8d 65 e0 5b 41 5c 41
 RIP  [<ffffffff814dcc13>] sock_sendmsg+0x93/0xd0
 RSP <ffff880148745b00>
CR2: 0000000000000088

The client was in the middle of trying to send a frame when the
server->ssocket pointer got zeroed out. In most places, that we access
that pointer, the srv_mutex is held. There's only one spot that I see
that the server->ssocket pointer gets set and the srv_mutex isn't held.
This patch corrects that.

The upstream bug report was here:

    https://bugzilla.kernel.org/show_bug.cgi?id=60557

Cc: <stable@vger.kernel.org>
Reported-by: Oleksii Shevchuk <alxchk@gmail.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <smfrench@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@koalo @maximus21