Netdev List
 help / color / mirror / Atom feed
* [PATCH v2 net-next] drivers/net: Enable IOMMU pass through for be2net
From: Craig Hada @ 2013-01-19  2:23 UTC (permalink / raw)
  To: netdev, craig.hada
  Cc: sathya.perla, subbu.seetharaman, ajit.khaparde, linux-kernel
In-Reply-To: <1358562195-2028-1-git-send-email-craig.hada@hp.com>

This patch sets the coherent DMA mask to 64-bit after the be2net driver
has been acknowledged that the system is 64-bit DMA capable. The coherent
DMA mask is examined by the Intel IOMMU driver to determine whether to
allow pass through context mapping for all devices. With this patch, the
be2net driver combined with be2net compatible hardware provides
comparable performance to the case where vt-d is disabled. The main use
case for this change is to decrease the time necessary to copy virtual
machine memory during KVM live migration instantiations.

This patch was tested on a system that enables the IOMMU in non-coherent
mode. Two DMA remapper issues were encountered in the previous version and
both patches have been committed.
    commit ea2447f700cab264019b52e2b417d689e052dcfd
    commit 2e12bc29fc5a12242d68e11875db3dd58efad9ff

Signed-off-by: Craig Hada <craig.hada@hp.com>
---
 drivers/net/ethernet/emulex/benet/be_main.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 9dca22b..b507e99 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -4052,6 +4052,12 @@ static int be_probe(struct pci_dev *pdev, const struct pci_device_id *pdev_id)
 
 	status = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
 	if (!status) {
+		status = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
+		if (status < 0) {
+			dev_err(&pdev->dev,
+				"dma_set_coherent_mask failed, aborting\n");
+			goto free_netdev;
+		}
 		netdev->features |= NETIF_F_HIGHDMA;
 	} else {
 		status = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
-- 
1.7.1

^ permalink raw reply related

* [PATCH v2 net-next] drivers/net: Enable IOMMU pass through for be2net
From: Craig Hada @ 2013-01-19  2:23 UTC (permalink / raw)
  To: netdev, craig.hada
  Cc: sathya.perla, subbu.seetharaman, ajit.khaparde, linux-kernel

This patch sets the coherent DMA mask to 64-bit after the be2net driver 
has been acknowledged that the system is 64-bit DMA capable. The coherent 
DMA mask is examined by the Intel IOMMU driver to determine whether to 
allow pass through context mapping for all devices. With this patch, the 
be2net driver combined with be2net compatible hardware provides 
comparable performance to the case where vt-d is disabled. The main use 
case for this change is to decrease the time necessary to copy virtual 
machine memory during KVM live migration instantiations.

This patch was tested on a system that enables the IOMMU in non-coherent 
mode. Two DMA remapper issues were encountered in the previous version and 
both patches have been committed.
    commit ea2447f700cab264019b52e2b417d689e052dcfd
    commit 2e12bc29fc5a12242d68e11875db3dd58efad9ff

The performance of this patch was measured with netperf with vt-d 
enabled and disabled along with kernel boot parameters intel_iommu 
and iommu. The command and parameters used in the netperf runs along 
with results are as follows:


# netperf -c -p 12865 -H 10.10.0.2 -t TCP_STREAM -l 60

vt-d enabled and intel-iommu=on
Recv   Send    Send                          Utilization       Service Demand
Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
Size   Size    Size     Time     Throughput  local    remote   local   remote
bytes  bytes   bytes    secs.    10^6bits/s  % S      % U      us/KB   us/KB

 87380  16384  16384    60.00      4636.58   16.76    -1.00    1.777   -1.000
 87380  16384  16384    60.00      4718.79   17.30    -1.00    1.802   -1.000
 87380  16384  16384    60.00      4566.91   18.04    -1.00    1.942   -1.000

vt-d enabled
Recv   Send    Send                          Utilization       Service Demand
Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
Size   Size    Size     Time     Throughput  local    remote   local   remote
bytes  bytes   bytes    secs.    10^6bits/s  % S      % U      us/KB   us/KB

 87380  16384  16384    60.00      6837.12   7.21     -1.00    0.518   -1.000
 87380  16384  16384    60.00      6956.20   6.38     -1.00    0.450   -1.000
 87380  16384  16384    60.00      6903.63   6.75     -1.00    0.481   -1.000

vt-d disabled
Recv   Send    Send                          Utilization       Service Demand
Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
Size   Size    Size     Time     Throughput  local    remote   local   remote
bytes  bytes   bytes    secs.    10^6bits/s  % S      % U      us/KB   us/KB

 87380  16384  16384    60.00      6909.70   6.57     -1.00    0.467   -1.000
 87380  16384  16384    60.00      6811.79   6.90     -1.00    0.498   -1.000
 87380  16384  16384    60.00      6861.32   6.71     -1.00    0.481   -1.000

vt-d enabled and intel_iommu=on and iommu=pt
Recv   Send    Send                          Utilization       Service Demand
Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
Size   Size    Size     Time     Throughput  local    remote   local   remote
bytes  bytes   bytes    secs.    10^6bits/s  % S      % U      us/KB   us/KB

 87380  16384  16384    60.00      6768.45   7.30     -1.00    0.530   -1.000
 87380  16384  16384    60.00      6841.40   7.39     -1.00    0.531   -1.000
 87380  16384  16384    60.00      6861.78   5.68     -1.00    0.407   -1.000


Craig Hada (1):
  drivers/net: Enable IOMMU pass through for be2net

 drivers/net/ethernet/emulex/benet/be_main.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

^ permalink raw reply

* Re: IPsec AH use of ahash
From: Michal Kubecek @ 2013-01-19  2:33 UTC (permalink / raw)
  To: Tom St Denis
  Cc: Waskiewicz Jr, Peter P, David Miller, steffen klassert, herbert,
	linux-kernel, netdev
In-Reply-To: <1702471741.91455.1358548305936.JavaMail.root@elliptictech.com>

On Fri, Jan 18, 2013 at 05:31:45PM -0500, Tom St Denis wrote:
> My gripe here is suppose I spend professional paid time working on an
> AH patch to [in my opinion] fix it and then I get
> ignored/stonewalled/etc because I didn't cross a t or dot an i ...
...
> ... if the likelihood of seeing it in mainline is basically around 0%.

You received a detailed response from subsystem maintainer who is an
extremely busy man; that doesn't look like ignoring to me. And as for
stonewalling, I don't see anything like that either. You were just told
what is wrong and asked to send a fixed version. Once you do that, the
chances of the patch to be accepted are actually very high.

Yes, kernel rules for submitting and coding style may seem too strict at
the first glance. But there is a good reason for them: linux kernel is
huge and without strict rules it would be one big mess. Part of my work
consists of resolving bugs in various software projects, often these are
projects the sources of I have never seen before. And I wish more (or
rather as many as possible) had rules similar to the kernel because
looking for something in code which is badly structured and lacks
unified coding style, in project without good and descriptive commit
description, can be a terrible experience.

You may call it nitpicking and mock it, you may even feel offended, but
the open source world would be much better place to live in if other
projects had rules similar to linux kernel (and its networking in
particular).

> It would have taken them very little time to merge the patch I sent
> in, fix the (), maybe address some of the coding style "errors" and
> then form a patch for mainline based on that.  Don't you think I have
> better things to do than re-submit working code repeatedly?

So you consider your time too precious to conform to the kernel coding
style and, on the other hand, the time of subsystem maintainers totally
worthless so that you feel it is their job to tidy up your patches?

Someone already pointed you to http://patchwork.ozlabs.org/
Please do take a look there. I just did and found that in last three
months, about 3500 patches were submitted to this list, i.e. about
40 patches per day (including weekends and Christmas). All of these need
to be reviewed by a few maintainers who are also doing their part of
development. How do they manage to handle it, honestly I don't know.
And then you come and tell us they should also fix coding style and
obvious mistakes for everyone who is too lazy to do it themselves.
Don't you think it would be much more effective if we tried to make
their work easier rather than put more work on their shoulders?

> Ok but as "maintainers" they should be "maintaining" code ... if
> coding standards change (and btw the XCBC code was likely submitted
> long after the coding standards were put in place) then the
> maintainers should go through code they're responsible for and update
> it.

Fixing the wrong coding style of existing code would be definitely
useful but unlike reviewing patches, fixing bugs and developing
features, it doesn't require detailed knowledge of the code. Using
highly skilled and experienced developers (which is who subsystem
maintainers are), that would be a real wasting of resources.

> "xcbc.c" for instance was last touched in 2011.  It hasn't been
> maintained at all apparently.  There were a handful of patches against
> it ... none which address these "coding standards."

The fact that there are only few changes doesn't necessarily mean the
code is unmaintained. It can also mean that it works well and it doesn't
need new features or adjusting to new hardware or protocol versions. And
when the code doesn't change too often, the urge to fix its style is
rather low. But presence of old badly styled code doesn't justify
introducing more badly styled code.

                                                          Michal Kubecek

^ permalink raw reply

* [PATCH v3] net/hyperv: fix wrong length of mac address
From: Amos Kong @ 2013-01-19  2:52 UTC (permalink / raw)
  To: davem, kys, haiyangz; +Cc: netdev, devel, Amos Kong
In-Reply-To: <20130118083031.GA2303@ppwaskie-mobl2.amr.corp.intel.com>

This patch fixed wrong mac length, it should be ETH_ALEN,
also replaced the hardcode 6 in hyperv_net.h

Signed-off-by: Amos Kong <kongjianjun@gmail.com>
---
v2: update commitlog, fix cc list
v3: fix hardcode length in hyperv_net.h
---
 drivers/net/hyperv/hyperv_net.h | 2 +-
 drivers/net/hyperv/netvsc_drv.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 5fd6f46..e6fe0d8 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -84,7 +84,7 @@ struct hv_netvsc_packet {
 };
 
 struct netvsc_device_info {
-	unsigned char mac_adr[6];
+	unsigned char mac_adr[ETH_ALEN];
 	bool link_state;	/* 0 - link up, 1 - link down */
 	int  ring_size;
 };
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index f825a62..8264f0e 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -349,7 +349,7 @@ static int netvsc_set_mac_addr(struct net_device *ndev, void *p)
 	struct net_device_context *ndevctx = netdev_priv(ndev);
 	struct hv_device *hdev =  ndevctx->device_ctx;
 	struct sockaddr *addr = p;
-	char save_adr[14];
+	char save_adr[ETH_ALEN];
 	unsigned char save_aatype;
 	int err;
 
-- 
1.7.11.7

^ permalink raw reply related

* Re: IPsec AH use of ahash
From: Tom St Denis @ 2013-01-19  2:59 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: Waskiewicz Jr, Peter P, David Miller, steffen klassert, herbert,
	linux-kernel, netdev
In-Reply-To: <20130119023355.GB10964@lion>

----- Original Message -----
> From: "Michal Kubecek" <mkubecek@suse.cz>
> To: "Tom St Denis" <tstdenis@elliptictech.com>
> Cc: "Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@intel.com>, "David Miller" <davem@davemloft.net>, "steffen
> klassert" <steffen.klassert@secunet.com>, herbert@gondor.apana.org.au, linux-kernel@vger.kernel.org,
> netdev@vger.kernel.org
> Sent: Friday, 18 January, 2013 9:33:55 PM
> Subject: Re: IPsec AH use of ahash
> 
> On Fri, Jan 18, 2013 at 05:31:45PM -0500, Tom St Denis wrote:
> > My gripe here is suppose I spend professional paid time working on
> > an
> > AH patch to [in my opinion] fix it and then I get
> > ignored/stonewalled/etc because I didn't cross a t or dot an i ...
> ...
> > ... if the likelihood of seeing it in mainline is basically around
> > 0%.
> 
> You received a detailed response from subsystem maintainer who is an
> extremely busy man; that doesn't look like ignoring to me. And as for
> stonewalling, I don't see anything like that either. You were just
> told
> what is wrong and asked to send a fixed version. Once you do that,
> the
> chances of the patch to be accepted are actually very high.

My objection is mostly that the feedback wasn't practical.  [more on that later].

> Yes, kernel rules for submitting and coding style may seem too strict
> at
> the first glance. But there is a good reason for them: linux kernel
> is
> huge and without strict rules it would be one big mess. Part of my
> work
> consists of resolving bugs in various software projects, often these
> are
> projects the sources of I have never seen before. And I wish more (or
> rather as many as possible) had rules similar to the kernel because
> looking for something in code which is badly structured and lacks
> unified coding style, in project without good and descriptive commit
> description, can be a terrible experience.

Having worked with a fair bit of Kernel code I can definitely say there isn't really a unified standard.  About the only thing Kernel developers agree on is they use C and don't comment their code.

> You may call it nitpicking and mock it, you may even feel offended,
> but
> the open source world would be much better place to live in if other
> projects had rules similar to linux kernel (and its networking in
> particular).

Having run several [admittedly much smaller] OSS projects I never turned away contributors with such raw efficiency as what I've seen here.

Sometimes that means you do the bitch work to format things pretty or document API changes or do any number of non-glorious tasks.  I consider it a professional crime that "use the source luke" is not merely a joke but an actual de facto work standard.  
 
> So you consider your time too precious to conform to the kernel
> coding
> style and, on the other hand, the time of subsystem maintainers
> totally
> worthless so that you feel it is their job to tidy up your patches?

Yes.  They're maintainers of code that millions of people use.  If they're too busy to take it on they should take their names off the maintainers list.

You don't get the credit without doing the work.  

> Someone already pointed you to http://patchwork.ozlabs.org/
> Please do take a look there. I just did and found that in last three
> months, about 3500 patches were submitted to this list, i.e. about
> 40 patches per day (including weekends and Christmas). All of these
> need
> to be reviewed by a few maintainers who are also doing their part of
> development. How do they manage to handle it, honestly I don't know.
> And then you come and tell us they should also fix coding style and
> obvious mistakes for everyone who is too lazy to do it themselves.
> Don't you think it would be much more effective if we tried to make
> their work easier rather than put more work on their shoulders?

Maybe the netdev project is too large then?  If they can't maintain it either there isn't enough staff or it's just too large to organize.

The RFC for AH-GMAC is hardly new yet the IPsec AH driver cannot support it.  Furthermore, AEAD is a better match for people [like me] doing hardware acceleration since it means I only have to write one set of drivers.  For instance I do null-cipher ESP through an AEAD driver that implements "ecb(null_cipher),foo" to cut down the code I have to write considerably.  If AH used AEAD I could fully remove my ahash code and have much less to worry about.

So from a user point of view AH is implemented poorly.
 
> Fixing the wrong coding style of existing code would be definitely
> useful but unlike reviewing patches, fixing bugs and developing
> features, it doesn't require detailed knowledge of the code. Using
> highly skilled and experienced developers (which is who subsystem
> maintainers are), that would be a real wasting of resources.

The problem is coding styles are those of the owners.  I don't professionally develop code anything like the Kernel format and I will *never* adopt it.

For instance

if (foo)
   statement;

Is horrible to me.  I've personally seen co-workers cause problems with that format [without the braces].  etc...

So realistically it's not fair to require contributors from all walks of life to adopt some random coding style that isn't even applied to the accepted code.

> > "xcbc.c" for instance was last touched in 2011.  It hasn't been
> > maintained at all apparently.  There were a handful of patches
> > against
> > it ... none which address these "coding standards."
> 
> The fact that there are only few changes doesn't necessarily mean the
> code is unmaintained. It can also mean that it works well and it
> doesn't
> need new features or adjusting to new hardware or protocol versions.
> And
> when the code doesn't change too often, the urge to fix its style is
> rather low. But presence of old badly styled code doesn't justify
> introducing more badly styled code.

That's ironic that you accept "if it works leave it alone" but at the same time maintain that standards exist for a reason and should be adhered to.

More so, I stand by what I said.  As a new contributor if I can't base my original content on existing content as a template for "proper design" then what can I?  How is this not simply "do as I say not as I do?"

Tom

^ permalink raw reply

* Re: IPsec AH use of ahash
From: Eric Dumazet @ 2013-01-19  3:59 UTC (permalink / raw)
  To: Michal Kubecek
  Cc: Tom St Denis, Waskiewicz Jr, Peter P, David Miller,
	steffen klassert, herbert, linux-kernel, netdev
In-Reply-To: <20130119023355.GB10964@lion>

On Sat, 2013-01-19 at 03:33 +0100, Michal Kubecek wrote:

> Someone already pointed you to http://patchwork.ozlabs.org/
> Please do take a look there. I just did and found that in last three
> months, about 3500 patches were submitted to this list, i.e. about
> 40 patches per day (including weekends and Christmas). All of these need
> to be reviewed by a few maintainers who are also doing their part of
> development. How do they manage to handle it, honestly I don't know.

I want to make here a huge thanks to David Miller, and of course
to all contributors.

I truly believe we did very well, and I really hope new contributors
will come and continue the impressive work.

Sure, sometime we can react not as good as we could do if we were
not overloaded. (I mean, 6 hours listening Lance Amstrong confession.
You really cant avoid such a scoop !)

I understand that for a new contributor, it might be difficult to
catch up with various rules, but reading netdev archives should be
enough to understand how it really works. Its not like the process
was a secret.

Tom, even a maintainer can make errors, thats not a big deal, as long
as things can go forward.

If you felt you had 0% chance to get your patch being accepted, then
you had a wrong feeling.

If the patch makes sense and you agree to address reviewers feedback, it
definitely can be accepted. If you think you don't have time to do that,
then maybe the patch is not really needed.

^ permalink raw reply

* splice() giving unexpected EOF in 3.7.3 and 3.8-rc4+
From: Eric Wong @ 2013-01-19  4:49 UTC (permalink / raw)
  To: linux-kernel; +Cc: netdev, linux-fsdevel, Eric Dumazet, Willy Tarreau

With the following flow, I'm sometimes getting an unexpected EOF on the
pipe reader even though I never close the pipe writer:

  tcp_wr -write-> tcp_rd -splice-> pipe_wr -> pipe_rd -splice-> /dev/null

I encounter this in in 3.7.3, 3.8-rc3, and the latest from Linus
3.8-rc4+(5da1f88b8b727dc3a66c52d4513e871be6d43d19)

It takes longer (about 20s) to reproduce this issue on my KVM (2 cores)
running the latest Linus kernel, so maybe real/faster hardware is needed.
My dual-core laptop (on 3.7.3) which hosts the VM does encounter this
issue within a few seconds (or even <1s).

Using schedtool to pin to a single core (any CPU core) on real hardware
seems to avoid this issue on real hardware.  Not sure how KVM uses CPUs,
but schedtool doesn't help inside my VM (not even schedtool on the KVM
process).

Example code below (and via: git clone git://bogomips.org/spliceeof )

Expected outout from ./spliceeof:
	done writing
	splice(in) EOF (expected)

Output I get from ./spliceeof:
	splice(out) EOF (UNEXPECTED)
	in left: 47716 # the byte value varies

I've successfully run similar code within the past year on some 3.x
kernels, so I think this issue is fairly recent (Cc-ing folks who
have touched splice lately).

Any likely candidates before I start bisection?  Thanks for reading.

-------------------------------- 8< ------------------------------
#define _GNU_SOURCE
#include <poll.h>
#include <sys/ioctl.h>
#include <pthread.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <netinet/tcp.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <assert.h>
#include <limits.h>
#include <sys/times.h>

static void tcp_socketpair(int sv[2], int accept_flags)
{
	struct sockaddr_in addr;
	socklen_t addrlen = sizeof(addr);
	int l = socket(PF_INET, SOCK_STREAM, 0);
	int c = socket(PF_INET, SOCK_STREAM, 0);
	int a;

	addr.sin_family = AF_INET;
	addr.sin_addr.s_addr = INADDR_ANY;
	addr.sin_port = 0;
	assert(0 == bind(l, (struct sockaddr*)&addr, addrlen));
	assert(0 == listen(l, 5));
	assert(0 == getsockname(l, (struct sockaddr *)&addr, &addrlen));
	assert(0 == connect(c, (struct sockaddr *)&addr, addrlen));
	a = accept4(l, NULL, NULL, accept_flags);
	assert(a >= 0);
	close(l);
	sv[0] = a;
	sv[1] = c;
}

static void * write_loop(void * fdp)
{
	int fd = *(int *)fdp;
	char buf[16384];
	ssize_t w;
	size_t want = ULONG_MAX; /* try changing this around */

	while (want > 0) {
		size_t to_write = want > sizeof(buf) ? sizeof(buf) : want;

		w = write(fd, buf, to_write);

		if (w < 0) {
			dprintf(2, "write returned zero with %zu left\n", want);
			goto fail;
		} else if (w == 0) {
			dprintf(2, "write failed: %m with %zu left\n", want);
			goto fail;
		} else {
			want -= (size_t)w;
		}
	}
	dprintf(2, "done writing\n");
fail:
	close(fd);
	return NULL;
}

static void io_wait(int fd, short events)
{
	struct pollfd p;
	int rc;

	p.fd = fd;
	p.events = events;

	rc = poll(&p, 1, -1);
	assert(rc == 1 && "poll failed");
}

int main(void)
{
	int tcp_pair[2];
	int pbuf[2];
	pthread_t wt;
	int dst = open("/dev/null", O_WRONLY);
	size_t len = 1024 * 1024;
	ssize_t in, out;
	size_t in_total = 0;
	size_t out_total = 0;
	int fl = SPLICE_F_NONBLOCK;

	assert(dst >= 0 && "open(/dev/null) failed");
	tcp_socketpair(tcp_pair, SOCK_NONBLOCK);
	assert(0 == pthread_create(&wt, NULL, write_loop, &tcp_pair[1]));
	assert(0 == pipe2(pbuf, O_NONBLOCK));

	for (;;) {
		in = splice(tcp_pair[0], NULL, pbuf[1], NULL, len, fl);

		if (in < 0) {
			if (errno == EAGAIN) {
				io_wait(tcp_pair[0], POLLIN);
				io_wait(pbuf[1], POLLOUT);
				continue;
			}
			dprintf(2, "splice(in) err: %m\n");
			break;
		} else if (in == 0) {
			dprintf(2, "splice(in) EOF (expected)\n");
			break;
		}

		in_total += in;
		while (in > 0) {
			out = splice(pbuf[0], NULL, dst, NULL, (size_t)in, fl);
			if (out < 0) {
				dprintf(2, "splice(out) err: %m\n");
				exit(1);
			} else if (out == 0) {
				dprintf(2, "splice(out) EOF (UNEXPECTED)\n");
				dprintf(2, "in left: %zd\n", in);
				exit(1);
			} else {
				in -= out;
				out_total += out;
			}
		}
	}
	assert(0 == pthread_join(wt, NULL));
	return 0;
}
-------------------------------- 8< ------------------------------

^ permalink raw reply

* Re: splice() giving unexpected EOF in 3.7.3 and 3.8-rc4+
From: Eric Dumazet @ 2013-01-19  5:54 UTC (permalink / raw)
  To: Eric Wong; +Cc: linux-kernel, netdev, linux-fsdevel, Willy Tarreau
In-Reply-To: <20130119044957.GA25395@dcvr.yhbt.net>

On Sat, 2013-01-19 at 04:49 +0000, Eric Wong wrote:
> With the following flow, I'm sometimes getting an unexpected EOF on the
> pipe reader even though I never close the pipe writer:
> 
>   tcp_wr -write-> tcp_rd -splice-> pipe_wr -> pipe_rd -splice-> /dev/null
> 
> I encounter this in in 3.7.3, 3.8-rc3, and the latest from Linus
> 3.8-rc4+(5da1f88b8b727dc3a66c52d4513e871be6d43d19)
> 
> It takes longer (about 20s) to reproduce this issue on my KVM (2 cores)
> running the latest Linus kernel, so maybe real/faster hardware is needed.
> My dual-core laptop (on 3.7.3) which hosts the VM does encounter this
> issue within a few seconds (or even <1s).
> 
> Using schedtool to pin to a single core (any CPU core) on real hardware
> seems to avoid this issue on real hardware.  Not sure how KVM uses CPUs,
> but schedtool doesn't help inside my VM (not even schedtool on the KVM
> process).
> 
> Example code below (and via: git clone git://bogomips.org/spliceeof )
> 
> Expected outout from ./spliceeof:
> 	done writing
> 	splice(in) EOF (expected)
> 
> Output I get from ./spliceeof:
> 	splice(out) EOF (UNEXPECTED)
> 	in left: 47716 # the byte value varies
> 
> I've successfully run similar code within the past year on some 3.x
> kernels, so I think this issue is fairly recent (Cc-ing folks who
> have touched splice lately).
> 
> Any likely candidates before I start bisection?  Thanks for reading.
> 
> -------------------------------- 8< ------------------------------

Hmm, this might be already fixed in net-next tree, could you try it ?

Thanks !



^ permalink raw reply

* Re: splice() giving unexpected EOF in 3.7.3 and 3.8-rc4+
From: Eric Dumazet @ 2013-01-19  6:13 UTC (permalink / raw)
  To: Eric Wong, David Miller
  Cc: linux-kernel, netdev, linux-fsdevel, Willy Tarreau
In-Reply-To: <1358574847.3464.422.camel@edumazet-glaptop>

On Fri, 2013-01-18 at 21:54 -0800, Eric Dumazet wrote:

> 
> Hmm, this might be already fixed in net-next tree, could you try it ?
> 

Yes, running your program on net-next seems OK.

David, we need the two following commits.

They actually fixed a bug : current code in Linus tree
can push to the pipe a 0-length frag, because of the :

flen = min_t(unsigned int, flen, PAGE_SIZE - poff);

It can happen if poff == PAGE_SIZE, when one skb frag has this
particular starting offset.


commit 9ca1b22d6d228177e6f929f6818a1cd3d5e30c4a
Author: Eric Dumazet <edumazet@google.com>
Date:   Sat Jan 5 21:31:18 2013 +0000

    net: splice: avoid high order page splitting
    
    splice() can handle pages of any order, but network code tries hard to
    split them in PAGE_SIZE units. Not quite successfully anyway, as
    __splice_segment() assumed poff < PAGE_SIZE. This is true for
    the skb->data part, not necessarily for the fragments.
    
    This patch removes this logic to give the pages as they are in the skb.
    
    Signed-off-by: Eric Dumazet <edumazet@google.com>
    Cc: Willy Tarreau <w@1wt.eu>
    Signed-off-by: David S. Miller <davem@davemloft.net>


commit 18aafc622abf492809723d9c5a3c5dcea287169e
Author: Eric Dumazet <edumazet@google.com>
Date:   Fri Jan 11 14:46:37 2013 +0000

    net: splice: fix __splice_segment()
    
    commit 9ca1b22d6d2 (net: splice: avoid high order page splitting)
    forgot that skb->head could need a copy into several page frags.
    
    This could be the case for loopback traffic mostly.
    
    Also remove now useless skb argument from linear_to_page()
    and __splice_segment() prototypes.
    
    Signed-off-by: Eric Dumazet <edumazet@google.com>
    Cc: Willy Tarreau <w@1wt.eu>
    Signed-off-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* Re: splice() giving unexpected EOF in 3.7.3 and 3.8-rc4+
From: Willy Tarreau @ 2013-01-19  7:04 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Eric Wong, David Miller, linux-kernel, netdev, linux-fsdevel
In-Reply-To: <1358575996.3464.447.camel@edumazet-glaptop>

On Fri, Jan 18, 2013 at 10:13:16PM -0800, Eric Dumazet wrote:
> On Fri, 2013-01-18 at 21:54 -0800, Eric Dumazet wrote:
> 
> > 
> > Hmm, this might be already fixed in net-next tree, could you try it ?
> > 
> 
> Yes, running your program on net-next seems OK.
> 
> David, we need the two following commits.
> 
> They actually fixed a bug : current code in Linus tree
> can push to the pipe a 0-length frag, because of the :
(...)

And FWIW I confirm that my test machines which have been running 3.7
with these two patches since you proposed them have never experienced
such an issue.

Willy

^ permalink raw reply

* Re: splice() giving unexpected EOF in 3.7.3 and 3.8-rc4+
From: Eric Wong @ 2013-01-19  7:15 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, linux-kernel, netdev, linux-fsdevel, Willy Tarreau
In-Reply-To: <1358575996.3464.447.camel@edumazet-glaptop>

Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2013-01-18 at 21:54 -0800, Eric Dumazet wrote:
> > Hmm, this might be already fixed in net-next tree, could you try it ?
> 
> Yes, running your program on net-next seems OK.
> 
> David, we need the two following commits.

> commit 9ca1b22d6d228177e6f929f6818a1cd3d5e30c4a
> commit 18aafc622abf492809723d9c5a3c5dcea287169e

Thanks Eric!  I cherry picked both of these on top on 3.7.3
and everything is great \o/

^ permalink raw reply

* [PATCH 0/21] NTB and ntb_netdev patches
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger

Please apply these patches to Greg KH's char-misc-next tree.

Thanks,
Jon

^ permalink raw reply

* [PATCH 01/21] NTB: correct missing readq/writeq errors
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger
In-Reply-To: <1358586155-23322-1-git-send-email-jon.mason@intel.com>

Atomic readq and writeq do not exist by default on some 32bit
architectures, thus causing compile errors due to non-existent symbols.
In those cases, use the definitions of those symbols from
include/asm-generic/io-64-nonatomic-hi-lo.h

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_hw.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 4c71b17..0b46fef 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -45,6 +45,7 @@
  * Contact Information:
  * Jon Mason <jon.mason@intel.com>
  */
+#include <asm-generic/io-64-nonatomic-hi-lo.h>
 #include <linux/debugfs.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 02/21] NTB: Handle ntb client device probes without present hardware
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger
In-Reply-To: <1358586155-23322-1-git-send-email-jon.mason@intel.com>

Attempts to probe client ntb drivers without ntb hardware present will
result in null pointer dereference due to the lack of the ntb bus device
being registers.  Check to see if this is the case, and fail all calls
by the clients registering their drivers.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 250190f..1d17857 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -288,6 +288,9 @@ int ntb_register_client_dev(char *device_name)
 	struct ntb_transport *nt;
 	int rc;
 
+	if (list_empty(&ntb_transport_list))
+		return -ENODEV;
+
 	list_for_each_entry(nt, &ntb_transport_list, entry) {
 		struct device *dev;
 
@@ -336,6 +339,9 @@ int ntb_register_client(struct ntb_client *drv)
 {
 	drv->driver.bus = &ntb_bus_type;
 
+	if (list_empty(&ntb_transport_list))
+		return -ENODEV;
+
 	return driver_register(&drv->driver);
 }
 EXPORT_SYMBOL_GPL(ntb_register_client);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 03/21] NTB: correct memory barrier
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger
In-Reply-To: <1358586155-23322-1-git-send-email-jon.mason@intel.com>

mmiowb is not sufficient to flush the data and is causing data
corruption.  Change to wmb and the data corruption is no more.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 1d17857..e9666bd 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -1009,7 +1009,7 @@ static void ntb_tx_copy_task(struct ntb_transport_qp *qp,
 	hdr->ver = qp->tx_pkts;
 
 	/* Ensure that the data is fully copied out before setting the flag */
-	mmiowb();
+	wmb();
 	hdr->flags = entry->flags | DESC_DONE_FLAG;
 
 	ntb_ring_sdb(qp->ndev, qp->qp_num);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 05/21] NTB: No sleeping in interrupt context
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger
In-Reply-To: <1358586155-23322-1-git-send-email-jon.mason@intel.com>

Move all cancel_delayed_work_sync to a work thread to prevent sleeping
in interrupt context (when the NTB link goes down).  Caught via
'Sleep inside atomic section checking'

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 2823087..bf7ade1 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -110,6 +110,7 @@ struct ntb_transport_qp {
 
 	void (*event_handler) (void *data, int status);
 	struct delayed_work link_work;
+	struct work_struct link_cleanup;
 
 	struct dentry *debugfs_dir;
 	struct dentry *debugfs_stats;
@@ -148,6 +149,7 @@ struct ntb_transport {
 	unsigned long qp_bitmap;
 	bool transport_link;
 	struct delayed_work link_work;
+	struct work_struct link_cleanup;
 	struct dentry *debugfs_dir;
 };
 
@@ -510,8 +512,11 @@ static int ntb_set_mw(struct ntb_transport *nt, int num_mw, unsigned int size)
 	return 0;
 }
 
-static void ntb_qp_link_down(struct ntb_transport_qp *qp)
+static void ntb_qp_link_cleanup(struct work_struct *work)
 {
+	struct ntb_transport_qp *qp = container_of(work,
+						   struct ntb_transport_qp,
+						   link_cleanup);
 	struct ntb_transport *nt = qp->transport;
 	struct pci_dev *pdev = ntb_query_pdev(nt->ndev);
 
@@ -531,8 +536,15 @@ static void ntb_qp_link_down(struct ntb_transport_qp *qp)
 				      msecs_to_jiffies(NTB_LINK_DOWN_TIMEOUT));
 }
 
-static void ntb_transport_conn_down(struct ntb_transport *nt)
+static void ntb_qp_link_down(struct ntb_transport_qp *qp)
+{
+	schedule_work(&qp->link_cleanup);
+}
+
+static void ntb_transport_link_cleanup(struct work_struct *work)
 {
+	struct ntb_transport *nt = container_of(work, struct ntb_transport,
+						link_cleanup);
 	int i;
 
 	if (nt->transport_link == NTB_LINK_DOWN)
@@ -562,7 +574,7 @@ static void ntb_transport_event_callback(void *data, enum ntb_hw_event event)
 		schedule_delayed_work(&nt->link_work, 0);
 		break;
 	case NTB_EVENT_HW_LINK_DOWN:
-		ntb_transport_conn_down(nt);
+		schedule_work(&nt->link_cleanup);
 		break;
 	default:
 		BUG();
@@ -769,6 +781,7 @@ static void ntb_transport_init_queue(struct ntb_transport *nt,
 	}
 
 	INIT_DELAYED_WORK(&qp->link_work, ntb_qp_link_work);
+	INIT_WORK(&qp->link_cleanup, ntb_qp_link_cleanup);
 
 	spin_lock_init(&qp->ntb_rx_pend_q_lock);
 	spin_lock_init(&qp->ntb_rx_free_q_lock);
@@ -814,6 +827,7 @@ int ntb_transport_init(struct pci_dev *pdev)
 		ntb_transport_init_queue(nt, i);
 
 	INIT_DELAYED_WORK(&nt->link_work, ntb_transport_link_work);
+	INIT_WORK(&nt->link_cleanup, ntb_transport_link_cleanup);
 
 	rc = ntb_register_event_callback(nt->ndev,
 					 ntb_transport_event_callback);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 07/21] NTB: zero PCI driver data
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger
In-Reply-To: <1358586155-23322-1-git-send-email-jon.mason@intel.com>

Zero pci_device_id driver_data variable.  Unused, but 'EXTRA_CFLAGS=-W'
complained of uninitialized variables.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_hw.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 0b46fef..867ccaa 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -83,14 +83,14 @@ enum {
 #define MW_TO_BAR(mw)	(mw * 2 + 2)
 
 static DEFINE_PCI_DEVICE_TABLE(ntb_pci_tbl) = {
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_BWD)},
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_JSF)},
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_JSF)},
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_JSF)},
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_SNB)},
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_SNB)},
-	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_SNB)},
-	{0}
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_BWD),		0},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_JSF),		0},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_JSF),	0},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_JSF),		0},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_RP_SNB),		0},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_B2B_SNB),		0},
+	{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_NTB_CLASSIC_SNB),	0},
+	{0, 0, 0, 0, 0, 0, 0}
 };
 MODULE_DEVICE_TABLE(pci, ntb_pci_tbl);
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 08/21] NTB: declare unused variables
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger
In-Reply-To: <1358586155-23322-1-git-send-email-jon.mason@intel.com>

Tag pci_device_id in ntb_pci_probe as unused function parameters.  This
corrects issues found by 'EXTRA_CFLAGS=-W'.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_hw.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 867ccaa..2357c2d 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -1007,7 +1007,8 @@ static void ntb_free_callbacks(struct ntb_device *ndev)
 	kfree(ndev->db_cb);
 }
 
-static int ntb_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+static int ntb_pci_probe(struct pci_dev *pdev,
+			 __attribute__((unused)) const struct pci_device_id *id)
 {
 	struct ntb_device *ndev;
 	int rc, i;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 09/21] NTB: namespacecheck cleanups
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger
In-Reply-To: <1358586155-23322-1-git-send-email-jon.mason@intel.com>

Declare ntb_bus_type static to remove it from name space, and remove
unused ntb_get_max_spads function.  Found via `make namespacecheck`.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_hw.c        |   14 --------------
 drivers/ntb/ntb_transport.c |    2 +-
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 2357c2d..18cb5dc 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -238,20 +238,6 @@ void ntb_unregister_transport(struct ntb_device *ndev)
 }
 
 /**
- * ntb_get_max_spads() - get the total scratch regs usable
- * @ndev: pointer to ntb_device instance
- *
- * This function returns the max 32bit scratchpad registers usable by the
- * upper layer.
- *
- * RETURNS: total number of scratch pad registers available
- */
-int ntb_get_max_spads(struct ntb_device *ndev)
-{
-	return ndev->limits.max_spads;
-}
-
-/**
  * ntb_write_local_spad() - write to the secondary scratchpad register
  * @ndev: pointer to ntb_device instance
  * @idx: index to the scratchpad register, 0 based
diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index c0eca02..903a72e 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -212,7 +212,7 @@ static int ntb_client_remove(struct device *dev)
 	return 0;
 }
 
-struct bus_type ntb_bus_type = {
+static struct bus_type ntb_bus_type = {
 	.name = "ntb_bus",
 	.match = ntb_match_bus,
 	.probe = ntb_client_probe,
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 10/21] NTB: whitespace cleanups
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger
In-Reply-To: <1358586155-23322-1-git-send-email-jon.mason@intel.com>

Whitespace cleanups found via `indent`

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |   40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 903a72e..e11b57e 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -930,7 +930,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 
 	if (!(hdr->flags & DESC_DONE_FLAG)) {
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
-				  &qp->rx_pend_q);
+			     &qp->rx_pend_q);
 		qp->rx_ring_empty++;
 		return -EAGAIN;
 	}
@@ -940,7 +940,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 			"qp %d: version mismatch, expected %llu - got %llu\n",
 			qp->qp_num, qp->rx_pkts, hdr->ver);
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
-				  &qp->rx_pend_q);
+			     &qp->rx_pend_q);
 		qp->rx_err_ver++;
 		return -EIO;
 	}
@@ -949,7 +949,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 		ntb_qp_link_down(qp);
 
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
-				  &qp->rx_pend_q);
+			     &qp->rx_pend_q);
 
 		/* Ensure that the data is fully copied out before clearing the
 		 * done flag
@@ -967,7 +967,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 		ntb_rx_copy_task(qp, entry, offset);
 	else {
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
-				  &qp->rx_pend_q);
+			     &qp->rx_pend_q);
 
 		/* Ensure that the data is fully copied out before clearing the
 		 * done flag
@@ -1057,8 +1057,8 @@ static int ntb_process_tx(struct ntb_transport_qp *qp,
 	hdr = offset + qp->tx_max_frame - sizeof(struct ntb_payload_header);
 
 	dev_dbg(&ntb_query_pdev(qp->ndev)->dev, "%lld - offset %p, tx %p, entry len %d flags %x buff %p\n",
-		 qp->tx_pkts, offset, qp->tx_offset, entry->len, entry->flags,
-		 entry->buf);
+		qp->tx_pkts, offset, qp->tx_offset, entry->len, entry->flags,
+		entry->buf);
 	if (hdr->flags) {
 		qp->tx_ring_full++;
 		return -EAGAIN;
@@ -1097,8 +1097,7 @@ static void ntb_send_link_down(struct ntb_transport_qp *qp)
 	dev_info(&pdev->dev, "qp %d: Link Down\n", qp->qp_num);
 
 	for (i = 0; i < NTB_LINK_DOWN_TIMEOUT; i++) {
-		entry = ntb_list_rm(&qp->ntb_tx_free_q_lock,
-					 &qp->tx_free_q);
+		entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q);
 		if (entry)
 			break;
 		msleep(100);
@@ -1167,7 +1166,7 @@ ntb_transport_create_queue(void *data, struct pci_dev *pdev,
 			goto err1;
 
 		ntb_list_add(&qp->ntb_rx_free_q_lock, &entry->entry,
-				  &qp->rx_free_q);
+			     &qp->rx_free_q);
 	}
 
 	for (i = 0; i < NTB_QP_DEF_NUM_ENTRIES; i++) {
@@ -1176,7 +1175,7 @@ ntb_transport_create_queue(void *data, struct pci_dev *pdev,
 			goto err2;
 
 		ntb_list_add(&qp->ntb_tx_free_q_lock, &entry->entry,
-				  &qp->tx_free_q);
+			     &qp->tx_free_q);
 	}
 
 	tasklet_init(&qp->rx_work, ntb_transport_rx, (unsigned long) qp);
@@ -1193,12 +1192,10 @@ ntb_transport_create_queue(void *data, struct pci_dev *pdev,
 err3:
 	tasklet_disable(&qp->rx_work);
 err2:
-	while ((entry =
-		ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q)))
+	while ((entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q)))
 		kfree(entry);
 err1:
-	while ((entry =
-		ntb_list_rm(&qp->ntb_rx_free_q_lock, &qp->rx_free_q)))
+	while ((entry = ntb_list_rm(&qp->ntb_rx_free_q_lock, &qp->rx_free_q)))
 		kfree(entry);
 	set_bit(free_queue, &nt->qp_bitmap);
 err:
@@ -1225,18 +1222,15 @@ void ntb_transport_free_queue(struct ntb_transport_qp *qp)
 	ntb_unregister_db_callback(qp->ndev, qp->qp_num);
 	tasklet_disable(&qp->rx_work);
 
-	while ((entry =
-		ntb_list_rm(&qp->ntb_rx_free_q_lock, &qp->rx_free_q)))
+	while ((entry = ntb_list_rm(&qp->ntb_rx_free_q_lock, &qp->rx_free_q)))
 		kfree(entry);
 
-	while ((entry =
-		ntb_list_rm(&qp->ntb_rx_pend_q_lock, &qp->rx_pend_q))) {
+	while ((entry = ntb_list_rm(&qp->ntb_rx_pend_q_lock, &qp->rx_pend_q))) {
 		dev_warn(&pdev->dev, "Freeing item from a non-empty queue\n");
 		kfree(entry);
 	}
 
-	while ((entry =
-		ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q)))
+	while ((entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q)))
 		kfree(entry);
 
 	set_bit(qp->qp_num, &qp->transport->qp_bitmap);
@@ -1270,8 +1264,7 @@ void *ntb_transport_rx_remove(struct ntb_transport_qp *qp, unsigned int *len)
 	buf = entry->cb_data;
 	*len = entry->len;
 
-	ntb_list_add(&qp->ntb_rx_free_q_lock, &entry->entry,
-			  &qp->rx_free_q);
+	ntb_list_add(&qp->ntb_rx_free_q_lock, &entry->entry, &qp->rx_free_q);
 
 	return buf;
 }
@@ -1305,8 +1298,7 @@ int ntb_transport_rx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
 	entry->buf = data;
 	entry->len = len;
 
-	ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
-			  &qp->rx_pend_q);
+	ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry, &qp->rx_pend_q);
 
 	return 0;
 }
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 11/21] NTB: correct stack usage warning in debugfs_read
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger
In-Reply-To: <1358586155-23322-1-git-send-email-jon.mason@intel.com>

Correct gcc warning of using too much stack debugfs_read.  This is done
by kmallocing the buffer instead of using the char array on stack.
Also, shrinking the buffer to something closer to what is currently
being used.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index e11b57e..1bed1ba 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -368,10 +368,14 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
 			    loff_t *offp)
 {
 	struct ntb_transport_qp *qp;
-	char buf[1024];
+	char *buf;
 	ssize_t ret, out_offset, out_count;
 
-	out_count = 1024;
+	out_count = 600;
+
+	buf = kmalloc(out_count, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
 
 	qp = filp->private_data;
 	out_offset = 0;
@@ -410,10 +414,13 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
 			       "tx_mw_end - \t%p\n", qp->tx_mw_end);
 
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "QP Link %s\n", (qp->qp_link == NTB_LINK_UP) ?
+			       "\nQP Link %s\n", (qp->qp_link == NTB_LINK_UP) ?
 			       "Up" : "Down");
+	if (out_offset > out_count)
+		out_offset = out_count;
 
 	ret = simple_read_from_buffer(ubuf, count, offp, buf, out_offset);
+	kfree(buf);
 	return ret;
 }
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 12/21] NTB: Remove reads across NTB
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger
In-Reply-To: <1358586155-23322-1-git-send-email-jon.mason@intel.com>

CPU reads across NTB are slow(er) and can hang the local system if an
ECC error is encountered on the remote.  To work around the need for a
read, have the remote side write its current position in the rx buffer
to the local system's buffer and use that to see if there is room when
transmitting.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |  137 ++++++++++++++++++++-----------------------
 1 file changed, 63 insertions(+), 74 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 1bed1ba..69c58da 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -78,6 +78,10 @@ struct ntb_queue_entry {
 	unsigned int flags;
 };
 
+struct ntb_rx_info {
+	unsigned int entry;
+};
+
 struct ntb_transport_qp {
 	struct ntb_transport *transport;
 	struct ntb_device *ndev;
@@ -87,13 +91,16 @@ struct ntb_transport_qp {
 	bool qp_link;
 	u8 qp_num;	/* Only 64 QP's are allowed.  0-63 */
 
+	struct ntb_rx_info *rx_info;
+	struct ntb_rx_info *remote_rx_info;
+
 	void (*tx_handler) (struct ntb_transport_qp *qp, void *qp_data,
 			    void *data, int len);
 	struct list_head tx_free_q;
 	spinlock_t ntb_tx_free_q_lock;
-	void *tx_mw_begin;
-	void *tx_mw_end;
-	void *tx_offset;
+	void *tx_mw;
+	unsigned int tx_index;
+	unsigned int tx_max_entry;
 	unsigned int tx_max_frame;
 
 	void (*rx_handler) (struct ntb_transport_qp *qp, void *qp_data,
@@ -103,9 +110,9 @@ struct ntb_transport_qp {
 	struct list_head rx_free_q;
 	spinlock_t ntb_rx_pend_q_lock;
 	spinlock_t ntb_rx_free_q_lock;
-	void *rx_buff_begin;
-	void *rx_buff_end;
-	void *rx_offset;
+	void *rx_buff;
+	unsigned int rx_index;
+	unsigned int rx_max_entry;
 	unsigned int rx_max_frame;
 
 	void (*event_handler) (void *data, int status);
@@ -394,11 +401,11 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
 			       "rx_err_ver - \t%llu\n", qp->rx_err_ver);
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "rx_buff_begin - %p\n", qp->rx_buff_begin);
+			       "rx_buff - \t%p\n", qp->rx_buff);
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "rx_offset - \t%p\n", qp->rx_offset);
+			       "rx_index - \t%u\n", qp->rx_index);
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "rx_buff_end - \t%p\n", qp->rx_buff_end);
+			       "rx_max_entry - \t%u\n", qp->rx_max_entry);
 
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
 			       "tx_bytes - \t%llu\n", qp->tx_bytes);
@@ -407,11 +414,11 @@ static ssize_t debugfs_read(struct file *filp, char __user *ubuf, size_t count,
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
 			       "tx_ring_full - \t%llu\n", qp->tx_ring_full);
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "tx_mw_begin - \t%p\n", qp->tx_mw_begin);
+			       "tx_mw - \t%p\n", qp->tx_mw);
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "tx_offset - \t%p\n", qp->tx_offset);
+			       "tx_index - \t%u\n", qp->tx_index);
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
-			       "tx_mw_end - \t%p\n", qp->tx_mw_end);
+			       "tx_max_entry - \t%u\n", qp->tx_max_entry);
 
 	out_offset += snprintf(buf + out_offset, out_count - out_offset,
 			       "\nQP Link %s\n", (qp->qp_link == NTB_LINK_UP) ?
@@ -465,7 +472,7 @@ static void ntb_transport_setup_qp_mw(struct ntb_transport *nt,
 	struct ntb_transport_qp *qp = &nt->qps[qp_num];
 	unsigned int rx_size, num_qps_mw;
 	u8 mw_num = QP_TO_MW(qp_num);
-	void *offset;
+	unsigned int i;
 
 	WARN_ON(nt->mw[mw_num].virt_addr == 0);
 
@@ -474,18 +481,24 @@ static void ntb_transport_setup_qp_mw(struct ntb_transport *nt,
 	else
 		num_qps_mw = nt->max_qps / NTB_NUM_MW;
 
-	rx_size = nt->mw[mw_num].size / num_qps_mw;
-	qp->rx_buff_begin = nt->mw[mw_num].virt_addr +
-			    (qp_num / NTB_NUM_MW * rx_size);
-	qp->rx_buff_end = qp->rx_buff_begin + rx_size;
-	qp->rx_offset = qp->rx_buff_begin;
+	rx_size = (unsigned int) nt->mw[mw_num].size / num_qps_mw;
+	qp->remote_rx_info = nt->mw[mw_num].virt_addr +
+			     (qp_num / NTB_NUM_MW * rx_size);
+	rx_size -= sizeof(struct ntb_rx_info);
+
+	qp->rx_buff = qp->remote_rx_info + sizeof(struct ntb_rx_info);
 	qp->rx_max_frame = min(transport_mtu, rx_size);
+	qp->rx_max_entry = rx_size / qp->rx_max_frame;
+	qp->rx_index = 0;
+
+	qp->remote_rx_info->entry = qp->rx_max_entry;
 
 	/* setup the hdr offsets with 0's */
-	for (offset = qp->rx_buff_begin + qp->rx_max_frame -
-		      sizeof(struct ntb_payload_header);
-	     offset < qp->rx_buff_end; offset += qp->rx_max_frame)
+	for (i = 0; i < qp->rx_max_entry; i++) {
+		void *offset = qp->rx_buff + qp->rx_max_frame * (i + 1) -
+			       sizeof(struct ntb_payload_header);
 		memset(offset, 0, sizeof(struct ntb_payload_header));
+	}
 
 	qp->rx_pkts = 0;
 	qp->tx_pkts = 0;
@@ -762,12 +775,15 @@ static void ntb_transport_init_queue(struct ntb_transport *nt,
 	else
 		num_qps_mw = nt->max_qps / NTB_NUM_MW;
 
-	tx_size = ntb_get_mw_size(qp->ndev, mw_num) / num_qps_mw;
-	qp->tx_mw_begin = ntb_get_mw_vbase(nt->ndev, mw_num) +
-			  (qp_num / NTB_NUM_MW * tx_size);
-	qp->tx_mw_end = qp->tx_mw_begin + tx_size;
-	qp->tx_offset = qp->tx_mw_begin;
+	tx_size = (unsigned int) ntb_get_mw_size(qp->ndev, mw_num) / num_qps_mw;
+	qp->rx_info = ntb_get_mw_vbase(nt->ndev, mw_num) +
+		      (qp_num / NTB_NUM_MW * tx_size);
+	tx_size -= sizeof(struct ntb_rx_info);
+
+	qp->tx_mw = qp->rx_info + sizeof(struct ntb_rx_info);
 	qp->tx_max_frame = min(transport_mtu, tx_size);
+	qp->tx_max_entry = tx_size / qp->tx_max_frame;
+	qp->tx_index = 0;
 
 	if (nt->debugfs_dir) {
 		char debugfs_name[4];
@@ -894,21 +910,8 @@ void ntb_transport_free(void *transport)
 static void ntb_rx_copy_task(struct ntb_transport_qp *qp,
 			     struct ntb_queue_entry *entry, void *offset)
 {
-
-	struct ntb_payload_header *hdr;
-
-	BUG_ON(offset < qp->rx_buff_begin ||
-	       offset + qp->rx_max_frame >= qp->rx_buff_end);
-
-	hdr = offset + qp->rx_max_frame - sizeof(struct ntb_payload_header);
-	entry->len = hdr->len;
-
 	memcpy(entry->buf, offset, entry->len);
 
-	/* Ensure that the data is fully copied out before clearing the flag */
-	wmb();
-	hdr->flags = 0;
-
 	if (qp->rx_handler && qp->client_ready == NTB_LINK_UP)
 		qp->rx_handler(qp, qp->cb_data, entry->cb_data, entry->len);
 
@@ -921,10 +924,11 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 	struct ntb_queue_entry *entry;
 	void *offset;
 
+	offset = qp->rx_buff + qp->rx_max_frame * qp->rx_index;
+	hdr = offset + qp->rx_max_frame - sizeof(struct ntb_payload_header);
+
 	entry = ntb_list_rm(&qp->ntb_rx_pend_q_lock, &qp->rx_pend_q);
 	if (!entry) {
-		hdr = offset + qp->rx_max_frame -
-		      sizeof(struct ntb_payload_header);
 		dev_dbg(&ntb_query_pdev(qp->ndev)->dev,
 			"no buffer - HDR ver %llu, len %d, flags %x\n",
 			hdr->ver, hdr->len, hdr->flags);
@@ -932,9 +936,6 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 		return -ENOMEM;
 	}
 
-	offset = qp->rx_offset;
-	hdr = offset + qp->rx_max_frame - sizeof(struct ntb_payload_header);
-
 	if (!(hdr->flags & DESC_DONE_FLAG)) {
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
 			     &qp->rx_pend_q);
@@ -957,30 +958,20 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
 			     &qp->rx_pend_q);
-
-		/* Ensure that the data is fully copied out before clearing the
-		 * done flag
-		 */
-		wmb();
-		hdr->flags = 0;
 		goto out;
 	}
 
 	dev_dbg(&ntb_query_pdev(qp->ndev)->dev,
-		"rx offset %p, ver %llu - %d payload received, buf size %d\n",
-		qp->rx_offset, hdr->ver, hdr->len, entry->len);
+		"rx offset %u, ver %llu - %d payload received, buf size %d\n",
+		qp->rx_index, hdr->ver, hdr->len, entry->len);
 
-	if (hdr->len <= entry->len)
+	if (hdr->len <= entry->len) {
+		entry->len = hdr->len;
 		ntb_rx_copy_task(qp, entry, offset);
-	else {
+	} else {
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
 			     &qp->rx_pend_q);
 
-		/* Ensure that the data is fully copied out before clearing the
-		 * done flag
-		 */
-		wmb();
-		hdr->flags = 0;
 		qp->rx_err_oflow++;
 		dev_dbg(&ntb_query_pdev(qp->ndev)->dev,
 			"RX overflow! Wanted %d got %d\n",
@@ -991,9 +982,13 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 	qp->rx_pkts++;
 
 out:
-	qp->rx_offset += qp->rx_max_frame;
-	if (qp->rx_offset + qp->rx_max_frame >= qp->rx_buff_end)
-		qp->rx_offset = qp->rx_buff_begin;
+	/* Ensure that the data is fully copied out before clearing the flag */
+	wmb();
+	hdr->flags = 0;
+	qp->rx_info->entry = qp->rx_index;
+
+	qp->rx_index++;
+	qp->rx_index %= qp->rx_max_entry;
 
 	return 0;
 }
@@ -1024,9 +1019,6 @@ static void ntb_tx_copy_task(struct ntb_transport_qp *qp,
 {
 	struct ntb_payload_header *hdr;
 
-	BUG_ON(offset < qp->tx_mw_begin ||
-	       offset + qp->tx_max_frame >= qp->tx_mw_end);
-
 	memcpy_toio(offset, entry->buf, entry->len);
 
 	hdr = offset + qp->tx_max_frame - sizeof(struct ntb_payload_header);
@@ -1057,16 +1049,14 @@ static void ntb_tx_copy_task(struct ntb_transport_qp *qp,
 static int ntb_process_tx(struct ntb_transport_qp *qp,
 			  struct ntb_queue_entry *entry)
 {
-	struct ntb_payload_header *hdr;
 	void *offset;
 
-	offset = qp->tx_offset;
-	hdr = offset + qp->tx_max_frame - sizeof(struct ntb_payload_header);
+	offset = qp->tx_mw + qp->tx_max_frame * qp->tx_index;
 
-	dev_dbg(&ntb_query_pdev(qp->ndev)->dev, "%lld - offset %p, tx %p, entry len %d flags %x buff %p\n",
-		qp->tx_pkts, offset, qp->tx_offset, entry->len, entry->flags,
+	dev_dbg(&ntb_query_pdev(qp->ndev)->dev, "%lld - offset %p, tx %u, entry len %d flags %x buff %p\n",
+		qp->tx_pkts, offset, qp->tx_index, entry->len, entry->flags,
 		entry->buf);
-	if (hdr->flags) {
+	if (qp->tx_index == qp->remote_rx_info->entry) {
 		qp->tx_ring_full++;
 		return -EAGAIN;
 	}
@@ -1082,9 +1072,8 @@ static int ntb_process_tx(struct ntb_transport_qp *qp,
 
 	ntb_tx_copy_task(qp, entry, offset);
 
-	qp->tx_offset += qp->tx_max_frame;
-	if (qp->tx_offset + qp->tx_max_frame >= qp->tx_mw_end)
-		qp->tx_offset = qp->tx_mw_begin;
+	qp->tx_index++;
+	qp->tx_index %= qp->tx_max_entry;
 
 	qp->tx_pkts++;
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 13/21] NTB: Out of free receive entries issue
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger
In-Reply-To: <1358586155-23322-1-git-send-email-jon.mason@intel.com>

If the NTB client driver enqueues the maximum number of rx buffers, it
will not be able to re-enqueue another in its callback handler due to a
lack of free entries.  This can be avoided by adding the current entry
to the free queue prior to calling the client callback handler.  With
this change, ntb_netdev will no longer encounter a rx error on its first
packet.

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_transport.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 69c58da..b3afb24 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -910,12 +910,15 @@ void ntb_transport_free(void *transport)
 static void ntb_rx_copy_task(struct ntb_transport_qp *qp,
 			     struct ntb_queue_entry *entry, void *offset)
 {
-	memcpy(entry->buf, offset, entry->len);
+	void *cb_data = entry->cb_data;
+	unsigned int len = entry->len;
 
-	if (qp->rx_handler && qp->client_ready == NTB_LINK_UP)
-		qp->rx_handler(qp, qp->cb_data, entry->cb_data, entry->len);
+	memcpy(entry->buf, offset, entry->len);
 
 	ntb_list_add(&qp->ntb_rx_free_q_lock, &entry->entry, &qp->rx_free_q);
+
+	if (qp->rx_handler && qp->client_ready == NTB_LINK_UP)
+		qp->rx_handler(qp, qp->cb_data, cb_data, len);
 }
 
 static int ntb_process_rxc(struct ntb_transport_qp *qp)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 14/21] NTB: Fix Sparse Warnings
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger
In-Reply-To: <1358586155-23322-1-git-send-email-jon.mason@intel.com>

Address the sparse warnings and resulting fallout

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_hw.c        |    7 ++++---
 drivers/ntb/ntb_hw.h        |    4 ++--
 drivers/ntb/ntb_transport.c |   32 ++++++++++++++++----------------
 3 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index 18cb5dc..b792ccd 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -104,8 +104,9 @@ MODULE_DEVICE_TABLE(pci, ntb_pci_tbl);
  *
  * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
  */
-int ntb_register_event_callback(struct ntb_device *ndev,
-				void (*func)(void *handle, unsigned int event))
+int
+ntb_register_event_callback(struct ntb_device *ndev,
+			    void (*func)(void *handle, enum ntb_hw_event event))
 {
 	if (ndev->event_cb)
 		return -EINVAL;
@@ -344,7 +345,7 @@ int ntb_read_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 *val)
  *
  * RETURNS: pointer to virtual address, or NULL on error.
  */
-void *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw)
+void __iomem *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw)
 {
 	if (mw > NTB_NUM_MW)
 		return NULL;
diff --git a/drivers/ntb/ntb_hw.h b/drivers/ntb/ntb_hw.h
index 5e00951..3a3038c 100644
--- a/drivers/ntb/ntb_hw.h
+++ b/drivers/ntb/ntb_hw.h
@@ -165,14 +165,14 @@ int ntb_register_db_callback(struct ntb_device *ndev, unsigned int idx,
 void ntb_unregister_db_callback(struct ntb_device *ndev, unsigned int idx);
 int ntb_register_event_callback(struct ntb_device *ndev,
 				void (*event_cb_func) (void *handle,
-						       unsigned int event));
+						      enum ntb_hw_event event));
 void ntb_unregister_event_callback(struct ntb_device *ndev);
 int ntb_get_max_spads(struct ntb_device *ndev);
 int ntb_write_local_spad(struct ntb_device *ndev, unsigned int idx, u32 val);
 int ntb_read_local_spad(struct ntb_device *ndev, unsigned int idx, u32 *val);
 int ntb_write_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 val);
 int ntb_read_remote_spad(struct ntb_device *ndev, unsigned int idx, u32 *val);
-void *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw);
+void __iomem *ntb_get_mw_vbase(struct ntb_device *ndev, unsigned int mw);
 resource_size_t ntb_get_mw_size(struct ntb_device *ndev, unsigned int mw);
 void ntb_ring_sdb(struct ntb_device *ndev, unsigned int idx);
 void *ntb_find_transport(struct pci_dev *pdev);
diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index b3afb24..e0bdfd7 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -58,7 +58,7 @@
 #include <linux/ntb.h>
 #include "ntb_hw.h"
 
-#define NTB_TRANSPORT_VERSION	1
+#define NTB_TRANSPORT_VERSION	2
 
 static unsigned int transport_mtu = 0x401E;
 module_param(transport_mtu, uint, 0644);
@@ -91,14 +91,14 @@ struct ntb_transport_qp {
 	bool qp_link;
 	u8 qp_num;	/* Only 64 QP's are allowed.  0-63 */
 
-	struct ntb_rx_info *rx_info;
+	struct ntb_rx_info __iomem *rx_info;
 	struct ntb_rx_info *remote_rx_info;
 
 	void (*tx_handler) (struct ntb_transport_qp *qp, void *qp_data,
 			    void *data, int len);
 	struct list_head tx_free_q;
 	spinlock_t ntb_tx_free_q_lock;
-	void *tx_mw;
+	void __iomem *tx_mw;
 	unsigned int tx_index;
 	unsigned int tx_max_entry;
 	unsigned int tx_max_frame;
@@ -166,7 +166,7 @@ enum {
 };
 
 struct ntb_payload_header {
-	u64 ver;
+	unsigned int ver;
 	unsigned int len;
 	unsigned int flags;
 };
@@ -474,7 +474,7 @@ static void ntb_transport_setup_qp_mw(struct ntb_transport *nt,
 	u8 mw_num = QP_TO_MW(qp_num);
 	unsigned int i;
 
-	WARN_ON(nt->mw[mw_num].virt_addr == 0);
+	WARN_ON(nt->mw[mw_num].virt_addr == NULL);
 
 	if (nt->max_qps % NTB_NUM_MW && mw_num < nt->max_qps % NTB_NUM_MW)
 		num_qps_mw = nt->max_qps / NTB_NUM_MW + 1;
@@ -933,7 +933,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 	entry = ntb_list_rm(&qp->ntb_rx_pend_q_lock, &qp->rx_pend_q);
 	if (!entry) {
 		dev_dbg(&ntb_query_pdev(qp->ndev)->dev,
-			"no buffer - HDR ver %llu, len %d, flags %x\n",
+			"no buffer - HDR ver %u, len %d, flags %x\n",
 			hdr->ver, hdr->len, hdr->flags);
 		qp->rx_err_no_buf++;
 		return -ENOMEM;
@@ -946,9 +946,9 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 		return -EAGAIN;
 	}
 
-	if (hdr->ver != qp->rx_pkts) {
+	if (hdr->ver != (u32) qp->rx_pkts) {
 		dev_dbg(&ntb_query_pdev(qp->ndev)->dev,
-			"qp %d: version mismatch, expected %llu - got %llu\n",
+			"qp %d: version mismatch, expected %llu - got %u\n",
 			qp->qp_num, qp->rx_pkts, hdr->ver);
 		ntb_list_add(&qp->ntb_rx_pend_q_lock, &entry->entry,
 			     &qp->rx_pend_q);
@@ -965,7 +965,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 	}
 
 	dev_dbg(&ntb_query_pdev(qp->ndev)->dev,
-		"rx offset %u, ver %llu - %d payload received, buf size %d\n",
+		"rx offset %u, ver %u - %d payload received, buf size %d\n",
 		qp->rx_index, hdr->ver, hdr->len, entry->len);
 
 	if (hdr->len <= entry->len) {
@@ -988,7 +988,7 @@ out:
 	/* Ensure that the data is fully copied out before clearing the flag */
 	wmb();
 	hdr->flags = 0;
-	qp->rx_info->entry = qp->rx_index;
+	iowrite32(qp->rx_index, &qp->rx_info->entry);
 
 	qp->rx_index++;
 	qp->rx_index %= qp->rx_max_entry;
@@ -1018,19 +1018,19 @@ static void ntb_transport_rxc_db(void *data, int db_num)
 
 static void ntb_tx_copy_task(struct ntb_transport_qp *qp,
 			     struct ntb_queue_entry *entry,
-			     void *offset)
+			     void __iomem *offset)
 {
-	struct ntb_payload_header *hdr;
+	struct ntb_payload_header __iomem *hdr;
 
 	memcpy_toio(offset, entry->buf, entry->len);
 
 	hdr = offset + qp->tx_max_frame - sizeof(struct ntb_payload_header);
-	hdr->len = entry->len;
-	hdr->ver = qp->tx_pkts;
+	iowrite32(entry->len, &hdr->len);
+	iowrite32((u32) qp->tx_pkts, &hdr->ver);
 
 	/* Ensure that the data is fully copied out before setting the flag */
 	wmb();
-	hdr->flags = entry->flags | DESC_DONE_FLAG;
+	iowrite32(entry->flags | DESC_DONE_FLAG, &hdr->flags);
 
 	ntb_ring_sdb(qp->ndev, qp->qp_num);
 
@@ -1052,7 +1052,7 @@ static void ntb_tx_copy_task(struct ntb_transport_qp *qp,
 static int ntb_process_tx(struct ntb_transport_qp *qp,
 			  struct ntb_queue_entry *entry)
 {
-	void *offset;
+	void __iomem *offset;
 
 	offset = qp->tx_mw + qp->tx_max_frame * qp->tx_index;
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 15/21] NTB: Update Version
From: Jon Mason @ 2013-01-19  9:02 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, netdev, Dave Jiang, Nicholas Bellinger
In-Reply-To: <1358586155-23322-1-git-send-email-jon.mason@intel.com>

Update NTB version to 0.25

Signed-off-by: Jon Mason <jon.mason@intel.com>
---
 drivers/ntb/ntb_hw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ntb/ntb_hw.c b/drivers/ntb/ntb_hw.c
index b792ccd..df86882 100644
--- a/drivers/ntb/ntb_hw.c
+++ b/drivers/ntb/ntb_hw.c
@@ -56,7 +56,7 @@
 #include "ntb_regs.h"
 
 #define NTB_NAME	"Intel(R) PCI-E Non-Transparent Bridge Driver"
-#define NTB_VER		"0.24"
+#define NTB_VER		"0.25"
 
 MODULE_DESCRIPTION(NTB_NAME);
 MODULE_VERSION(NTB_VER);
-- 
1.7.9.5

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox