Date: Wed, 20 Aug 2008 15:53:07 -0600 (MDT) From: "M. Warner Losh" <imp@bsdimp.com> To: alfred@freebsd.org Cc: freebsd-usb@freebsd.org Subject: Re: usb4bsd patch review Message-ID: <20080820.155307.1102526075.imp@bsdimp.com> In-Reply-To: <20080820211644.GD16977@elvis.mu.org> References: <200808201809.24620.hselasky@c2i.net> <48AC54B8.4040406@FreeBSD.org> <20080820211644.GD16977@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <20080820211644.GD16977@elvis.mu.org>
Alfred Perlstein <alfred@freebsd.org> writes:
: [[ Reply to Hans at end of this ]]
: [[ Reply to Kris inline ]]
:
: * Kris Kennaway <kris@FreeBSD.org> [080820 10:30] wrote:
: > Hans Petter Selasky wrote:
: > >On Tuesday 19 August 2008, Kris Kennaway wrote:
: > >>
: > >>>>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.
:
: I think there has been a huge amount of effort just on the usb
: side, I think expecting Hans to come up with a patch that
: is "fix all freebsd issues" is a bit too much. I think we can
: proceed in an organic matter and clean up these small nits as
: time goes on, just like we do with almost every other problem...
We should at least tag these, understand them and make sure they don't
drop on the floor rather than just dismissing things out of hand... I
have a few action items here because things have dropped on the floor
in the past...
: > >>>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.
:
: It's a two line ifdef, I dare say we're being a bit pedantic here.
:
: How much of a showstopper is this in reality? How _wrong_ is it
: compared to how _wrong_ the old usb code is? Are you taking into
: account that these nits can be fixed easily compared to the extra
: effort we will have to go through to submit all these stuff just for
: a two line delta?
Why not just use PAGE_SHIFT w/o defining it again? It looks like it
is defined everywhere these days... It is part of the defined
interface that the MD code is supposed to provide for everybody...
Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080820.155307.1102526075.imp>