Jeff The patch and description provided no information about
Jeff whether or not it would be better to blacklist 8132
Jeff globally, as we have already done with the 8131.
8131 and 8132 are quite different: AMD 8131 simply did not implement
MSI at all, so any attempt to use MSI by a
David I was waiting for a resolution in the fact that the copy
David Chas posted only had one of the transformations whereas as
David you pointed out all of the ones in your original patch were
David necessary.
Actually I think there was some confusion because akpm forwarded a
' (at offset 0x2130) and 'he_service_tbrq'
Fix this by changing the __init functions to __devinit.
Signed-off-by: Roland Dreier [EMAIL PROTECTED]
---
Dave, this was acked by Chas (and he even requested it go into 2.6.18)
but it seems to have gotten dropped somewhere -- it's not in Linus's
tree
-specific data
IB/uverbs: Pass userspace data to modify_srq and modify_qp methods
IB/ipath: Performance improvements via mmap of queues
Roland Dreier:
IB/uverbs: Use idr_read_cq() where appropriate
IB/uverbs: Fix lockdep warning when QP is created with 2 CQs
IB: Whitespace
WARNING: drivers/atm/he.o - Section mismatch: reference to .init.text: from
.text between 'he_start' (at offset 0x218a) and 'he_service_tbrq'
-static int __init
+static int __devinit
he_init_group(struct he_dev *he_dev, int group)
There are a ton of other __init functions
author chas williams - CONTRACTOR [EMAIL PROTECTED] Sat, 16 Sep 2006
15:44:55 -0400
Not really a big deal -- but in general it's probably better to try
and preserve authorship information.
All you need is to add a
From: Roland Dreier [EMAIL PROTECTED]
line at the beginning of the body
+struct tsi108_prv_data {
+volatile u32 regs; /* Base of normal regs */
+volatile u32 phyregs; /* Base of register bank used for PHY access */
Why volatile? This looks really wrong here.
+data-regs = (u32)ioremap(einfo-regs, 0x400); /*FIX ME */
+data-phyregs =
' (at offset 0x2130) and 'he_service_tbrq'
Fix this by changing the __init functions to __devinit.
Signed-off-by: Roland Dreier [EMAIL PROTECTED]
---
diff --git a/drivers/atm/he.c b/drivers/atm/he.c
index d369130..9e0383d 100644
--- a/drivers/atm/he.c
+++ b/drivers/atm/he.c
@@ -454,7 +454,7
Here is a series of patches that adds iWARP (RDMA over IP) support to
the InfiniBand support already in the kernel. Since the iWARP RDMA
model is quite close to the InfiniBand model, the changes are not that
large. The biggest difference is in how connections are established,
since iWARP
of RDMA_NODE_RNIC, and update everything for this.
Signed-off-by: Tom Tucker [EMAIL PROTECTED]
Signed-off-by: Steve Wise [EMAIL PROTECTED]
Signed-off-by: Roland Dreier [EMAIL PROTECTED]
---
drivers/infiniband/core/Makefile |4
drivers/infiniband/core/addr.c | 18
Here's the compressed patch adding the amso1100 driver. You can also
find this in my git tree at
git://git.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.git
in the for-2.6.19 branch.
0003-RDMA-amso1100-Add-driver-for-Ammasso-1100-RNIC.txt.bz2
Description: application/bzip
, and are
available also for transport dependent ULPs.
Signed-off-by: Tom Tucker [EMAIL PROTECTED]
Signed-off-by: Steve Wise [EMAIL PROTECTED]
Signed-off-by: Roland Dreier [EMAIL PROTECTED]
---
drivers/infiniband/core/iwcm.c | 1019
drivers/infiniband/core
I'm looking at updating IP-over-InfiniBand to use NAPI, and due to the
way IB works, the driver is going to be susceptible to the rotting
packet problem. It seems I'm going to have to call netif_rx_reschedule().
However I'm confused about a couple of things, and there are only two
uses of
Stephen The undo should really be handled by the caller, not in
Stephen netif_rx_reschedule. The existing interface assumes you
Stephen have already deducted N from your quota and so it needs
Stephen to be put back.
makes sense -- especially since the caller probably also
Roland makes sense -- especially since the caller probably also
Roland deducted N from *budget, and netif_rx_reschedule() doesn't
Roland touch that.
Actually, why does undoing the change to quota make sense? Presumably
I passed N packets to netif_receive_skb() -- why shouldn't I
While merging this, I uninlined rdma_node_get_transport, since I don't
think there's any reason to make it inline:
add/remove: 1/0 grow/shrink: 7/16 up/down: 65/-146 (-81)
function old new delta
rdma_node_get_transport- 33
I'm finally getting around to merging this up, and:
--- /dev/null
+++ b/drivers/infiniband/hw/amso1100/README
@@ -0,0 +1,11 @@
+This is the OpenFabrics provider driver for the
+AMSO1100 1Gb RNIC adapter.
+
+This adapter is available in limited quantities
+for development
David Don't touch interrupts until both RX and TX queue work is
David fullydepleted. You seem to have this notion that RX and TX
David interrupts are seperate. They aren't, even if your device
David can generate those events individually. Whatever interrupt
David you get,
Michael Cold you oplease consider IB/mthca: recover from device
Michael errors as well?
Yes, I will. There's still plenty of time before 2.6.19 opens up.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at
really want
Michael S. Tsirkin:
IB/mthca: Don't use privileged UAR for kernel access
IB/ipoib: Fix flush/start xmit race (from code review)
Roland Dreier:
IB/uverbs: Use idr_read_cq() where appropriate
IB/uverbs: Fix lockdep
Also this won't allow
struct pci_driver {
...
#ifdef CONFIG_PM
int (*suspend)(...);
int (*resume)(...);
#endif
...
};
which is good for a) space savings in CONFIG_PM=n case, and
b) making drivers care
David For the cases where it is no actually necessary, the code
David generation cost on RISC cpus is very high. Byte loads and
David stores will be used to access _every_ member of such
David structures on RISC machines because the compiler cannot
David guarentee the
Steve Here is the iWARP Core Support patchset merged to your
Steve latest for-2.6.19 branch. It has gone through 3 reviews on
Steve lklm and netdev a while ago, and I think its ready to be
Steve pulled in.
I agree. I'll read this over and queue it for 2.6.19 unless someone
David Why is this a relevant analogy? Well, you have physical
David hard-disks in your computer today, but at some point that
David device becomes largely superfluous. It makes more sense to
David have just a cpu with a 10-gigabit ethernet interface
David incorporated onto
skbuff.h has an #ifndef CONFIG_HAVE_ARCH_DEV_ALLOC_SKB to allow
architectures to reimplement __dev_alloc_skb. It's not set on any
architecture and now that we have an architecture-overrideable
NET_SKB_PAD there is not point at all to have one either.
I missed this when hch first posted it,
The commit b3a6251915df9e3d80d4a0d32bd8d24223906688, [PKT_SCHED] HTB:
initialize upper bound properly, broke my compile because the new
line it added was missing a final semicolon.
Cc: Stephen Hemminger [EMAIL PROTECTED]
Cc: Jamal Hadi Salim [EMAIL PROTECTED]
Signed-off-by: Roland Dreier [EMAIL
Never mind -- looks like davej sent the fix directly to Linus already.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Andrew Sure. Although I am a little surprised to be be receiving
Andrew them while Roland is in
Andrew taking-time-off-but-not-really-doing-so mode.
Michael Well, I don't know what's up either, but Roland acked
Michael patches explicitly so I figured that's what he wants,
Then why in the world would we put up explicit web pages that
say TOE is bad, here's a list of reasons why if we had any
intention of ever adding support for these kinds of devices?
I think there's a little bit of leap of logic there. Everyone agrees
that winmodems are bad and yet there's
Andi Perhaps a good start of that discussion David asked for
Andi would be if you could give us an overview of the differences
Andi and how you avoid the TOE problems.
Well, here's a quick overview, leaving out some of the details. The
difference between TOE and iWARP/RDMA is really
Roland stated that it has never been the case that we have
rejected adding support for a certain class of devices on the
kinds of merits being discussed in this thread. And I'm saying
that TOE is such a case where we have emphatically done so.
Well, in the past it's seemed more like
You snipped my question about what specifically you wanted reverted,
so I'm going to assume that after cooling down and understanding the
situation, you're OK with everything that's in Linus's tree...
The integration of iWARP with the Linux networking, while much better
than TOE, is still
static void i2c_wait_for_writes(struct ipath_devdata *dd)
{
+ mb();
(void)ipath_read_kreg32(dd, dd-ipath_kregs-kr_scratch);
}
That's a bit weird. I wouldn't have expected the compiler to muck around
with a readl().
I never liked this patch. The last time it came up
David Roland, there is no way in the world we would have let
David support for RDMA into the kernel tree had we seen and
David reviewed it on netdev. I've discussed this with Andrew
David Morton, and we'd like you to please revert all of the RDMA
David code from Linus's tree
Dave, you're going to have to be more specific. What do you mean by
RDMA? The whole drivers/infiniband infrastructure, which handles RDMA
over IB, has been upstream for a year and a half, and was in fact
originally merged by you, so I'm guessing that's not what you mean.
NET_DMA
15) I wonder if SA_SHIRQ needs to be set, for MSI?
Why would it? MSI vectors are always unique, aren't they?
- R.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Andrew Is it usual to prohibit IRQ sharing with msix?
With current code at least, MSI/MSI-X interrupts can never be shared.
- R.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at
I think this code needs to be refactored so that it can share with the
ehca InfiniBand driver (which should be merged upstream soon). For
example, you have ehea_hcall_7arg_7ret() and the ehca driver has an
identical ehca_hcall_7arg_7ret().
Also:
+++ kernel/drivers/net/ehea/ehea_hcall.h
I just realized it could be the spam filters. You have some comments
with three 'X's in a row which might be getting it blocked. Is that
possible?
- R.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at
David As long as you never take priv-lock while -xmit_lock is
David held your patch should be OK.
Duh ... unfortunately priv-lock is taken from interrupt context so
that patch isn't safe. A correct fix would be the following, which
leads to a trivial conversion to using netif_tx_lock().
IPOIB is going to BUG() with this change. Because now, in their
multicast code, you're going to local_bh_disable() via
netif_tx_unlock() with hw IRQs disabled which is illegal.
It shows a bug here in the locking of the IPOIB driver.
Sorry, I haven't followed this thread closely. Can
Roland Sorry, I haven't followed this thread closely. Can you
Roland expand on what the bug in ipoib's multicast locking is?
You know, looking at the ipoib code, I can't even recreate why
xmit_lock is taken in the set_multicast_list method anyway, or how it
works at all -- it seems
Roland You know, looking at the ipoib code, I can't even recreate
Roland why xmit_lock is taken in the set_multicast_list method
Roland anyway, or how it works at all -- it seems
Roland set_multicast_list will always be called with xmit_lock
Roland already held. What the heck
David Isn't the IPOIB driver LLTX and therefore the upper layers
David are not taking the xmit_lock?
Yeah, but I think that should be OK. If I'm remembering the intention
of the code correctly, the reason xmit_lock is being taken there is
just to protect dev-mc_list -- and this is needed
+EXPORT_SYMBOL(copy_addr);
I think if you want to export this, it needs a less generic name
(something with an rdma_ prefix probably). Otherwise it's going to
collide someday...
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More
Steve The function is needed by the iwcm module, so that's why we
Steve exported it. I could change the name to rdma_copy_addr(),
Steve or make the function a static inline in a header file since
Steve its kinda small anyway...
It looks too big to inline to me, and I don't think
Can you reorder things so these changes go last? Otherwise after this
patch we're left with a kernel tree that has a Makefile that refers to
sources that don't exist yet. It's not really a practical issue but
it is neater to do that way.
(It's easy to do in stgit -- just pop all the patches and
Herbert However, lockless drivers do not take the xmit_lock so
Herbert this method is ineffective. Such drivers need to do
Herbert their own checking inside whatever locks that they do
Herbert take. For example, tg3 could get around this by checking
Herbert whether the queue
Amit We'll implement the minor changes rightaway and post an
Amit update. We are also thinking about what's the best way to
Amit incorporate the data structure changes you have
Amit suggested. Will post a reply on that too soon.
By the way, my suggestion to the original posting
+static int __devinit
+netxen_nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+#if defined(CONFIG_PCI_MSI)
+adapter-flags |= NETXEN_NIC_MSI_ENABLED;
+if (pci_enable_msi(pdev)) {
+adapter-flags = ~NETXEN_NIC_MSI_ENABLED;
+
Still some suspicious uses of volatile here.
For example:
+struct myri10ge_priv {
...
+ volatile u8 __iomem *sram;
as far as I can see this is always used with proper __iomem accessors,
often with casts to strip the volatile anyway. So why is volatile needed?
I would suggest an audit
A few quick obvious comments:
+#ifdef MYRI10GE_MCP
+typedef signed char int8_t;
+typedef signed shortint16_t;
+typedef signed int int32_t;
+typedef signed long longint64_t;
+typedef unsigned char uint8_t;
+typedef unsigned short uint16_t;
+typedef struct {
+mcp_kreq_ether_recv_t __iomem *lanai; /* lanai ptr for recv ring */
+volatile uint8_t __iomem *wc_fifo; /* w/c rx dma addr fifo address
*/
+mcp_kreq_ether_recv_t *shadow; /* host shadow of recv ring */
+struct myri10ge_rx_buffer_state *info;
Keir Where should we get our entropy from in a VM environment?
Keir Leaving the pool empty can cause processes to hang.
You could have something like a virtual HW RNG driver (with a frontend
and backend), which steals from the dom0 /dev/random pool.
- R.
-
To unsubscribe from this list:
Herbert I don't think this is enough though since this timer is
Herbert one of those self-rescheduling timers. You need to
Herbert provide some sort of a flag for it to stop scheduling
Herbert itself and synchronise it properly.
I'm probably missing an obvious race but it seems
+case 0x1030 ... 0x1034:
Do we use the gcc case range extension in the kernel? (This is an
honest question -- I don't think I've seen it used anywhere, and I'd
be curious to know what the taste arbiters think of it)
-
To unsubscribe from this list: send the line unsubscribe netdev in
the
Just a couple of very general comments:
- The kernel style is to avoid // comments like
+//reset abyte_cnt and dummy_byte_cnt
please replace all of this with comments like
/* single-line comment */
or
/*
* multi-line comments
* look like
Michael BTW, is there some way to see your git tree e.g. with gitweb?
Sure,
http://www.kernel.org/git/?p=linux/kernel/git/roland/infiniband.git;a=summary
- R.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo
+struct workqueue_struct *rdma_wq;
+EXPORT_SYMBOL(rdma_wq);
Sean, I don't think I saw an answer when I asked you this before. Why
is ib_addr exporting a workqueue? Is there some sort of ordering
constraint that is forcing other modules to go through the same
workqueue for things?
This
I added this patch to the rdma_cm branch in my git tree. When I was
doing that, I noticed that it builds rdma_ucm.ko unconditionally. It
seems that we want this to depend on CONFIG_INFINIBAND_USER_ACCESS,
since that controls ib_uverbs.ko and ib_ucm.ko.
To do this I rejiggered the Kconfig and
Sean It's dropping the reference on cma_dev, as opposed to
Sean id_priv.
Duh, sorry.
- R.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
The ib_addr module depends on CONFIG_INET, because it uses symbols
like arp_tbl, which are only exported if INET is enabled.
I fixed this up by creating a new (non-user-visible) config symbol to
control when ib_addr is built -- I put the following diff on top of
your patch in my tree:
diff --git
.
Thanks, and sorry about the screw-up.
---
Get rid of the last place in IPoIB where we clear
neigh-neighbour-ops-destructor. This is broken now that the
destructor member has moved to neigh_params.
Signed-off-by: Roland Dreier [EMAIL PROTECTED]
diff --git a/drivers/infiniband/ulp/ipoib
-by: Michael S. Tsirkin [EMAIL PROTECTED]
Signed-off-by: Roland Dreier [EMAIL PROTECTED]
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c
b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index c3b5f79..9d9cecd 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib
David Why not just put this into your -ipoib patch set in -mm?
David The change was for IPOIB's sake anyways...
OK, good idea. I'll put this in my for-2.6.17 and for-mm queues.
- R.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL
David How limited are the IPoIB devices, TX descriptor wise?
David One side effect of the TSO changes is that one extra
David descriptor will be used for outgoing packets. This is
David because we have to put the headers as well as the user
David data, into page based buffers
David I wish you had started the thread by mentioning this
David specific patch, we wasted an enormous amount of precious
David developer time speculating and asking for arbitrary tests
David to be run in order to narrow down the problem, yet you knew
David the specific change
Sean Export ip_dev_find to allow locating a net_device given an
Sean IP address.
My plan is to queue all of this stuff for merging in 2.6.17.
Is there any objection from netdev or openib-general people?
I just looked back, and the original unexport ip_dev_find() patch
was a de-Bunk-ing
+struct rdma_ucm_query_route_resp {
+__u64 node_guid;
+struct ib_user_path_rec ib_route[2];
+struct sockaddr_in6 src_addr;
+struct sockaddr_in6 dst_addr;
+__u32 num_paths;
+__u8 port_num;
+__u8 reserved[3];
+};
Is there a 32-bit/64-bit compatibility
I think it makes sense to merge patches 1-5 independently of this
patch. The kernel interface is needed by iSER and NFS/RDMA, and
maintaining compatibility isn't a huge deal, so we can merge it now
(assuming it looks mergable).
On the other hand I think it would be good to let this userspace
Sean Unless I miss counted, they should be aligned.
Sean ib_user_path_rec is defined near the end of patch 1/6.
You're right. struct sockaddr_in6 is 28 bytes long (not a multiple of
8) but gcc seems to lay everything out the same on 32-bit and 64-bit
architectures just the same.
- R.
-
David Please make sure you check x86_64 vs. x86, and then
David something like powerpc64 vs. powerpc32 or sparc64
David vs. sparc32, as those are the two different classes of ABI
David layouts.
Yes, I tried ppc64 vs ppc and it still comes out the same.
Unfortunately I don't have
David I wrote a test program and it looks ok:
Cool, thanks.
I should look into getting some niagara machines to test with -- with
PCIe slots they should actually be good for IB testing.
- R.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to
Roland I should look into getting some niagara machines to test
Roland with -- with PCIe slots they should actually be good for
Roland IB testing.
David You'll be cpu limited until we have Van Jacobson net
David channels.
For IPoIB maybe but not for native IB which offloads
Bryan Depends on the driver. Ours needs the interrupt vector
Bryan rather than the number, which means we don't work without
Bryan CONFIG_PCI_MSI. That is, unless there's some other way to
Bryan get the vector that I don't know about (entirely likely).
OK, fair enough. But
Roland struct neigh_ops currently has a destructor field, which
Roland no in-kernel drivers outside of infiniband use.
Andrew net/atm/clip.c begs to disagree.
err... my fault for trusting the patch changelog and not
double-checking. I think the fix is as simple as, although, given
Andrew I'm also wondering why a combination of the net and
Andrew infiniband trees does this:
Andrew drivers/infiniband/ulp/ipoib/ipoib_multicast.c: In
Andrew function `ipoib_mcast_free':
Andrew drivers/infiniband/ulp/ipoib/ipoib_multicast.c:118: error:
Andrew structure
Sean I can resubmit the patches if necessary.
Yes, can you please send out the patches again, with descriptive
subjects for each patch and with the ip_dev_find() re-export split
into its own patch? That would make it easier for me to pull into my
git tree.
Thanks,
Roland
-
To unsubscribe
Hi Dave, have you had a chance to look at this? I can resend again if
you've lost the original mail. Also, let me know if you want me to
merge this through my tree when 2.6.17 opens up.
The only feedback I've seen is that Yoshfuji-san has said that this
looks sane.
Thanks,
Roland
-
To
David Not yet, it's very low on the priority list at the moment,
David but I do still have it in my inbox so don't worry.
Do you think you'll get a chance to look at it for 2.6.17? If not we
can work around things in the IPoIB driver in a slightly uglier way
for 2.6.17.
Thanks,
Roland
James And the solution is to treat it as a boolean instead?! I'm
James not sure which is more ugly.
James Why wouldn't explicit comparison against NULL be the
James preferred fix?
if (ptr) and if (!ptr) are the preferred idiom for testing whether
a pointer is NULL. What is
Stephen Since many motherboards and chipsets don't handle MSI
Stephen properly, it is probably better to do a test interrupt
Stephen before fully using MSI. See tg3 for an example.
Is forcedeth ever used with a discrete ethernet controller? I thought
that nforce NICs are always
Roland Is forcedeth ever used with a discrete ethernet
Roland controller? I thought that nforce NICs are always
Roland intergrated in the chipset, and hence the entire scope for
Roland MSI problems is within a single chip. However, I don't
Roland know if there are devices
David It's sitting in my backlog, and will be a net-2.6.17 issue
David not a net-2.6.16 one as we're in bug fix mode there.
David Sorry if you need this in 2.6.16, but that's not really
David practical.
No, that's fine... I was just resending in case you were using RED to
manage
-by: Roland Dreier [EMAIL PROTECTED]
---
diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 6fa9ae1..b0666d6 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -68,6 +68,7 @@ struct neigh_parms
struct net_device *dev;
struct neigh_parms *next
+struct ucma_file {
+struct semaphoremutex;
This should be a struct mutex instead, I think.
+static DECLARE_MUTEX(ctx_mutex);
Same here.
- R.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info
+UCMA_MAX_BACKLOG= 128
Is there any reason that we might want to make this a tunable? Maybe
as a module parameter that's writable in sysfs...
- R.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info
Kumar I'm actually searching for any examples of drivers that
Kumar deal with the issues related to DMA'ng directly two and
Kumar from user space memory.
It's not quite the same story as what you're doing with DMA engines
inside the CPU, but you could look at drivers/infiniband,
201 - 288 of 288 matches
Mail list logo