Messages in this thread | | | Subject | Re: [patch 2.6.15-mm4] sem2mutex: infiniband, #2 | From | Roland Dreier <> | Date | Mon, 30 Jan 2006 15:17:09 -0800 |
| |
Thanks, I finally got around to looking at this, and I applied the infiniband/ulp/srp part of this to my tree. However, I noticed some fishy things in other parts of the patch. First of all, the entire user_mad.c diff you sent is:
> --- linux.orig/drivers/infiniband/core/user_mad.c > +++ linux/drivers/infiniband/core/user_mad.c > @@ -47,7 +47,7 @@ > #include <linux/kref.h> > > #include <asm/uaccess.h> > -#include <asm/semaphore.h> > +#include <linux/mutex.h> > > #include <rdma/ib_mad.h> > #include <rdma/ib_user_mad.h>
it seems that you're just getting lucky that struct semaphore is getting defined implicitly, since you didn't remove the actual semaphore use.
The semaphore usage in user_mad.c seems a little tricky to convert to a mutex, if I understand things correctly. The idea is that we create a special file that turns on the "IsSM" bit on the InfiniBand port when it's open (don't worry about what that means, really). We use a semaphore to prevent anyone else from opening the file and trying to be a SM until the first file is closed -- but of course the last reference to a file might be dropped by a different process than the one that opened it, so the up() might be called from a different process than the original down().
Second, in the mthca changes, you have
> --- linux.orig/drivers/infiniband/hw/mthca/mthca_dev.h > +++ linux/drivers/infiniband/hw/mthca/mthca_dev.h > @@ -44,7 +44,9 @@ > #include <linux/pci.h> > #include <linux/dma-mapping.h> > #include <linux/timer.h> > -#include <asm/semaphore.h> > +#include <linux/mutex.h>
but again not all semaphore usages have been removed.
For example, a few lines later in the same file, we have:
> @@ -111,7 +113,7 @@ enum { > struct mthca_cmd { > struct pci_pool *pool; > int use_events; > - struct semaphore hcr_sem; > + struct mutex hcr_mutex; > struct semaphore poll_sem; > struct semaphore event_sem;
so poll_sem and event_sem remain.
Again, these seem hard to convert to mutexes, especially event_sem. The device firmware allows some number of commands to be queued up (64 is a typical number) and event_sem is used to keep track of when a command slot is available. I guess I could rewrite things so that I use something like wait_event() to block until there is a free command slot, but I'm not convinced that's an improvement.
Thanks, Roland - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |