3.9 merge window kernels

[Antti Palosaari]

On 02/23/2013 03:30 AM, Mauro Carvalho Chehab wrote:
> Em Sat, 23 Feb 2013 02:09:01 +0100
> poma <pomidorabelisima at gmail.com> escreveu:
>
>> On 02/23/13 01:09, Mauro Carvalho Chehab wrote:

>> This one stayed;
>> …
>> ------------[ cut here ]------------
>> WARNING: at lib/dma-debug.c:947 check_for_stack+0xa7/0xf0()
>> Hardware name: M720-US3
>> ehci-pci 0000:00:04.1: DMA-API: device driver maps memory fromstack
>> [addr=ffff8801283d7be1]
>> Modules linked in: dvb_usb_af9015(OF+)… dvb_usb_v2(OF)… dvb_core(OF)
>> wm8775(OF) tda9887(OF) tda8290(OF)… tuner(OF)… cx25840(OF)… bttv(OF+)…
>> rc_core(OF) btcx_risc(OF) ivtv(OF) snd_bt87x… videobuf_dma_sg(OF)
>> videobuf_core(OF)… cx2341x(OF) tveeprom(OF) v4l2_common(OF)…
>> videodev(OF)… video i2c_algo_bit… (irrelevant omitted)
>> Pid: 38, comm: kworker/2:1 Tainted: GF          O
>> 3.9.0-0.rc0.git5.1.fc18.x86_64 #1
>> Call Trace:
>> usb 4-2: pl2303 converter now attached to ttyUSB0
>>   [<ffffffff81069b5f>] warn_slowpath_common+0x7f/0xc0
>>   [<ffffffff81069c56>] warn_slowpath_fmt+0x46/0x50
>>   [<ffffffff81386467>] check_for_stack+0xa7/0xf0
>>   [<ffffffff81388228>] debug_dma_map_page+0x118/0x150
>>   [<ffffffff814fa5d9>] usb_hcd_map_urb_for_dma+0x569/0x630
>>   [<ffffffff811be091>] ? set_track+0x61/0x1b0
>>   [<ffffffff814fa945>] usb_hcd_submit_urb+0x2a5/0x900
>>   [<ffffffff810d814c>] ? lockdep_init_map+0xac/0x540
>>   [<ffffffff814fc07f>] usb_submit_urb+0xff/0x3d0
>>   [<ffffffff814fd042>] usb_start_wait_urb+0x82/0x1a0
>>   [<ffffffff814fbdae>] ? usb_alloc_urb+0x1e/0x50
>>   [<ffffffff814fd22c>] usb_bulk_msg+0xcc/0x180
>>   [<ffffffffa05be7e8>] dvb_usbv2_generic_rw+0xd8/0x270 [dvb_usb_v2]
>>   [<ffffffffa05cc5da>] af9015_ctrl_msg+0x1da/0x280 [dvb_usb_af9015]
>>   [<ffffffff810ae7aa>] ? cpuacct_charge+0xfa/0x210
>>   [<ffffffff810ae6c3>] ? cpuacct_charge+0x13/0x210
>>   [<ffffffffa05ccb4c>] af9015_identify_state+0x3c/0x90 [dvb_usb_af9015]
>>   [<ffffffffa05bcf18>] dvb_usbv2_init_work+0x68/0x11a0 [dvb_usb_v2]
>>   [<ffffffff8108e36f>] ? process_one_work+0x19f/0x690
>>   [<ffffffff8108e3db>] process_one_work+0x20b/0x690
>>   [<ffffffff8108e36f>] ? process_one_work+0x19f/0x690
>>   [<ffffffff8108ec10>] worker_thread+0x110/0x380
>>   [<ffffffff8108eb00>] ? rescuer_thread+0x260/0x260
>>   [<ffffffff8109569d>] kthread+0xed/0x100
>>   [<ffffffff810955b0>] ? flush_kthread_worker+0x190/0x190
>>   [<ffffffff8173882c>] ret_from_fork+0x7c/0xb0
>>   [<ffffffff810955b0>] ? flush_kthread_worker+0x190/0x190
>> ---[ end trace 4f4bc1c2abe300b5 ]---
>> --
>
> Ah, you're running it on a non-x86 hardware, right? On (modern) x86 machines,
> almost all memories can do DMA, including the area used by stack. That's
> not true on other archs like arm.
>
> I suspect that Antti never tried to run this driver outside x86.

Yep, that's true! I will generally test only x86 64bit and let the 
others test x86 32bit. Basically x86 is only which is tested by multiple 
users. MIPS and ARM are something that could be nice to test frequently, 
but I have no hw...

> Let me look into it...
>
> static int af9015_ctrl_msg(struct dvb_usb_device *d, struct req_t *req)
> {
> #define BUF_LEN 63
> #define REQ_HDR_LEN 8 /* send header size */
> #define ACK_HDR_LEN 2 /* rece header size */
>          struct af9015_state *state = d_to_priv(d);
>          int ret, wlen, rlen;
>          u8 buf[BUF_LEN];
> ...
>          ret = dvb_usbv2_generic_rw(d, buf, wlen, buf, rlen);
>
>
>
> int dvb_usbv2_generic_rw(struct dvb_usb_device *d, u8 *wbuf, u16 wlen, u8 *rbuf,
>                  u16 rlen)
> {
>          int ret, actual_length;
>
> ...
>          ret = usb_bulk_msg(d->udev, usb_sndbulkpipe(d->udev,
>                          d->props->generic_bulk_ctrl_endpoint), wbuf, wlen,
>                          &actual_length, 2000);
>
>
> That's the problem: "buf" is at stack, and it is passed directly to
> usb_bulk_msg(). This doesn't work on -arm (and sometimes may cause
> data to be lost even on x86).
>
> In order to fix it, the driver should be doing, instead, something like:
>
> static int af9015_ctrl_msg(struct dvb_usb_device *d, struct req_t *req)
> {
> ...
> 	u8 *buf = kmalloc(BUF_LEN, GFP_ATOMIC);
>
> 	...
>
> 	kfree(buf);
> ...
> }

I remember patch set about two years ago which removes usb_control_msg() 
stacked buffers from the all the /media/ USB drivers. At that time 
stacked use of usb_bulk_msg() was left as it was OK(?).


> You shouldn't blame the merge windows kernel here. This never worked
> on arm, even on older kernel versions ;)
>
> A more permanent fix would be to allocate a temporary 80 bytes buffer
> embedded with dvb_usb_v2 structs, and change dvb_usbv2_generic_rw to
> always use this temporary buffer for the control URB transfers.
>
> Antti,
>
> Could you please tale a look in it?

I am happy to alloc() all the buffers what are used by drivers I made, 
if that is the reason of these bugs (or even if it is the correct way 
despite that bug or not).

regards
Antti

-- 
http://palosaari.fi/