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
#14 (comment)
.

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