linux-kernel - Re: [PATCH] [USB] UAS: Achitecture; TMF; more

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <807184.43101.qm@web31810.mail.mud.yahoo.com>
Date:	Mon, 13 Dec 2010 23:58:38 -0800 (PST)
From:	Luben Tuikov <ltuikov@...oo.com>
To:	Sarah Sharp <sarah.a.sharp@...ux.intel.com>
Cc:	Greg KH <greg@...ah.com>, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Matthew Wilcox <willy@...ux.intel.com>
Subject: Re: [PATCH] [USB] UAS: Achitecture; TMF; more

--- On Fri, 11/19/10, Sarah Sharp <sarah.a.sharp@...ux.intel.com> wrote:
> 
> Why do you limit the number of streams this way?  USB
> 3.0 devices can
> support up to 65533 streams, so why place an arbitrary
> limit like this?
> Just use the MaxStreams field in the SuperSpeed Endpoint
> Companion
> descriptor in the call to usb_alloc_streams().  I
> don't see how limiting
> the number of streams you allocate to 128 when the original
> driver tried
> to allocate 256 is an improvement.

Is any of this relevant now? Just take a look at uasp.c--it's how it's done.

> Couldn't the renaming code could have made it into a
> separate
> patch?

That was the least of the changed I would've done. Refer to uasp.c--it's how it's done.

> It also makes it hard to wade through the
> changes.  You should
> have two patches at least: one to rename uas_dev_info, and
> one to rename
> qdepth.  There's places in the code where you've
> changed how you use
> qdepth, and it's hard to see because the rename and change
> are in one
> patch.

I think I've responded to this before to Alan and Greg, it basically goes some like "... one changed warranted another and so on and so forth and I ended up with this big thing..." Better off rip the whole thing off and do it right.

> Matthew and you can dicker over the name for uas_dev_info,

"dicker"? Sorry, I've not time for this. Refer to uasp.c--it's how it's done.

> but I really
> don't understand your reasoning for renaming it to
> uas_tport_info.  What
> type of "ports" are you talking about?  USB ports or
> some other type of
> ports?  To me, uas_dev_info made sense because it
> stored information
> about the particular UAS device, not the USB port it was
> attached to...

Refer to uasp.c, and the 18 points which I listed talking about the I_T_L_Q nexus...

> Tag ID or stream ID is the same, I suppose.  But
> stream IDs are always
> positive integers, so why should tag be signed?

I don't like to educate on mailing lists, especially here. (It always leads to this pointless argument, so I either ignore the question or patronize the questioner in order to get on with it.) Let alone trying to pass a patch that changes this... Just use uasp.c.

> I'm a bit confused by this comment, perhaps because you
> didn't explain
> what TMFs are in your comment or your commit message.
> 
> However, if you're telling the xHCI driver to allocate
> UAS_MAX_STREAMS,
> the UAS driver gets to put stream ID 1 to stream ID
> UAS_MAX_STREAMS in
> urb->stream_id.  If you submit an URB with
> urb->stream_id greater than
> UAS_MAX_STREAMS, the xHCI driver is going to reject that
> submission.
> 
> It looks like from the comment you expect to be able to use
> a stream ID
> as big as UAS_MAX_STREAMS+2+TMF_MAX_TAGS, is that what you
> were trying
> to do?

Check with uasp.c on how it's done.

   Luben

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/