Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 20 Aug 2008 19:30:32 +0200
From:      Kris Kennaway <kris@FreeBSD.org>
To:        Hans Petter Selasky <hselasky@c2i.net>
Cc:        freebsd-usb@freebsd.org
Subject:   Re: usb4bsd patch review [was Re: ...]
Message-ID:  <48AC54B8.4040406@FreeBSD.org>
In-Reply-To: <200808201809.24620.hselasky@c2i.net>
References:  <20080818205914.GJ16977@elvis.mu.org> <200808192219.19246.hselasky@c2i.net> <48AB3D3A.4040303@FreeBSD.org> <200808201809.24620.hselasky@c2i.net>

next in thread | previous in thread | raw e-mail | index | archive | help
Hans Petter Selasky wrote:
> On Tuesday 19 August 2008, Kris Kennaway wrote:
>> Hans Petter Selasky wrote:
> 
>>>> This raises the question of why the kernel changes need to be committed
>>>> now, and what benefit they bring in the absence of a compatible
>>>> userland.  Shouldn't the commit happen after both kernel and userland
>>>> are ready (and reviewed)?
>>> I think that is correct.
>> OK, so does this mean we should expect to see a revised commit candidate
>> patch from you and/or alfred later once that is finished?
> 
> Alfred, I leave this up to you.
> 
>> You could look at what the sound code does, (I think it is specifically
>> the snd_driver module).  This implements auto-loading of the "right"
>> drivers by loading them and then unloading the ones that don't attach.
>> This still has overhead though, so users often will just compile in the
>> ones they know they have.
>>
>>>> Also is it safe to drop and reacquire the lock here?
>>> You have to drop the lock, else I get witness warnings.
>> Yes, and presumably rightly so.  It doesn't mean that dropping the lock
>> and later reacquiring it is safe though; it could introduce races.
> 
> Yes, I know.

...so what is the answer?

>> OK, this seems unrelated to USB then and is something you should discuss
>> with the sound maintainers.  It sounds to me like the ability to "run
>> without mutexes" is the real bug here, and "support" for that should be
>> removed completely instead of patching it up downstream.
> 
> Yes, and it would be nice if the sound maintainers would try out the new USB 
> stack, and propose how they want to solve the problems that exist in the 
> sound system.

This is backwards.  If you perceive problems in other subsystems you 
should take the lead on engaging with the relevant developers and 
resolving those problems to your mutual satisfaction, instead of 
inviting them to come to you.

>>> There are multiple issues:
>>>
>>> 1) If PAGE_SIZE is 16384 bytes, then the code simply fails.
>>> 2) Sometimes PAGE_SHIFT is already defined.
>>>
>>> #if PAGE_SIZE == 4096
>>> #define PAGE_SHIFT      12
>>> #elif PAGE_SIZE == 8192
>>> #define PAGE_SHIFT      13
>>> #else
>>> #error PAGE_SHIFT undefined!
>>> #endif
>> OK, but again, why?  If some architecture is not defining the same
>> symbols as the others then maybe that architecture should be fixed,
>> instead of working around the effect downstream.
> 
> Yes, but then there will be even more patches, and I've been told to reduce 
> the number of patches outside USB.

The way you minimize patches outside of USB is by submitting separate 
patches that fix upstream issues at their source, instead of adding 
workarounds like several of those that appeared in this diff.

>> P.S. There were a number of questions you didn't answer, can I assume
>> those will follow later?
> 
> Could you please repeat the questions you did not get an answer to?

I've repeated some of them already.  e.g. in this mail you still didn't 
answer the "is it safe to drop the locks" question which was asked 
twice.  Rather than me repeating them yet again, I'd suggest you go back 
and take another close read over my emails and your responses, and reply 
to the questions that are still not resolved.

Thanks,

Kris



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?48AC54B8.4040406>