Re: usbmon: tentative implementation for 'bus0' interface

On Wed, 28 Feb 2007 10:03:11 +0100, Paolo Abeni <[email protected]> wrote:

> This patch create the bus0 to the usbmon interface. Bus0 provides events
> for all attached USB buses to any kind of reader (text, binary). 

Looks very good indeed, although I disagree with your coding style
as usual :-)

> A dummy mon_bus structure is created at usbmon initialization time. This
> is not attached to any real USB bus, but real mon readers can attach to it. 

> When any event is received from usbmon, the bus0 readers list is walked
> and attached readers are invoked. Then the event is processed as usual.

Very good idea.

> There are a couple of things that should be polished: 
> - mon_bus->u_bus can now be null and must be tested every time it's used.
> - at event reception two spinlock must be hold and released: one for
> bus0 and one for bus-n. Previously only one lock was needed. 

Well... if that's what it takes, so be it. I'll think a bit if we
can optimize bus0 out if there aren't any readers attached to it.

> +static void mon_submit_error(struct usb_bus *ubus, struct urb *urb, int error)
>  {
>  	struct mon_bus *mbus;
> +	mon_bus_submit_error(bus0, urb, error);
> +
> +	mbus = ubus->mon_bus;
> +	if (mbus == NULL)
> +		return;
> +	mon_bus_submit(mbus, urb);
> +}

This seems like an overzealous copy-paste.

> @@ -441,7 +441,7 @@ static void mon_bin_event(struct mon_rea
>  	/* We use the fact that usb_pipein() returns 0x80 */
>  	ep->epnum = usb_pipeendpoint(urb->pipe) | usb_pipein(urb->pipe);
>  	ep->devnum = usb_pipedevice(urb->pipe);
> -	ep->busnum = rp->r.m_bus->u_bus->busnum;
> +	ep->busnum = urb->dev->bus->busnum;
........
> @@ -18,6 +18,7 @@ struct mon_bus {
>  	struct list_head bus_link;
>  	spinlock_t lock;
>  	struct usb_bus *u_bus;
> +	int busnum;

These two ideas are not completely in agreement with each other.
I think I'll take the second one, a local copy of the bus number.

I'll regenerate the patch and resend it in the next few days.

Thanks a lot!
-- Pete

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel