Page MenuHomeFreeBSD

sound: Prefer idle primary channels when allocating
Needs ReviewPublic

Authored by christos on Fri, Aug 21, 3:29 PM.
Tags
None
Referenced Files
F170752533: D59084.diff
Sun, Sep 6, 10:19 AM
Unknown Object (File)
Sat, Sep 5, 7:28 AM
Unknown Object (File)
Sat, Sep 5, 12:31 AM
Unknown Object (File)
Fri, Sep 4, 10:33 AM
Unknown Object (File)
Fri, Sep 4, 10:15 AM
Unknown Object (File)
Thu, Sep 3, 10:22 PM
Unknown Object (File)
Thu, Sep 3, 5:23 PM
Unknown Object (File)
Thu, Sep 3, 5:13 PM
Subscribers

Details

Reviewers
markj
emaste
Summary

dsp_chn_alloc() stopped at the first primary channel that was either
idle or already had vchans. Since the list is walked in order, the first
channel matched both conditions once it had been used, so every client
after the first was stacked onto it as a vchan and the remaining primary
channels were never allocated at all.

This is invisible on devices with a single primary channel, but not on
those which provide several. snd_emu10kx(4), for instance, registers
four primary channels for its front device, each able to run with its
own rate.

Look for an idle primary channel first, and only fall back to sharing
one that already has vchans when there is none left.

PR: 287687
Sponsored by: The FreeBSD Foundation
MFC after: 2 weeks

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 75947
Build 72830: arc lint + arc unit

Event Timeline

The patch looks like it does what the review description says it does, but apparently it doesn't actually fix the cited PR?

The patch looks like it does what the review description says it does, but apparently it doesn't actually fix the cited PR?

Yes. I cited the PR because it does fix an issue raised indirectly in the PR. Should I omit the "PR" line?

The patch looks like it does what the review description says it does, but apparently it doesn't actually fix the cited PR?

Yes. I cited the PR because it does fix an issue raised indirectly in the PR. Should I omit the "PR" line?

I'm not sure. It depends on the root cause of the bug. Maybe this patch helps but isn't sufficient, in which case the PR line should be included, or maybe the problem is unrelated, in which case it shouldn't be included.

The patch looks like it does what the review description says it does, but apparently it doesn't actually fix the cited PR?

Yes. I cited the PR because it does fix an issue raised indirectly in the PR. Should I omit the "PR" line?

I'm not sure. It depends on the root cause of the bug. Maybe this patch helps but isn't sufficient, in which case the PR line should be included, or maybe the problem is unrelated, in which case it shouldn't be included.

I thought the problem is partially related, but the PR submitter said it wasn't apparently.

I will remove the "PR" line from the commit message, are you okay with the patch itself so I can commit it?