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).
I'm afraid that I have to take back some of the superlatives that
I attached to this patch previously. A few issues still need a thought.
#1 - Greg made us to move the class device from /sys/class to /sys/devices.
But it does not square with this patch, because a mon_bus for bus zero has
no device. BTW, apparently I lost your "third try" for the class, so
the patch below may fuzz on your tree.
#2 - The setting and testing of the monitoring flag was completely
ignored by the patch. To fix it, I thought about creating a global flag
and having the hooks to test something like:
if (ubus->monitored || mon_bus0_monitored)
hook(ubus, urb);
This did not look too nice... The current situation is to walk buses,
paying extreme attention to SMP locking. Does not look nice either.
#3 - It may still need some factorization here and there, maybe some
kind of mon_bus_constructor(), mon_bus_destructor() or whatnot.
What I have now is below. It needs _very_ serious beating on SMP with
a full test of:
- rmmod ehci_hcd while client is listening
- rmmod uhci_hcd while client loops opening and closing
- several clients reading simultaneously
- All of the above, both for bus zero and nonzero.
I poked at it a bit at my laptop, but not enough. This is not ready to
go to Greg yet. About -mm, maybe, dunno. We'll see how it stacks up.
Also, needs docs.
-- Pete
diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c
index a1721a2..e883396 100644
--- a/drivers/usb/mon/mon_bin.c
+++ b/drivers/usb/mon/mon_bin.c
@@ -383,8 +383,10 @@ static inline char mon_bin_get_setup(unsigned char *setupb,
const struct urb *urb, char ev_type)
{
- if (urb->transfer_flags & URB_NO_SETUP_DMA_MAP)
+ if (urb->dev->bus->uses_dma &&
+ (urb->transfer_flags & URB_NO_SETUP_DMA_MAP)) {
return mon_dmapeek(setupb, urb->setup_dma, SETUP_LEN);
+ }
if (urb->setup_packet == NULL)
return 'Z';
@@ -396,7 +398,8 @@ static char mon_bin_get_data(const struct mon_reader_bin
*rp,
unsigned int offset, struct urb *urb, unsigned int length)
{
- if (urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP) {
+ if (urb->dev->bus->uses_dma &&
+ (urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
mon_dmapeek_vec(rp, offset, urb->transfer_dma, length);
return 0;
}
@@ -503,7 +506,7 @@ static void mon_bin_event(struct mon_reader_bin *rp, struct
urb *urb,
/* 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;
ep->id = (unsigned long) urb;
ep->ts_sec = ts.tv_sec;
ep->ts_usec = ts.tv_usec;
@@ -585,7 +588,7 @@ static void mon_bin_error(void *data, struct urb *urb, int
error)
/* 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;
ep->id = (unsigned long) urb;
ep->status = error;
@@ -600,7 +603,6 @@ static void mon_bin_error(void *data, struct urb *urb, int
error)
static int mon_bin_open(struct inode *inode, struct file *file)
{
struct mon_bus *mbus;
- struct usb_bus *ubus;
struct mon_reader_bin *rp;
size_t size;
int rc;
@@ -610,7 +612,7 @@ static int mon_bin_open(struct inode *inode, struct file
*file)
mutex_unlock(&mon_lock);
return -ENODEV;
}
- if ((ubus = mbus->u_bus) == NULL) {
+ if (mbus != &mon_bus0 && mbus->u_bus == NULL) {
printk(KERN_ERR TAG ": consistency error on open\n");
mutex_unlock(&mon_lock);
return -ENODEV;
@@ -1243,23 +1245,26 @@ static void mon_free_buff(struct mon_pgmap *map, int
npages)
free_page((unsigned long) map[n].ptr);
}
-int mon_bin_add(struct mon_bus *mbus, const struct usb_bus *ubus)
+int mon_bin_add(struct mon_bus *mbus, int busnum)
{
struct device *dev;
- unsigned minor = ubus->busnum;
+ unsigned minor = busnum;
if (minor >= MON_BIN_MAX_MINOR)
return 0;
dev = device_create(mon_bin_class, NULL,
MKDEV(MAJOR(mon_bin_dev0), minor), "usbmon%d", minor);
- return !IS_ERR(dev);
+ if (IS_ERR(dev))
+ return 0;
+
+ mbus->classdev = dev;
+ return 1;
}
void mon_bin_del(struct mon_bus *mbus)
{
- unsigned minor = mbus->u_bus->busnum;
- device_destroy(mon_bin_class, MKDEV(MAJOR(mon_bin_dev0), minor));
+ device_destroy(mon_bin_class, mbus->classdev->devt);
}
int __init mon_bin_init(void)
diff --git a/drivers/usb/mon/mon_main.c b/drivers/usb/mon/mon_main.c
index 35137de..2a96e2e 100644
--- a/drivers/usb/mon/mon_main.c
+++ b/drivers/usb/mon/mon_main.c
@@ -16,8 +16,6 @@
#include "usb_mon.h"
#include "../core/hcd.h"
-static void mon_submit(struct usb_bus *ubus, struct urb *urb);
-static void mon_complete(struct usb_bus *ubus, struct urb *urb);
static void mon_stop(struct mon_bus *mbus);
static void mon_dissolve(struct mon_bus *mbus, struct usb_bus *ubus);
static void mon_bus_drop(struct kref *r);
@@ -25,6 +23,7 @@ static void mon_bus_init(struct usb_bus *ubus);
DEFINE_MUTEX(mon_lock);
+struct mon_bus mon_bus0; /* Pseudo bus meaning "all buses" */
static LIST_HEAD(mon_buses); /* All buses we know: struct mon_bus */
/*
@@ -35,22 +34,19 @@ static LIST_HEAD(mon_buses); /* All buses we
know: struct mon_bus */
void mon_reader_add(struct mon_bus *mbus, struct mon_reader *r)
{
unsigned long flags;
- struct usb_bus *ubus;
+ struct list_head *p;
spin_lock_irqsave(&mbus->lock, flags);
if (mbus->nreaders == 0) {
- ubus = mbus->u_bus;
- if (ubus->monitored) {
- /*
- * Something is really broken, refuse to go on and
- * possibly corrupt ops pointers or worse.
- */
- printk(KERN_ERR TAG ": bus %d is already monitored\n",
- ubus->busnum);
- spin_unlock_irqrestore(&mbus->lock, flags);
- return;
+ if (mbus == &mon_bus0) {
+ list_for_each (p, &mon_buses) {
+ struct mon_bus *m1;
+ m1 = list_entry(p, struct mon_bus, bus_link);
+ m1->u_bus->monitored = 1;
+ }
+ } else {
+ mbus->u_bus->monitored = 1;
}
- ubus->monitored = 1;
}
mbus->nreaders++;
list_add_tail(&r->r_link, &mbus->r_list);
@@ -80,77 +76,79 @@ void mon_reader_del(struct mon_bus *mbus, struct mon_reader
*r)
/*
*/
-static void mon_submit(struct usb_bus *ubus, struct urb *urb)
+static void mon_bus_submit(struct mon_bus *mbus, struct urb *urb)
{
- struct mon_bus *mbus;
unsigned long flags;
struct list_head *pos;
struct mon_reader *r;
- mbus = ubus->mon_bus;
- if (mbus == NULL)
- goto out_unlocked;
-
spin_lock_irqsave(&mbus->lock, flags);
- if (mbus->nreaders == 0)
- goto out_locked;
-
mbus->cnt_events++;
list_for_each (pos, &mbus->r_list) {
r = list_entry(pos, struct mon_reader, r_link);
r->rnf_submit(r->r_data, urb);
}
-
spin_unlock_irqrestore(&mbus->lock, flags);
return;
+}
-out_locked:
- spin_unlock_irqrestore(&mbus->lock, flags);
-out_unlocked:
- return;
+static void mon_submit(struct usb_bus *ubus, struct urb *urb)
+{
+ struct mon_bus *mbus;
+
+ if ((mbus = ubus->mon_bus) != NULL)
+ mon_bus_submit(mbus, urb);
+ mon_bus_submit(&mon_bus0, urb);
}
/*
*/
-static void mon_submit_error(struct usb_bus *ubus, struct urb *urb, int error)
+static void mon_bus_submit_error(struct mon_bus *mbus, struct urb *urb, int
error)
{
- struct mon_bus *mbus;
unsigned long flags;
struct list_head *pos;
struct mon_reader *r;
- mbus = ubus->mon_bus;
- if (mbus == NULL)
- goto out_unlocked;
-
spin_lock_irqsave(&mbus->lock, flags);
- if (mbus->nreaders == 0)
- goto out_locked;
-
mbus->cnt_events++;
list_for_each (pos, &mbus->r_list) {
r = list_entry(pos, struct mon_reader, r_link);
r->rnf_error(r->r_data, urb, error);
}
-
spin_unlock_irqrestore(&mbus->lock, flags);
return;
+}
-out_locked:
- spin_unlock_irqrestore(&mbus->lock, flags);
-out_unlocked:
- return;
+static void mon_submit_error(struct usb_bus *ubus, struct urb *urb, int error)
+{
+ struct mon_bus *mbus;
+
+ if ((mbus = ubus->mon_bus) != NULL)
+ mon_bus_submit_error(mbus, urb, error);
+ mon_bus_submit_error(&mon_bus0, urb, error);
}
/*
*/
-static void mon_complete(struct usb_bus *ubus, struct urb *urb)
+static void mon_bus_complete(struct mon_bus *mbus, struct urb *urb)
{
- struct mon_bus *mbus;
unsigned long flags;
struct list_head *pos;
struct mon_reader *r;
+ spin_lock_irqsave(&mbus->lock, flags);
+ mbus->cnt_events++;
+ list_for_each (pos, &mbus->r_list) {
+ r = list_entry(pos, struct mon_reader, r_link);
+ r->rnf_complete(r->r_data, urb);
+ }
+ spin_unlock_irqrestore(&mbus->lock, flags);
+}
+
+static void mon_complete(struct usb_bus *ubus, struct urb *urb)
+{
+ struct mon_bus *mbus;
+
mbus = ubus->mon_bus;
if (mbus == NULL) {
/*
@@ -162,13 +160,8 @@ static void mon_complete(struct usb_bus *ubus, struct urb
*urb)
return;
}
- spin_lock_irqsave(&mbus->lock, flags);
- mbus->cnt_events++;
- list_for_each (pos, &mbus->r_list) {
- r = list_entry(pos, struct mon_reader, r_link);
- r->rnf_complete(r->r_data, urb);
- }
- spin_unlock_irqrestore(&mbus->lock, flags);
+ mon_bus_complete(mbus, urb);
+ mon_bus_complete(&mon_bus0, urb);
}
/* int (*unlink_urb) (struct urb *urb, int status); */
@@ -179,14 +172,26 @@ static void mon_complete(struct usb_bus *ubus, struct urb
*urb)
static void mon_stop(struct mon_bus *mbus)
{
struct usb_bus *ubus = mbus->u_bus;
+ struct list_head *p;
- /*
- * A stop can be called for a dissolved mon_bus in case of
- * a reader staying across an rmmod foo_hcd.
- */
- if (ubus != NULL) {
- ubus->monitored = 0;
- mb();
+ if (mbus == &mon_bus0) {
+ list_for_each (p, &mon_buses) {
+ mbus = list_entry(p, struct mon_bus, bus_link);
+ /*
+ * We do not change nreaders here, so rely on mon_lock.
+ */
+ if (mbus->nreaders == 0 && (ubus = mbus->u_bus) != NULL)
+ ubus->monitored = 0;
+ }
+ } else {
+ /*
+ * A stop can be called for a dissolved mon_bus in case of
+ * a reader staying across an rmmod foo_hcd, so test ->u_bus.
+ */
+ if (mon_bus0.nreaders == 0 && (ubus = mbus->u_bus) != NULL) {
+ ubus->monitored = 0;
+ mb();
+ }
}
}
@@ -199,6 +204,10 @@ static void mon_stop(struct mon_bus *mbus)
static void mon_bus_add(struct usb_bus *ubus)
{
mon_bus_init(ubus);
+ mutex_lock(&mon_lock);
+ if (mon_bus0.nreaders != 0)
+ ubus->monitored = 1;
+ mutex_unlock(&mon_lock);
}
/*
@@ -252,12 +261,7 @@ static struct usb_mon_operations mon_ops_0 = {
static void mon_dissolve(struct mon_bus *mbus, struct usb_bus *ubus)
{
- /*
- * Never happens, but...
- */
if (ubus->monitored) {
- printk(KERN_ERR TAG ": bus %d is dissolved while monitored\n",
- ubus->busnum);
ubus->monitored = 0;
mb();
}
@@ -265,6 +269,8 @@ static void mon_dissolve(struct mon_bus *mbus, struct
usb_bus *ubus)
ubus->mon_bus = NULL;
mbus->u_bus = NULL;
mb();
+
+ /* We want synchronize_irq() here, but that needs an argument. */
}
/*
@@ -297,10 +303,9 @@ static void mon_bus_init(struct usb_bus *ubus)
*/
mbus->u_bus = ubus;
ubus->mon_bus = mbus;
- mbus->uses_dma = ubus->uses_dma;
- mbus->text_inited = mon_text_add(mbus, ubus);
- mbus->bin_inited = mon_bin_add(mbus, ubus);
+ mbus->text_inited = mon_text_add(mbus, ubus->busnum);
+ mbus->bin_inited = mon_bin_add(mbus, ubus->busnum);
mutex_lock(&mon_lock);
list_add_tail(&mbus->bus_link, &mon_buses);
@@ -311,6 +316,18 @@ err_alloc:
return;
}
+static void mon_bus0_init(void)
+{
+ struct mon_bus *mbus = &mon_bus0;
+
+ kref_init(&mbus->ref);
+ spin_lock_init(&mbus->lock);
+ INIT_LIST_HEAD(&mbus->r_list);
+
+ mbus->text_inited = mon_text_add(mbus, 0);
+ mbus->bin_inited = mon_bin_add(mbus, 0);
+}
+
/*
* Search a USB bus by number. Notice that USB bus numbers start from one,
* which we may later use to identify "all" with zero.
@@ -324,6 +341,9 @@ struct mon_bus *mon_bus_lookup(unsigned int num)
struct list_head *p;
struct mon_bus *mbus;
+ if (num == 0) {
+ return &mon_bus0;
+ }
list_for_each (p, &mon_buses) {
mbus = list_entry(p, struct mon_bus, bus_link);
if (mbus->u_bus->busnum == num) {
@@ -343,6 +363,8 @@ static int __init mon_init(void)
if ((rc = mon_bin_init()) != 0)
goto err_bin;
+ mon_bus0_init();
+
if (usb_mon_register(&mon_ops_0) != 0) {
printk(KERN_NOTICE TAG ": unable to register with the core\n");
rc = -ENODEV;
@@ -376,6 +398,7 @@ static void __exit mon_exit(void)
usb_mon_deregister();
mutex_lock(&mon_lock);
+
while (!list_empty(&mon_buses)) {
p = mon_buses.next;
mbus = list_entry(p, struct mon_bus, bus_link);
@@ -401,6 +424,13 @@ static void __exit mon_exit(void)
mon_dissolve(mbus, mbus->u_bus);
kref_put(&mbus->ref, mon_bus_drop);
}
+
+ mbus = &mon_bus0;
+ if (mbus->text_inited)
+ mon_text_del(mbus);
+ if (mbus->bin_inited)
+ mon_bin_del(mbus);
+
mutex_unlock(&mon_lock);
mon_text_exit();
diff --git a/drivers/usb/mon/mon_text.c b/drivers/usb/mon/mon_text.c
index bbf5439..ec0cc51 100644
--- a/drivers/usb/mon/mon_text.c
+++ b/drivers/usb/mon/mon_text.c
@@ -124,8 +124,10 @@ static inline char mon_text_get_setup(struct
mon_event_text *ep,
if (!usb_pipecontrol(urb->pipe) || ev_type != 'S')
return '-';
- if (mbus->uses_dma && (urb->transfer_flags & URB_NO_SETUP_DMA_MAP))
+ if (urb->dev->bus->uses_dma &&
+ (urb->transfer_flags & URB_NO_SETUP_DMA_MAP)) {
return mon_dmapeek(ep->setup, urb->setup_dma, SETUP_MAX);
+ }
if (urb->setup_packet == NULL)
return 'Z'; /* '0' would be not as pretty. */
@@ -160,8 +162,10 @@ static inline char mon_text_get_data(struct mon_event_text
*ep, struct urb *urb,
* contain non-NULL garbage in case the upper level promised to
* set DMA for the HCD.
*/
- if (mbus->uses_dma && (urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP))
+ if (urb->dev->bus->uses_dma &&
+ (urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
return mon_dmapeek(ep->data, urb->transfer_dma, len);
+ }
if (urb->transfer_buffer == NULL)
return 'Z'; /* '0' would be not as pretty. */
@@ -201,7 +205,7 @@ static void mon_text_event(struct mon_reader_text *rp,
struct urb *urb,
ep->type = ev_type;
ep->pipe = urb->pipe;
ep->id = (unsigned long) urb;
- ep->busnum = rp->r.m_bus->u_bus->busnum;
+ ep->busnum = urb->dev->bus->busnum;
ep->tstamp = stamp;
ep->length = (ev_type == 'S') ?
urb->transfer_buffer_length : urb->actual_length;
@@ -305,13 +309,11 @@ static struct mon_event_text *mon_text_fetch(struct
mon_reader_text *rp,
static int mon_text_open(struct inode *inode, struct file *file)
{
struct mon_bus *mbus;
- struct usb_bus *ubus;
struct mon_reader_text *rp;
int rc;
mutex_lock(&mon_lock);
mbus = inode->i_private;
- ubus = mbus->u_bus;
rp = kzalloc(sizeof(struct mon_reader_text), GFP_KERNEL);
if (rp == NULL) {
@@ -335,8 +337,7 @@ static int mon_text_open(struct inode *inode, struct file
*file)
rp->r.rnf_error = mon_text_error;
rp->r.rnf_complete = mon_text_complete;
- snprintf(rp->slab_name, SLAB_NAME_SZ, "mon%dt_%lx", ubus->busnum,
- (long)rp);
+ snprintf(rp->slab_name, SLAB_NAME_SZ, "mon_text_%p", rp);
rp->e_slab = kmem_cache_create(rp->slab_name,
sizeof(struct mon_event_text), sizeof(long), 0,
mon_text_ctor, NULL);
@@ -654,14 +655,14 @@ static const struct file_operations mon_fops_text_u = {
.release = mon_text_release,
};
-int mon_text_add(struct mon_bus *mbus, const struct usb_bus *ubus)
+int mon_text_add(struct mon_bus *mbus, int busnum)
{
struct dentry *d;
enum { NAMESZ = 10 };
char name[NAMESZ];
int rc;
- rc = snprintf(name, NAMESZ, "%dt", ubus->busnum);
+ rc = snprintf(name, NAMESZ, "%dt", busnum);
if (rc <= 0 || rc >= NAMESZ)
goto err_print_t;
d = debugfs_create_file(name, 0600, mon_dir, mbus, &mon_fops_text_t);
@@ -669,7 +670,7 @@ int mon_text_add(struct mon_bus *mbus, const struct usb_bus
*ubus)
goto err_create_t;
mbus->dent_t = d;
- rc = snprintf(name, NAMESZ, "%du", ubus->busnum);
+ rc = snprintf(name, NAMESZ, "%du", busnum);
if (rc <= 0 || rc >= NAMESZ)
goto err_print_u;
d = debugfs_create_file(name, 0600, mon_dir, mbus, &mon_fops_text_u);
@@ -677,7 +678,7 @@ int mon_text_add(struct mon_bus *mbus, const struct usb_bus
*ubus)
goto err_create_u;
mbus->dent_u = d;
- rc = snprintf(name, NAMESZ, "%ds", ubus->busnum);
+ rc = snprintf(name, NAMESZ, "%ds", busnum);
if (rc <= 0 || rc >= NAMESZ)
goto err_print_s;
d = debugfs_create_file(name, 0600, mon_dir, mbus, &mon_fops_stat);
diff --git a/drivers/usb/mon/usb_mon.h b/drivers/usb/mon/usb_mon.h
index 5ca33ad..ae1810b 100644
--- a/drivers/usb/mon/usb_mon.h
+++ b/drivers/usb/mon/usb_mon.h
@@ -24,7 +24,7 @@ struct mon_bus {
struct dentry *dent_s; /* Debugging file */
struct dentry *dent_t; /* Text interface file */
struct dentry *dent_u; /* Second text interface file */
- int uses_dma;
+ struct device *classdev; /* Device in usbmon class */
/* Ref */
int nreaders; /* Under mon_lock AND mbus->lock */
@@ -54,9 +54,9 @@ void mon_reader_del(struct mon_bus *mbus, struct mon_reader
*r);
struct mon_bus *mon_bus_lookup(unsigned int num);
-int /*bool*/ mon_text_add(struct mon_bus *mbus, const struct usb_bus *ubus);
+int /*bool*/ mon_text_add(struct mon_bus *mbus, int busnum);
void mon_text_del(struct mon_bus *mbus);
-int /*bool*/ mon_bin_add(struct mon_bus *mbus, const struct usb_bus *ubus);
+int /*bool*/ mon_bin_add(struct mon_bus *mbus, int busnum);
void mon_bin_del(struct mon_bus *mbus);
int __init mon_text_init(void);
@@ -84,4 +84,6 @@ extern struct mutex mon_lock;
extern const struct file_operations mon_fops_stat;
+extern struct mon_bus mon_bus0; /* Only for redundant checks */
+
#endif /* __USB_MON_H */
-------------------------------------------------------------------------
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
