input: xpad: Add queue for int-out urb

[kuikka]

Xpad driver had an issue where the same interrupt out urb was being used for every int-out transfer. Eventually this would lead to that urb being used while it was "busy" i.e. owned by the USB stack.

This patch fixes that by introducing an queue for int-out data transfers.

[adamdmoss]

What's the backtrace for the 'busy' complaint you're seeing from the USB stack? I've only seen one recurring USB stack error of this nature, but I believe that it was simply caused by the force-feedback code (xpad_play_effect) not acquiring the spinlock before using irq_out, so very non-invasive to fix. Here's the trace I'm talking about:

[ 4170.443037] URB ffff88020d047d80 submitted while active
..
[ 4170.443081] Call Trace:
[ 4170.443082]  <IRQ>  [<ffffffff8171a214>] dump_stack+0x45/0x56
[ 4170.443089]  [<ffffffff810676bd>] warn_slowpath_common+0x7d/0xa0
[ 4170.443091]  [<ffffffff8106772c>] warn_slowpath_fmt+0x4c/0x50
[ 4170.443093]  [<ffffffff815411db>] usb_submit_urb+0x45b/0x470
[ 4170.443096]  [<ffffffffa0ad54b1>] xpad_play_effect+0x271/0x2b0 [xpad_steamos]
[ 4170.443098]  [<ffffffffa0ab77c2>] ml_play_effects+0x102/0x600 [ff_memless]
[ 4170.443101]  [<ffffffffa0ab7e00>] ? ml_ff_playback+0x100/0x100 [ff_memless]
[ 4170.443103]  [<ffffffffa0ab7e3f>] ml_effect_timer+0x3f/0x200 [ff_memless]
[ 4170.443105]  [<ffffffff81074226>] call_timer_fn+0x36/0x100
[ 4170.443107]  [<ffffffffa0ab7e00>] ? ml_ff_playback+0x100/0x100 [ff_memless]
[ 4170.443109]  [<ffffffff810751bf>] run_timer_softirq+0x1ef/0x2f0
[ 4170.443111]  [<ffffffff8106caec>] __do_softirq+0xec/0x2c0
[ 4170.443113]  [<ffffffff8106d035>] irq_exit+0x105/0x110
[ 4170.443123]  [<ffffffff8172cfc5>] smp_apic_timer_interrupt+0x45/0x60
[ 4170.443125]  [<ffffffff8172b95d>] apic_timer_interrupt+0x6d/0x80

EDIT: still seeing this trace with the locking in place, so my minimal fix is indeed not enough. :I

[kuikka]

Hi Adam,

Yes, that is the path where this error mostly comes from, a game that uses the rumble a lot will cause this to happen more often (though that warning is only printed once).

Unfortunately merely taking a spin lock for the usb_submit_urb() call is not sufficient. From the moment the "struct urb *irq_out" is submitted to the USB subsystem using usb_submit_urb() to the moment the completion callback on the urb is called, it is owned by the USB subsystem.

Now, sometimes the force-feedback system generates out -events so fast that it tries to submit new one before the previous one has completed. That is where this error comes from.

Without this fix any ff request while urb is still pending will be ignored. Not a big deal usually but it can cause nastiness if, for example, a request to turn rumble off is missed and it stays on permanently.

• Juha

On Wed, Apr 29, 2015 at 7:40 AM, Adam D. Moss notifications@github.com wrote:

What's the backtrace for the 'busy' complaint you're seeing from the USB stack? I've only seen one recurring USB stack error of this nature, but I believe that it was simply caused by the force-feedback code (xpad_play_effect) not acquiring the spinlock before using irq_out, so very non-invasive to fix. Here's the trace I'm talking about:

[ 4170.443037] URB ffff88020d047d80 submitted while active .. [ 4170.443081] Call Trace: [ 4170.443082] [] dump_stack+0x45/0x56 [ 4170.443089] [] warn_slowpath_common+0x7d/0xa0 [ 4170.443091] [] warn_slowpath_fmt+0x4c/0x50 [ 4170.443093] [] usb_submit_urb+0x45b/0x470 [ 4170.443096] [] xpad_play_effect+0x271/0x2b0 [xpad_steamos] [ 4170.443098] [] ml_play_effects+0x102/0x600 [ff_memless] [ 4170.443101] [] ? ml_ff_playback+0x100/0x100 [ff_memless] [ 4170.443103] [] ml_effect_timer+0x3f/0x200 [ff_memless] [ 4170.443105] [] call_timer_fn+0x36/0x100 [ 4170.443107] [] ? ml_ff_playback+0x100/0x100 [ff_memless] [ 4170.443109] [] run_timer_softirq+0x1ef/0x2f0 [ 4170.443111] [] __do_softirq+0xec/0x2c0 [ 4170.443113] [] irq_exit+0x105/0x110 [ 4170.443123] [] smp_apic_timer_interrupt+0x45/0x60 [ 4170.443125] [] apic_timer_interrupt+0x6d/0x80

— Reply to this email directly or view it on GitHub https://github.com/ValveSoftware/steamos_kernel/pull/14#issuecomment-97451958 .

Duck tape is like the force, it has a light side and a dark side and it holds the universe together.