* Re: Performance regression with virtio_net
From: Seth Forshee @ 2017-07-28 13:12 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Jason Wang, virtualization, netdev
In-Reply-To: <20170728002913-mutt-send-email-mst@kernel.org>
On Fri, Jul 28, 2017 at 12:30:54AM +0300, Michael S. Tsirkin wrote:
> On Thu, Jul 27, 2017 at 04:14:30PM -0500, Seth Forshee wrote:
> > On Thu, Jul 27, 2017 at 11:38:52PM +0300, Michael S. Tsirkin wrote:
> > > On Thu, Jul 27, 2017 at 12:09:42PM -0500, Seth Forshee wrote:
> > > > I'm seeing a performance regression with virtio_net that looks to have
> > > > started in 4.12-rc1. I only see it in one context though, downloading
> > > > snap packages from the Ubuntu snap store. For example:
> > > >
> > > > https://api.snapcraft.io/api/v1/snaps/download/b8X2psL1ryVrPt5WEmpYiqfr5emixTd7_1797.snap
> > > >
> > > > which redirects to Internap's CDN. Normally this downloads in a few
> > > > seconds at ~10 MB/s, but with 4.12 and 4.13 it takes minutes with a rate
> > > > of ~150 KB/s. Everything else I've tried downloads as normal speeds.
> > >
> > > So just wget that URL should be enough?
> >
> > Yes. Note that sometimes it starts out faster then slows down.
> > > > I bisected this to 680557cf79f8 "virtio_net: rework mergeable buffer
> > > > handling". If I revert this on top of 4.13-rc2 (along with other changes
> > > > needed to successfully revert it) speeds return to normal.
> > > >
> > > > Thanks,
> > > > Seth
> > >
> > >
> > > Interesting. A more likely suspect would be
> > > e377fcc8486d40867c6c217077ad0fa40977e060 - could you please try
> > > reverting that one instead?
> >
> > I tried it, and I still get slow download speeds. I did test at
> > 680557cf79f82623e2c4fd42733077d60a843513 during the bisect so I'm
> > reasonably confident that this is the one where things went bad.
> > > Also, could you please look at mergeable_rx_buffer_size in sysfs with
> > > and without the change?
> >
> > In all cases (stock 4.13-rc2, 680557cf79f8 reverted, and e377fcc8486d
> > reverted) mergeable_rx_buffer_size was 1536.
> >
> > Thanks,
> > Seth
>
> Do you see any error counters incrementing after it slows down?
I see rx_dropped and rx_length_errors increasing in lockstep once it
slows down.
Thanks,
Seth
^ permalink raw reply
* Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings
From: Kurt Van Dijck @ 2017-07-28 13:02 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: Franklin S Cooper Jr, Andrew Lunn, linux-kernel, devicetree,
netdev, linux-can, wg, mkl, robh+dt, quentin.schulz,
sergei.shtylyov
>
> On 07/28/2017 06:57 AM, Kurt Van Dijck wrote:
>
> >So while _a_ transceiver may be spec'd to 1MBit during arbitration,
> >CAN FD packets may IMHO exceed that speed during data phase.
>
> When the bitrate is limited to 1Mbit/s you are ONLY allowed to use 1Mbit/s
> in the data section too (either with CAN or CAN FD).
My point is that the requirements posed to a transceiver
differ between arbitration & data phase for CAN FD.
So while a transceiver does not know about CAN FD, it may allow
higher bitrates for the data phase.
>
> >That was the whole point of CAN FD: exceed the limits required for
> >correct arbitration on transceiver & wire.
>
> No. CAN FD is about a different frame format with up to 64 bytes AND the
> possibility to increase the bitrate in the data section of the frame.
>
> >So I do not agree on the single bandwidth limitation.
>
> The transceiver provides a single maximum bandwidth. It's an ISO Layer 1
> device.
>
> >The word 'max-arbitration-bitrate' makes the difference very clear.
>
> I think you are mixing up ISO layer 1 and ISO layer 2.
In order to provide higher data throughput without putting extra limits
on transceiver & wire, the requirement for the round-trip delay to be
within 1 bittime has been eliminated, but only for the data phase when
arbitration is over.
So layer 2 (CAN FD) has been adapted to circumvent the layer 1
(transceiver + wire) limitations.
In fact, the round-trip delay requirement never actually did matter for
plain CAN during data bits either. CAN FD just makes use of that,
but is therefore incompatible on the wire.
I forgot the precise wording, but this is the principle that Bosch
explained on the CAN conference in Nurnberg several years ago, or at
least this is how I remembered it :-)
I haven't followed the developments of transceivers, but with the above
principle in mind, it's obvious that any transceiver allows higher
bitrates during the data segment because the TX-to-RX line delay must
not scale with the bitrate.
In reality, maybe not all transceivers will mention this in their
datasheet.
So whether you call it 'max-arbitration-bitrate' & 'max-data-bitrate'
or 'max-bitrate' & 'max-data-bitrate' does not really matter (I prefer
1st) but you will one day need 2 bitrates.
Kind regards,
Kurt
^ permalink raw reply
* Re: After a while of system running no incoming UDP any more?
From: Marc Haber @ 2017-07-28 12:14 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev
In-Reply-To: <1501229277.2597.1.camel@redhat.com>
On Fri, Jul 28, 2017 at 10:07:57AM +0200, Paolo Abeni wrote:
> Ad a workaround you can disable UDP early demux:
>
> echo 0 > /proc/sys/net/ipv4/udp_early_demux
>
> (will affect both ipv4 and ipv6).
>
> and (if the system is already into the bad state) increase the udp
> accounted memory limit, writing in /proc/sys/net/ipv4/udp_mem greater
> values than the current ones (the actual values depends on the system
> total memory).
I can confirm that these two changes make a system in bad state work
again immediately. Will try the patch on 4.12.4 later today.
Thanks for helping!
Greetings
Marc
--
-----------------------------------------------------------------------------
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Leimen, Germany | lose things." Winona Ryder | Fon: *49 6224 1600402
Nordisch by Nature | How to make an American Quilt | Fax: *49 6224 1600421
^ permalink raw reply
* [PATCH net-next 08/10] net/smc: common functions for RMBs and send buffers
From: Ursula Braun @ 2017-07-28 11:56 UTC (permalink / raw)
To: davem
Cc: hch, netdev, linux-rdma, linux-s390, jwi, schwidefsky,
heiko.carstens, raspl, ubraun
In-Reply-To: <20170728115622.17247-1-ubraun@linux.vnet.ibm.com>
Creation and deletion of SMC receive and send buffers shares a high
amount of common code . This patch introduces common functions to get
rid of duplicate code.
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
net/smc/af_smc.c | 16 +--
net/smc/smc_core.c | 348 ++++++++++++++++++++++-------------------------------
net/smc/smc_core.h | 3 +-
3 files changed, 148 insertions(+), 219 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index e0a95d50bf87..75518879b68a 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -436,12 +436,8 @@ static int smc_connect_rdma(struct smc_sock *smc)
smc_conn_save_peer_info(smc, &aclc);
- rc = smc_sndbuf_create(smc);
- if (rc) {
- reason_code = SMC_CLC_DECL_MEM;
- goto decline_rdma_unlock;
- }
- rc = smc_rmb_create(smc);
+ /* create send buffer and rmb */
+ rc = smc_buf_create(smc);
if (rc) {
reason_code = SMC_CLC_DECL_MEM;
goto decline_rdma_unlock;
@@ -813,12 +809,8 @@ static void smc_listen_work(struct work_struct *work)
}
link = &new_smc->conn.lgr->lnk[SMC_SINGLE_LINK];
- rc = smc_sndbuf_create(new_smc);
- if (rc) {
- reason_code = SMC_CLC_DECL_MEM;
- goto decline_rdma;
- }
- rc = smc_rmb_create(new_smc);
+ /* create send buffer and rmb */
+ rc = smc_buf_create(new_smc);
if (rc) {
reason_code = SMC_CLC_DECL_MEM;
goto decline_rdma;
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 8795c7ed9ce4..ab8cdac5edb3 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -206,16 +206,12 @@ static int smc_lgr_create(struct smc_sock *smc, __be32 peer_in_addr,
return rc;
}
-static void smc_sndbuf_unuse(struct smc_connection *conn)
+static void smc_buf_unuse(struct smc_connection *conn)
{
if (conn->sndbuf_desc) {
conn->sndbuf_desc->used = 0;
conn->sndbuf_size = 0;
}
-}
-
-static void smc_rmb_unuse(struct smc_connection *conn)
-{
if (conn->rmb_desc) {
conn->rmb_desc->reused = true;
conn->rmb_desc->used = 0;
@@ -232,8 +228,7 @@ void smc_conn_free(struct smc_connection *conn)
return;
smc_cdc_tx_dismiss_slots(conn);
smc_lgr_unregister_conn(conn);
- smc_rmb_unuse(conn);
- smc_sndbuf_unuse(conn);
+ smc_buf_unuse(conn);
}
static void smc_link_clear(struct smc_link *lnk)
@@ -246,51 +241,57 @@ static void smc_link_clear(struct smc_link *lnk)
smc_wr_free_link_mem(lnk);
}
-static void smc_lgr_free_sndbufs(struct smc_link_group *lgr)
+static void smc_buf_free(struct smc_buf_desc *buf_desc, struct smc_link *lnk,
+ bool is_rmb)
{
- struct smc_link *lnk = &lgr->lnk[SMC_SINGLE_LINK];
- struct smc_buf_desc *sndbuf_desc, *bf_desc;
- int i;
-
- for (i = 0; i < SMC_RMBE_SIZES; i++) {
- list_for_each_entry_safe(sndbuf_desc, bf_desc, &lgr->sndbufs[i],
- list) {
- list_del(&sndbuf_desc->list);
- smc_ib_buf_unmap_sg(lnk->smcibdev, sndbuf_desc,
- DMA_TO_DEVICE);
- sg_free_table(&sndbuf_desc->sgt[SMC_SINGLE_LINK]);
- free_pages((unsigned long)sndbuf_desc->cpu_addr,
- sndbuf_desc->order);
- kfree(sndbuf_desc);
- }
+ if (is_rmb) {
+ if (buf_desc->mr_rx[SMC_SINGLE_LINK])
+ smc_ib_put_memory_region(
+ buf_desc->mr_rx[SMC_SINGLE_LINK]);
+ smc_ib_buf_unmap_sg(lnk->smcibdev, buf_desc,
+ DMA_FROM_DEVICE);
+ } else {
+ smc_ib_buf_unmap_sg(lnk->smcibdev, buf_desc,
+ DMA_TO_DEVICE);
}
+ sg_free_table(&buf_desc->sgt[SMC_SINGLE_LINK]);
+ if (buf_desc->cpu_addr)
+ free_pages((unsigned long)buf_desc->cpu_addr, buf_desc->order);
+ kfree(buf_desc);
}
-static void smc_lgr_free_rmbs(struct smc_link_group *lgr)
+static void __smc_lgr_free_bufs(struct smc_link_group *lgr, bool is_rmb)
{
struct smc_link *lnk = &lgr->lnk[SMC_SINGLE_LINK];
- struct smc_buf_desc *rmb_desc, *bf_desc;
+ struct smc_buf_desc *buf_desc, *bf_desc;
+ struct list_head *buf_list;
int i;
for (i = 0; i < SMC_RMBE_SIZES; i++) {
- list_for_each_entry_safe(rmb_desc, bf_desc, &lgr->rmbs[i],
+ if (is_rmb)
+ buf_list = &lgr->rmbs[i];
+ else
+ buf_list = &lgr->sndbufs[i];
+ list_for_each_entry_safe(buf_desc, bf_desc, buf_list,
list) {
- list_del(&rmb_desc->list);
- smc_ib_put_memory_region(
- rmb_desc->mr_rx[SMC_SINGLE_LINK]);
- smc_ib_buf_unmap_sg(lnk->smcibdev, rmb_desc,
- DMA_FROM_DEVICE);
- kfree(rmb_desc->cpu_addr);
- kfree(rmb_desc);
+ list_del(&buf_desc->list);
+ smc_buf_free(buf_desc, lnk, is_rmb);
}
}
}
+static void smc_lgr_free_bufs(struct smc_link_group *lgr)
+{
+ /* free send buffers */
+ __smc_lgr_free_bufs(lgr, false);
+ /* free rmbs */
+ __smc_lgr_free_bufs(lgr, true);
+}
+
/* remove a link group */
void smc_lgr_free(struct smc_link_group *lgr)
{
- smc_lgr_free_rmbs(lgr);
- smc_lgr_free_sndbufs(lgr);
+ smc_lgr_free_bufs(lgr);
smc_link_clear(&lgr->lnk[SMC_SINGLE_LINK]);
kfree(lgr);
}
@@ -455,45 +456,25 @@ int smc_conn_create(struct smc_sock *smc, __be32 peer_in_addr,
return rc ? rc : local_contact;
}
-/* try to reuse a sndbuf description slot of the sndbufs list for a certain
- * buf_size; if not available, return NULL
+/* try to reuse a sndbuf or rmb description slot for a certain
+ * buffer size; if not available, return NULL
*/
static inline
-struct smc_buf_desc *smc_sndbuf_get_slot(struct smc_link_group *lgr,
- int compressed_bufsize)
+struct smc_buf_desc *smc_buf_get_slot(struct smc_link_group *lgr,
+ int compressed_bufsize,
+ rwlock_t *lock,
+ struct list_head *buf_list)
{
- struct smc_buf_desc *sndbuf_slot;
-
- read_lock_bh(&lgr->sndbufs_lock);
- list_for_each_entry(sndbuf_slot, &lgr->sndbufs[compressed_bufsize],
- list) {
- if (cmpxchg(&sndbuf_slot->used, 0, 1) == 0) {
- read_unlock_bh(&lgr->sndbufs_lock);
- return sndbuf_slot;
- }
- }
- read_unlock_bh(&lgr->sndbufs_lock);
- return NULL;
-}
+ struct smc_buf_desc *buf_slot;
-/* try to reuse an rmb description slot of the rmbs list for a certain
- * rmbe_size; if not available, return NULL
- */
-static inline
-struct smc_buf_desc *smc_rmb_get_slot(struct smc_link_group *lgr,
- int compressed_bufsize)
-{
- struct smc_buf_desc *rmb_slot;
-
- read_lock_bh(&lgr->rmbs_lock);
- list_for_each_entry(rmb_slot, &lgr->rmbs[compressed_bufsize],
- list) {
- if (cmpxchg(&rmb_slot->used, 0, 1) == 0) {
- read_unlock_bh(&lgr->rmbs_lock);
- return rmb_slot;
+ read_lock_bh(lock);
+ list_for_each_entry(buf_slot, buf_list, list) {
+ if (cmpxchg(&buf_slot->used, 0, 1) == 0) {
+ read_unlock_bh(lock);
+ return buf_slot;
}
}
- read_unlock_bh(&lgr->rmbs_lock);
+ read_unlock_bh(lock);
return NULL;
}
@@ -506,185 +487,142 @@ static inline int smc_rmb_wnd_update_limit(int rmbe_size)
return min_t(int, rmbe_size / 10, SOCK_MIN_SNDBUF / 2);
}
-/* create the tx buffer for an SMC socket */
-int smc_sndbuf_create(struct smc_sock *smc)
+static int __smc_buf_create(struct smc_sock *smc, bool is_rmb)
{
struct smc_connection *conn = &smc->conn;
struct smc_link_group *lgr = conn->lgr;
- struct smc_buf_desc *sndbuf_desc;
+ struct smc_buf_desc *buf_desc = NULL;
+ struct list_head *buf_list;
int bufsize, bufsize_short;
+ struct smc_link *lnk;
+ int sk_buf_size;
+ rwlock_t *lock;
int rc;
- /* use socket send buffer size (w/o overhead) as start value */
+ lnk = &lgr->lnk[SMC_SINGLE_LINK];
+ if (is_rmb)
+ /* use socket recv buffer size (w/o overhead) as start value */
+ sk_buf_size = smc->sk.sk_rcvbuf / 2;
+ else
+ /* use socket send buffer size (w/o overhead) as start value */
+ sk_buf_size = smc->sk.sk_sndbuf / 2;
+
for (bufsize_short = smc_compress_bufsize(smc->sk.sk_sndbuf / 2);
bufsize_short >= 0; bufsize_short--) {
- bufsize = smc_uncompress_bufsize(bufsize_short);
- if ((1 << get_order(bufsize)) > SG_MAX_SINGLE_ALLOC)
- continue;
-
- /* check for reusable sndbuf_slot in the link group */
- sndbuf_desc = smc_sndbuf_get_slot(lgr, bufsize_short);
- if (sndbuf_desc) {
- memset(sndbuf_desc->cpu_addr, 0, bufsize);
- break; /* found reusable slot */
- }
- /* try to alloc a new send buffer */
- sndbuf_desc = kzalloc(sizeof(*sndbuf_desc), GFP_KERNEL);
- if (!sndbuf_desc)
- break; /* give up with -ENOMEM */
- sndbuf_desc->cpu_addr =
- (void *)__get_free_pages(GFP_KERNEL | __GFP_NOWARN |
- __GFP_NOMEMALLOC |
- __GFP_NORETRY | __GFP_ZERO,
- get_order(bufsize));
- if (!sndbuf_desc->cpu_addr) {
- kfree(sndbuf_desc);
- sndbuf_desc = NULL;
- /* if send buffer allocation has failed,
- * try a smaller one
- */
- continue;
- }
- sndbuf_desc->order = get_order(bufsize);
-
- rc = sg_alloc_table(&sndbuf_desc->sgt[SMC_SINGLE_LINK], 1,
- GFP_KERNEL);
- if (rc) {
- free_pages((unsigned long)sndbuf_desc->cpu_addr,
- sndbuf_desc->order);
- kfree(sndbuf_desc);
- sndbuf_desc = NULL;
- continue;
+ if (is_rmb) {
+ lock = &lgr->rmbs_lock;
+ buf_list = &lgr->rmbs[bufsize_short];
+ } else {
+ lock = &lgr->sndbufs_lock;
+ buf_list = &lgr->sndbufs[bufsize_short];
}
- sg_set_buf(sndbuf_desc->sgt[SMC_SINGLE_LINK].sgl,
- sndbuf_desc->cpu_addr, bufsize);
-
- rc = smc_ib_buf_map_sg(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
- sndbuf_desc, DMA_TO_DEVICE);
- if (rc != 1) {
- sg_free_table(&sndbuf_desc->sgt[SMC_SINGLE_LINK]);
- free_pages((unsigned long)sndbuf_desc->cpu_addr,
- sndbuf_desc->order);
- kfree(sndbuf_desc);
- sndbuf_desc = NULL;
- continue; /* if mapping failed, try smaller one */
- }
-
- sndbuf_desc->used = 1;
- write_lock_bh(&lgr->sndbufs_lock);
- list_add(&sndbuf_desc->list, &lgr->sndbufs[bufsize_short]);
- write_unlock_bh(&lgr->sndbufs_lock);
- break;
- }
- if (sndbuf_desc && sndbuf_desc->cpu_addr) {
- conn->sndbuf_desc = sndbuf_desc;
- conn->sndbuf_size = bufsize;
- smc->sk.sk_sndbuf = bufsize * 2;
- atomic_set(&conn->sndbuf_space, bufsize);
- return 0;
- } else {
- return -ENOMEM;
- }
-}
-
-/* create the RMB for an SMC socket (even though the SMC protocol
- * allows more than one RMB-element per RMB, the Linux implementation
- * uses just one RMB-element per RMB, i.e. uses an extra RMB for every
- * connection in a link group
- */
-int smc_rmb_create(struct smc_sock *smc)
-{
- struct smc_connection *conn = &smc->conn;
- struct smc_link_group *lgr = conn->lgr;
- struct smc_buf_desc *rmb_desc;
- int bufsize, bufsize_short;
- int rc;
-
- /* use socket recv buffer size (w/o overhead) as start value */
- for (bufsize_short = smc_compress_bufsize(smc->sk.sk_rcvbuf / 2);
- bufsize_short >= 0; bufsize_short--) {
bufsize = smc_uncompress_bufsize(bufsize_short);
if ((1 << get_order(bufsize)) > SG_MAX_SINGLE_ALLOC)
continue;
- /* check for reusable rmb_slot in the link group */
- rmb_desc = smc_rmb_get_slot(lgr, bufsize_short);
- if (rmb_desc) {
- memset(rmb_desc->cpu_addr, 0, bufsize);
+ /* check for reusable slot in the link group */
+ buf_desc = smc_buf_get_slot(lgr, bufsize_short, lock, buf_list);
+ if (buf_desc) {
+ memset(buf_desc->cpu_addr, 0, bufsize);
break; /* found reusable slot */
}
- /* try to alloc a new RMB */
- rmb_desc = kzalloc(sizeof(*rmb_desc), GFP_KERNEL);
- if (!rmb_desc)
+ /* try to allocate the determined number of pages */
+ buf_desc = kzalloc(sizeof(*buf_desc), GFP_KERNEL);
+ if (!buf_desc)
break; /* give up with -ENOMEM */
- rmb_desc->cpu_addr =
+
+ buf_desc->cpu_addr =
(void *)__get_free_pages(GFP_KERNEL | __GFP_NOWARN |
__GFP_NOMEMALLOC |
__GFP_NORETRY | __GFP_ZERO,
get_order(bufsize));
- if (!rmb_desc->cpu_addr) {
- kfree(rmb_desc);
- rmb_desc = NULL;
+ if (!buf_desc->cpu_addr) {
+ kfree(buf_desc);
+ buf_desc = NULL;
continue;
}
- rmb_desc->order = get_order(bufsize);
- rc = sg_alloc_table(&rmb_desc->sgt[SMC_SINGLE_LINK], 1,
+ rc = sg_alloc_table(&buf_desc->sgt[SMC_SINGLE_LINK], 1,
GFP_KERNEL);
if (rc) {
- free_pages((unsigned long)rmb_desc->cpu_addr,
- rmb_desc->order);
- kfree(rmb_desc);
- rmb_desc = NULL;
+ smc_buf_free(buf_desc, lnk, is_rmb);
+ buf_desc = NULL;
continue;
}
- sg_set_buf(rmb_desc->sgt[SMC_SINGLE_LINK].sgl,
- rmb_desc->cpu_addr, bufsize);
+ sg_set_buf(buf_desc->sgt[SMC_SINGLE_LINK].sgl,
+ buf_desc->cpu_addr, bufsize);
- rc = smc_ib_buf_map_sg(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
- rmb_desc, DMA_FROM_DEVICE);
+ /* map sg table to DMA address */
+ rc = smc_ib_buf_map_sg(lnk->smcibdev, buf_desc, is_rmb ?
+ DMA_FROM_DEVICE : DMA_TO_DEVICE);
+ /* SMC protocol depends on mapping to one DMA address only */
if (rc != 1) {
- sg_free_table(&rmb_desc->sgt[SMC_SINGLE_LINK]);
- free_pages((unsigned long)rmb_desc->cpu_addr,
- rmb_desc->order);
- kfree(rmb_desc);
- rmb_desc = NULL;
- continue; /* if mapping failed, try smaller one */
+ smc_buf_free(buf_desc, lnk, is_rmb);
+ buf_desc = NULL;
+ continue; /* if mapping failed, try smaller one */
}
- rc = smc_ib_get_memory_region(lgr->lnk[SMC_SINGLE_LINK].roce_pd,
- IB_ACCESS_REMOTE_WRITE |
- IB_ACCESS_LOCAL_WRITE,
- rmb_desc);
- if (rc) {
- smc_ib_buf_unmap_sg(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
- rmb_desc, DMA_FROM_DEVICE);
- sg_free_table(&rmb_desc->sgt[SMC_SINGLE_LINK]);
- free_pages((unsigned long)rmb_desc->cpu_addr,
- rmb_desc->order);
- kfree(rmb_desc);
- rmb_desc = NULL;
- continue;
+
+ /* create a new memory region for the RMB */
+ if (is_rmb) {
+ rc = smc_ib_get_memory_region(lnk->roce_pd,
+ IB_ACCESS_REMOTE_WRITE |
+ IB_ACCESS_LOCAL_WRITE,
+ buf_desc);
+ if (rc) {
+ smc_buf_free(buf_desc, lnk, is_rmb);
+ buf_desc = NULL;
+ continue;
+ }
}
- rmb_desc->used = 1;
- write_lock_bh(&lgr->rmbs_lock);
- list_add(&rmb_desc->list, &lgr->rmbs[bufsize_short]);
- write_unlock_bh(&lgr->rmbs_lock);
- break;
+ buf_desc->used = 1;
+ write_lock_bh(lock);
+ list_add(&buf_desc->list, buf_list);
+ write_unlock_bh(lock);
+ break; /* found */
}
- if (rmb_desc && rmb_desc->cpu_addr) {
- conn->rmb_desc = rmb_desc;
+
+ if (!buf_desc || !buf_desc->cpu_addr)
+ return -ENOMEM;
+
+ if (is_rmb) {
+ conn->rmb_desc = buf_desc;
conn->rmbe_size = bufsize;
conn->rmbe_size_short = bufsize_short;
smc->sk.sk_rcvbuf = bufsize * 2;
atomic_set(&conn->bytes_to_rcv, 0);
conn->rmbe_update_limit = smc_rmb_wnd_update_limit(bufsize);
- return 0;
} else {
- return -ENOMEM;
+ conn->sndbuf_desc = buf_desc;
+ conn->sndbuf_size = bufsize;
+ smc->sk.sk_sndbuf = bufsize * 2;
+ atomic_set(&conn->sndbuf_space, bufsize);
}
+ return 0;
+}
+
+/* create the send and receive buffer for an SMC socket;
+ * receive buffers are called RMBs;
+ * (even though the SMC protocol allows more than one RMB-element per RMB,
+ * the Linux implementation uses just one RMB-element per RMB, i.e. uses an
+ * extra RMB for every connection in a link group
+ */
+int smc_buf_create(struct smc_sock *smc)
+{
+ int rc;
+
+ /* create send buffer */
+ rc = __smc_buf_create(smc, false);
+ if (rc)
+ return rc;
+ /* create rmb */
+ rc = __smc_buf_create(smc, true);
+ if (rc)
+ smc_buf_free(smc->conn.sndbuf_desc,
+ &smc->conn.lgr->lnk[SMC_SINGLE_LINK], false);
+ return rc;
}
static inline int smc_rmb_reserve_rtoken_idx(struct smc_link_group *lgr)
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index 72c25cb3eb89..1d713e8e067c 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -186,8 +186,7 @@ struct smc_clc_msg_accept_confirm;
void smc_lgr_free(struct smc_link_group *lgr);
void smc_lgr_terminate(struct smc_link_group *lgr);
-int smc_sndbuf_create(struct smc_sock *smc);
-int smc_rmb_create(struct smc_sock *smc);
+int smc_buf_create(struct smc_sock *smc);
int smc_rmb_rtoken_handling(struct smc_connection *conn,
struct smc_clc_msg_accept_confirm *clc);
#endif
--
2.11.2
^ permalink raw reply related
* [PATCH net-next 10/10] net/smc: synchronize buffer usage with device
From: Ursula Braun @ 2017-07-28 11:56 UTC (permalink / raw)
To: davem
Cc: hch, netdev, linux-rdma, linux-s390, jwi, schwidefsky,
heiko.carstens, raspl, ubraun
In-Reply-To: <20170728115622.17247-1-ubraun@linux.vnet.ibm.com>
Usage of send buffer "sndbuf" is synced
(a) before filling sndbuf for cpu access
(b) after filling sndbuf for device access
Usage of receive buffer "RMB" is synced
(a) before reading RMB content for cpu access
(b) after reading RMB content for device access
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
net/smc/af_smc.c | 2 ++
net/smc/smc_core.c | 32 ++++++++++++++++++++++++++++++++
net/smc/smc_core.h | 4 ++++
net/smc/smc_ib.c | 41 +++++++++++++++++++++++++++++++++++++++++
net/smc/smc_ib.h | 6 ++++++
net/smc/smc_rx.c | 3 +++
net/smc/smc_tx.c | 3 +++
7 files changed, 91 insertions(+)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 75518879b68a..8c6d24b2995d 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -474,6 +474,7 @@ static int smc_connect_rdma(struct smc_sock *smc)
}
}
}
+ smc_rmb_sync_sg_for_device(&smc->conn);
rc = smc_clc_send_confirm(smc);
if (rc)
@@ -832,6 +833,7 @@ static void smc_listen_work(struct work_struct *work)
}
}
}
+ smc_rmb_sync_sg_for_device(&new_smc->conn);
rc = smc_clc_send_accept(new_smc, local_contact);
if (rc)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 447bd52da0e2..1a16d51e2330 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -615,6 +615,38 @@ static int __smc_buf_create(struct smc_sock *smc, bool is_rmb)
return 0;
}
+void smc_sndbuf_sync_sg_for_cpu(struct smc_connection *conn)
+{
+ struct smc_link_group *lgr = conn->lgr;
+
+ smc_ib_sync_sg_for_cpu(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
+ conn->sndbuf_desc, DMA_TO_DEVICE);
+}
+
+void smc_sndbuf_sync_sg_for_device(struct smc_connection *conn)
+{
+ struct smc_link_group *lgr = conn->lgr;
+
+ smc_ib_sync_sg_for_device(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
+ conn->sndbuf_desc, DMA_TO_DEVICE);
+}
+
+void smc_rmb_sync_sg_for_cpu(struct smc_connection *conn)
+{
+ struct smc_link_group *lgr = conn->lgr;
+
+ smc_ib_sync_sg_for_cpu(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
+ conn->rmb_desc, DMA_FROM_DEVICE);
+}
+
+void smc_rmb_sync_sg_for_device(struct smc_connection *conn)
+{
+ struct smc_link_group *lgr = conn->lgr;
+
+ smc_ib_sync_sg_for_device(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
+ conn->rmb_desc, DMA_FROM_DEVICE);
+}
+
/* create the send and receive buffer for an SMC socket;
* receive buffers are called RMBs;
* (even though the SMC protocol allows more than one RMB-element per RMB,
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index 1d713e8e067c..19c44bf4e391 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -189,4 +189,8 @@ void smc_lgr_terminate(struct smc_link_group *lgr);
int smc_buf_create(struct smc_sock *smc);
int smc_rmb_rtoken_handling(struct smc_connection *conn,
struct smc_clc_msg_accept_confirm *clc);
+void smc_sndbuf_sync_sg_for_cpu(struct smc_connection *conn);
+void smc_sndbuf_sync_sg_for_device(struct smc_connection *conn);
+void smc_rmb_sync_sg_for_cpu(struct smc_connection *conn);
+void smc_rmb_sync_sg_for_device(struct smc_connection *conn);
#endif
diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
index 021f061609f5..547e0e113b17 100644
--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c
@@ -13,6 +13,7 @@
#include <linux/random.h>
#include <linux/workqueue.h>
+#include <linux/scatterlist.h>
#include <rdma/ib_verbs.h>
#include "smc_pnet.h"
@@ -295,6 +296,46 @@ int smc_ib_get_memory_region(struct ib_pd *pd, int access_flags,
return 0;
}
+/* synchronize buffer usage for cpu access */
+void smc_ib_sync_sg_for_cpu(struct smc_ib_device *smcibdev,
+ struct smc_buf_desc *buf_slot,
+ enum dma_data_direction data_direction)
+{
+ struct scatterlist *sg;
+ unsigned int i;
+
+ /* for now there is just one DMA address */
+ for_each_sg(buf_slot->sgt[SMC_SINGLE_LINK].sgl, sg,
+ buf_slot->sgt[SMC_SINGLE_LINK].nents, i) {
+ if (!sg_dma_len(sg))
+ break;
+ ib_dma_sync_single_for_cpu(smcibdev->ibdev,
+ sg_dma_address(sg),
+ sg_dma_len(sg),
+ data_direction);
+ }
+}
+
+/* synchronize buffer usage for device access */
+void smc_ib_sync_sg_for_device(struct smc_ib_device *smcibdev,
+ struct smc_buf_desc *buf_slot,
+ enum dma_data_direction data_direction)
+{
+ struct scatterlist *sg;
+ unsigned int i;
+
+ /* for now there is just one DMA address */
+ for_each_sg(buf_slot->sgt[SMC_SINGLE_LINK].sgl, sg,
+ buf_slot->sgt[SMC_SINGLE_LINK].nents, i) {
+ if (!sg_dma_len(sg))
+ break;
+ ib_dma_sync_single_for_device(smcibdev->ibdev,
+ sg_dma_address(sg),
+ sg_dma_len(sg),
+ data_direction);
+ }
+}
+
/* Map a new TX or RX buffer SG-table to DMA */
int smc_ib_buf_map_sg(struct smc_ib_device *smcibdev,
struct smc_buf_desc *buf_slot,
diff --git a/net/smc/smc_ib.h b/net/smc/smc_ib.h
index 72acb19ffc67..9b927a33d5e6 100644
--- a/net/smc/smc_ib.h
+++ b/net/smc/smc_ib.h
@@ -68,4 +68,10 @@ long smc_ib_setup_per_ibdev(struct smc_ib_device *smcibdev);
int smc_ib_get_memory_region(struct ib_pd *pd, int access_flags,
struct smc_buf_desc *buf_slot);
void smc_ib_put_memory_region(struct ib_mr *mr);
+void smc_ib_sync_sg_for_cpu(struct smc_ib_device *smcibdev,
+ struct smc_buf_desc *buf_slot,
+ enum dma_data_direction data_direction);
+void smc_ib_sync_sg_for_device(struct smc_ib_device *smcibdev,
+ struct smc_buf_desc *buf_slot,
+ enum dma_data_direction data_direction);
#endif
diff --git a/net/smc/smc_rx.c b/net/smc/smc_rx.c
index f0c8b089f770..b17a333e9bb0 100644
--- a/net/smc/smc_rx.c
+++ b/net/smc/smc_rx.c
@@ -170,6 +170,7 @@ int smc_rx_recvmsg(struct smc_sock *smc, struct msghdr *msg, size_t len,
copylen, conn->rmbe_size - cons.count);
chunk_len_sum = chunk_len;
chunk_off = cons.count;
+ smc_rmb_sync_sg_for_cpu(conn);
for (chunk = 0; chunk < 2; chunk++) {
if (!(flags & MSG_TRUNC)) {
rc = memcpy_to_msg(msg, rcvbuf_base + chunk_off,
@@ -177,6 +178,7 @@ int smc_rx_recvmsg(struct smc_sock *smc, struct msghdr *msg, size_t len,
if (rc) {
if (!read_done)
read_done = -EFAULT;
+ smc_rmb_sync_sg_for_device(conn);
goto out;
}
}
@@ -190,6 +192,7 @@ int smc_rx_recvmsg(struct smc_sock *smc, struct msghdr *msg, size_t len,
chunk_len_sum += chunk_len;
chunk_off = 0; /* modulo offset in recv ring buffer */
}
+ smc_rmb_sync_sg_for_device(conn);
/* update cursors */
if (!(flags & MSG_PEEK)) {
diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index f4d58e2dd559..3c656beb8820 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -174,10 +174,12 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len)
copylen, conn->sndbuf_size - tx_cnt_prep);
chunk_len_sum = chunk_len;
chunk_off = tx_cnt_prep;
+ smc_sndbuf_sync_sg_for_cpu(conn);
for (chunk = 0; chunk < 2; chunk++) {
rc = memcpy_from_msg(sndbuf_base + chunk_off,
msg, chunk_len);
if (rc) {
+ smc_sndbuf_sync_sg_for_device(conn);
if (send_done)
return send_done;
goto out_err;
@@ -192,6 +194,7 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len)
chunk_len_sum += chunk_len;
chunk_off = 0; /* modulo offset in send ring buffer */
}
+ smc_sndbuf_sync_sg_for_device(conn);
/* update cursors */
smc_curs_add(conn->sndbuf_size, &prep, copylen);
smc_curs_write(&conn->tx_curs_prep,
--
2.11.2
^ permalink raw reply related
* [PATCH net-next 07/10] net/smc: introduce sg-logic for send buffers
From: Ursula Braun @ 2017-07-28 11:56 UTC (permalink / raw)
To: davem
Cc: hch, netdev, linux-rdma, linux-s390, jwi, schwidefsky,
heiko.carstens, raspl, ubraun
In-Reply-To: <20170728115622.17247-1-ubraun@linux.vnet.ibm.com>
SMC send buffers are processed the same way as RMBs. Since RMBs have
been converted to sg-logic, do the same for send buffers.
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
net/smc/smc_core.c | 46 +++++++++++++++++++++++++++++++++++-----------
net/smc/smc_core.h | 2 --
net/smc/smc_ib.c | 29 -----------------------------
net/smc/smc_ib.h | 6 ------
net/smc/smc_tx.c | 6 +++---
5 files changed, 38 insertions(+), 51 deletions(-)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 87bb3e4771a8..8795c7ed9ce4 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -248,6 +248,7 @@ static void smc_link_clear(struct smc_link *lnk)
static void smc_lgr_free_sndbufs(struct smc_link_group *lgr)
{
+ struct smc_link *lnk = &lgr->lnk[SMC_SINGLE_LINK];
struct smc_buf_desc *sndbuf_desc, *bf_desc;
int i;
@@ -255,10 +256,11 @@ static void smc_lgr_free_sndbufs(struct smc_link_group *lgr)
list_for_each_entry_safe(sndbuf_desc, bf_desc, &lgr->sndbufs[i],
list) {
list_del(&sndbuf_desc->list);
- smc_ib_buf_unmap(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
- smc_uncompress_bufsize(i),
- sndbuf_desc, DMA_TO_DEVICE);
- kfree(sndbuf_desc->cpu_addr);
+ smc_ib_buf_unmap_sg(lnk->smcibdev, sndbuf_desc,
+ DMA_TO_DEVICE);
+ sg_free_table(&sndbuf_desc->sgt[SMC_SINGLE_LINK]);
+ free_pages((unsigned long)sndbuf_desc->cpu_addr,
+ sndbuf_desc->order);
kfree(sndbuf_desc);
}
}
@@ -517,6 +519,9 @@ int smc_sndbuf_create(struct smc_sock *smc)
for (bufsize_short = smc_compress_bufsize(smc->sk.sk_sndbuf / 2);
bufsize_short >= 0; bufsize_short--) {
bufsize = smc_uncompress_bufsize(bufsize_short);
+ if ((1 << get_order(bufsize)) > SG_MAX_SINGLE_ALLOC)
+ continue;
+
/* check for reusable sndbuf_slot in the link group */
sndbuf_desc = smc_sndbuf_get_slot(lgr, bufsize_short);
if (sndbuf_desc) {
@@ -527,10 +532,12 @@ int smc_sndbuf_create(struct smc_sock *smc)
sndbuf_desc = kzalloc(sizeof(*sndbuf_desc), GFP_KERNEL);
if (!sndbuf_desc)
break; /* give up with -ENOMEM */
- sndbuf_desc->cpu_addr = kzalloc(bufsize,
- GFP_KERNEL | __GFP_NOWARN |
- __GFP_NOMEMALLOC |
- __GFP_NORETRY);
+
+ sndbuf_desc->cpu_addr =
+ (void *)__get_free_pages(GFP_KERNEL | __GFP_NOWARN |
+ __GFP_NOMEMALLOC |
+ __GFP_NORETRY | __GFP_ZERO,
+ get_order(bufsize));
if (!sndbuf_desc->cpu_addr) {
kfree(sndbuf_desc);
sndbuf_desc = NULL;
@@ -539,14 +546,31 @@ int smc_sndbuf_create(struct smc_sock *smc)
*/
continue;
}
- rc = smc_ib_buf_map(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
- bufsize, sndbuf_desc, DMA_TO_DEVICE);
+ sndbuf_desc->order = get_order(bufsize);
+
+ rc = sg_alloc_table(&sndbuf_desc->sgt[SMC_SINGLE_LINK], 1,
+ GFP_KERNEL);
if (rc) {
- kfree(sndbuf_desc->cpu_addr);
+ free_pages((unsigned long)sndbuf_desc->cpu_addr,
+ sndbuf_desc->order);
+ kfree(sndbuf_desc);
+ sndbuf_desc = NULL;
+ continue;
+ }
+ sg_set_buf(sndbuf_desc->sgt[SMC_SINGLE_LINK].sgl,
+ sndbuf_desc->cpu_addr, bufsize);
+
+ rc = smc_ib_buf_map_sg(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
+ sndbuf_desc, DMA_TO_DEVICE);
+ if (rc != 1) {
+ sg_free_table(&sndbuf_desc->sgt[SMC_SINGLE_LINK]);
+ free_pages((unsigned long)sndbuf_desc->cpu_addr,
+ sndbuf_desc->order);
kfree(sndbuf_desc);
sndbuf_desc = NULL;
continue; /* if mapping failed, try smaller one */
}
+
sndbuf_desc->used = 1;
write_lock_bh(&lgr->sndbufs_lock);
list_add(&sndbuf_desc->list, &lgr->sndbufs[bufsize_short]);
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index f7b40bdbf24c..72c25cb3eb89 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -102,8 +102,6 @@ struct smc_link {
/* tx/rx buffer list element for sndbufs list and rmbs list of a lgr */
struct smc_buf_desc {
struct list_head list;
- u64 dma_addr[SMC_LINKS_PER_LGR_MAX];
- /* mapped address of buffer */
void *cpu_addr; /* virtual address of buffer */
struct sg_table sgt[SMC_LINKS_PER_LGR_MAX];/* virtual buffer */
struct ib_mr *mr_rx[SMC_LINKS_PER_LGR_MAX];
diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
index 85e1831f591e..021f061609f5 100644
--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c
@@ -295,35 +295,6 @@ int smc_ib_get_memory_region(struct ib_pd *pd, int access_flags,
return 0;
}
-/* map a new TX or RX buffer to DMA */
-int smc_ib_buf_map(struct smc_ib_device *smcibdev, int buf_size,
- struct smc_buf_desc *buf_slot,
- enum dma_data_direction data_direction)
-{
- int rc = 0;
-
- if (buf_slot->dma_addr[SMC_SINGLE_LINK])
- return rc; /* already mapped */
- buf_slot->dma_addr[SMC_SINGLE_LINK] =
- ib_dma_map_single(smcibdev->ibdev, buf_slot->cpu_addr,
- buf_size, data_direction);
- if (ib_dma_mapping_error(smcibdev->ibdev,
- buf_slot->dma_addr[SMC_SINGLE_LINK]))
- rc = -EIO;
- return rc;
-}
-
-void smc_ib_buf_unmap(struct smc_ib_device *smcibdev, int buf_size,
- struct smc_buf_desc *buf_slot,
- enum dma_data_direction data_direction)
-{
- if (!buf_slot->dma_addr[SMC_SINGLE_LINK])
- return; /* already unmapped */
- ib_dma_unmap_single(smcibdev->ibdev, *buf_slot->dma_addr, buf_size,
- data_direction);
- buf_slot->dma_addr[SMC_SINGLE_LINK] = 0;
-}
-
/* Map a new TX or RX buffer SG-table to DMA */
int smc_ib_buf_map_sg(struct smc_ib_device *smcibdev,
struct smc_buf_desc *buf_slot,
diff --git a/net/smc/smc_ib.h b/net/smc/smc_ib.h
index b57d29f29042..72acb19ffc67 100644
--- a/net/smc/smc_ib.h
+++ b/net/smc/smc_ib.h
@@ -51,12 +51,6 @@ int smc_ib_register_client(void) __init;
void smc_ib_unregister_client(void);
bool smc_ib_port_active(struct smc_ib_device *smcibdev, u8 ibport);
int smc_ib_remember_port_attr(struct smc_ib_device *smcibdev, u8 ibport);
-int smc_ib_buf_map(struct smc_ib_device *smcibdev, int buf_size,
- struct smc_buf_desc *buf_slot,
- enum dma_data_direction data_direction);
-void smc_ib_buf_unmap(struct smc_ib_device *smcibdev, int bufsize,
- struct smc_buf_desc *buf_slot,
- enum dma_data_direction data_direction);
int smc_ib_buf_map_sg(struct smc_ib_device *smcibdev,
struct smc_buf_desc *buf_slot,
enum dma_data_direction data_direction);
diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 21ec1832ab51..f4d58e2dd559 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -277,6 +277,7 @@ static int smc_tx_rdma_writes(struct smc_connection *conn)
struct smc_link_group *lgr = conn->lgr;
int to_send, rmbespace;
struct smc_link *link;
+ dma_addr_t dma_addr;
int num_sges;
int rc;
@@ -334,12 +335,11 @@ static int smc_tx_rdma_writes(struct smc_connection *conn)
src_len = conn->sndbuf_size - sent.count;
}
src_len_sum = src_len;
+ dma_addr = sg_dma_address(conn->sndbuf_desc->sgt[SMC_SINGLE_LINK].sgl);
for (dstchunk = 0; dstchunk < 2; dstchunk++) {
num_sges = 0;
for (srcchunk = 0; srcchunk < 2; srcchunk++) {
- sges[srcchunk].addr =
- conn->sndbuf_desc->dma_addr[SMC_SINGLE_LINK] +
- src_off;
+ sges[srcchunk].addr = dma_addr + src_off;
sges[srcchunk].length = src_len;
sges[srcchunk].lkey = link->roce_pd->local_dma_lkey;
num_sges++;
--
2.11.2
^ permalink raw reply related
* [PATCH net-next 06/10] net/smc: remove Kconfig warning
From: Ursula Braun @ 2017-07-28 11:56 UTC (permalink / raw)
To: davem
Cc: hch, netdev, linux-rdma, linux-s390, jwi, schwidefsky,
heiko.carstens, raspl, ubraun
In-Reply-To: <20170728115622.17247-1-ubraun@linux.vnet.ibm.com>
Now separate memory regions are created and registered for separate
RMBs. The unsafe_global_rkey of the protection domain is no longer
used. Thus the exposing memory warning can be removed.
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
net/smc/Kconfig | 4 ----
1 file changed, 4 deletions(-)
diff --git a/net/smc/Kconfig b/net/smc/Kconfig
index 33954852f3f8..c717ef0896aa 100644
--- a/net/smc/Kconfig
+++ b/net/smc/Kconfig
@@ -8,10 +8,6 @@ config SMC
The Linux implementation of the SMC-R solution is designed as
a separate socket family SMC.
- Warning: SMC will expose all memory for remote reads and writes
- once a connection is established. Don't enable this option except
- for tightly controlled lab environment.
-
Select this option if you want to run SMC socket applications
config SMC_DIAG
--
2.11.2
^ permalink raw reply related
* [PATCH net-next 05/10] net/smc: register RMB-related memory region
From: Ursula Braun @ 2017-07-28 11:56 UTC (permalink / raw)
To: davem
Cc: hch, netdev, linux-rdma, linux-s390, jwi, schwidefsky,
heiko.carstens, raspl, ubraun
In-Reply-To: <20170728115622.17247-1-ubraun@linux.vnet.ibm.com>
A memory region created for a new RMB must be registered explicitly,
before the peer can make use of it for remote DMA transfer.
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
net/smc/af_smc.c | 38 ++++++++++++++++++++++++++++++++
net/smc/smc_core.c | 1 -
net/smc/smc_core.h | 12 +++++++++++
net/smc/smc_ib.c | 2 +-
net/smc/smc_wr.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
net/smc/smc_wr.h | 1 +
6 files changed, 115 insertions(+), 2 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 120a7b9b4d8e..e0a95d50bf87 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -338,6 +338,12 @@ static int smc_clnt_conf_first_link(struct smc_sock *smc, union ib_gid *gid)
return SMC_CLC_DECL_INTERR;
smc_wr_remember_qp_attr(link);
+
+ rc = smc_wr_reg_send(link,
+ smc->conn.rmb_desc->mr_rx[SMC_SINGLE_LINK]);
+ if (rc)
+ return SMC_CLC_DECL_INTERR;
+
/* send CONFIRM LINK response over RoCE fabric */
rc = smc_llc_send_confirm_link(link,
link->smcibdev->mac[link->ibport - 1],
@@ -459,6 +465,18 @@ static int smc_connect_rdma(struct smc_sock *smc)
reason_code = SMC_CLC_DECL_INTERR;
goto decline_rdma_unlock;
}
+ } else {
+ struct smc_buf_desc *buf_desc = smc->conn.rmb_desc;
+
+ if (!buf_desc->reused) {
+ /* register memory region for new rmb */
+ rc = smc_wr_reg_send(link,
+ buf_desc->mr_rx[SMC_SINGLE_LINK]);
+ if (rc) {
+ reason_code = SMC_CLC_DECL_INTERR;
+ goto decline_rdma_unlock;
+ }
+ }
}
rc = smc_clc_send_confirm(smc);
@@ -692,6 +710,12 @@ static int smc_serv_conf_first_link(struct smc_sock *smc)
int rc;
link = &lgr->lnk[SMC_SINGLE_LINK];
+
+ rc = smc_wr_reg_send(link,
+ smc->conn.rmb_desc->mr_rx[SMC_SINGLE_LINK]);
+ if (rc)
+ return SMC_CLC_DECL_INTERR;
+
/* send CONFIRM LINK request to client over the RoCE fabric */
rc = smc_llc_send_confirm_link(link,
link->smcibdev->mac[link->ibport - 1],
@@ -803,6 +827,20 @@ static void smc_listen_work(struct work_struct *work)
smc_close_init(new_smc);
smc_rx_init(new_smc);
+ if (local_contact != SMC_FIRST_CONTACT) {
+ struct smc_buf_desc *buf_desc = new_smc->conn.rmb_desc;
+
+ if (!buf_desc->reused) {
+ /* register memory region for new rmb */
+ rc = smc_wr_reg_send(link,
+ buf_desc->mr_rx[SMC_SINGLE_LINK]);
+ if (rc) {
+ reason_code = SMC_CLC_DECL_INTERR;
+ goto decline_rdma;
+ }
+ }
+ }
+
rc = smc_clc_send_accept(new_smc, local_contact);
if (rc)
goto out_err;
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index f1dd4e1cd3e1..87bb3e4771a8 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -175,7 +175,6 @@ static int smc_lgr_create(struct smc_sock *smc, __be32 peer_in_addr,
rc = smc_wr_alloc_link_mem(lnk);
if (rc)
goto free_lgr;
- init_waitqueue_head(&lnk->wr_tx_wait);
rc = smc_ib_create_protection_domain(lnk);
if (rc)
goto free_link_mem;
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index 17b5fea09901..f7b40bdbf24c 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -37,6 +37,14 @@ struct smc_wr_buf {
u8 raw[SMC_WR_BUF_SIZE];
};
+#define SMC_WR_REG_MR_WAIT_TIME (5 * HZ)/* wait time for ib_wr_reg_mr result */
+
+enum smc_wr_reg_state {
+ POSTED, /* ib_wr_reg_mr request posted */
+ CONFIRMED, /* ib_wr_reg_mr response: successful */
+ FAILED /* ib_wr_reg_mr response: failure */
+};
+
struct smc_link {
struct smc_ib_device *smcibdev; /* ib-device */
u8 ibport; /* port - values 1 | 2 */
@@ -65,6 +73,10 @@ struct smc_link {
u64 wr_rx_id; /* seq # of last recv WR */
u32 wr_rx_cnt; /* number of WR recv buffers */
+ struct ib_reg_wr wr_reg; /* WR register memory region */
+ wait_queue_head_t wr_reg_wait; /* wait for wr_reg result */
+ enum smc_wr_reg_state wr_reg_state; /* state of wr_reg request */
+
union ib_gid gid; /* gid matching used vlan id */
u32 peer_qpn; /* QP number of peer */
enum ib_mtu path_mtu; /* used mtu */
diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
index 08233492ec45..85e1831f591e 100644
--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c
@@ -231,10 +231,10 @@ int smc_ib_create_queue_pair(struct smc_link *lnk)
.recv_cq = lnk->smcibdev->roce_cq_recv,
.srq = NULL,
.cap = {
- .max_send_wr = SMC_WR_BUF_CNT,
/* include unsolicited rdma_writes as well,
* there are max. 2 RDMA_WRITE per 1 WR_SEND
*/
+ .max_send_wr = SMC_WR_BUF_CNT * 3,
.max_recv_wr = SMC_WR_BUF_CNT * 3,
.max_send_sge = SMC_IB_MAX_SEND_SGE,
.max_recv_sge = 1,
diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c
index 874ee9f9d796..ab56bda66783 100644
--- a/net/smc/smc_wr.c
+++ b/net/smc/smc_wr.c
@@ -68,6 +68,16 @@ static inline void smc_wr_tx_process_cqe(struct ib_wc *wc)
int i;
link = wc->qp->qp_context;
+
+ if (wc->opcode == IB_WC_REG_MR) {
+ if (wc->status)
+ link->wr_reg_state = FAILED;
+ else
+ link->wr_reg_state = CONFIRMED;
+ wake_up(&link->wr_reg_wait);
+ return;
+ }
+
pnd_snd_idx = smc_wr_tx_find_pending_index(link, wc->wr_id);
if (pnd_snd_idx == link->wr_tx_cnt)
return;
@@ -243,6 +253,52 @@ int smc_wr_tx_send(struct smc_link *link, struct smc_wr_tx_pend_priv *priv)
return rc;
}
+/* Register a memory region and wait for result. */
+int smc_wr_reg_send(struct smc_link *link, struct ib_mr *mr)
+{
+ struct ib_send_wr *failed_wr = NULL;
+ int rc;
+
+ ib_req_notify_cq(link->smcibdev->roce_cq_send,
+ IB_CQ_NEXT_COMP | IB_CQ_REPORT_MISSED_EVENTS);
+ link->wr_reg_state = POSTED;
+ link->wr_reg.wr.wr_id = (u64)(uintptr_t)mr;
+ link->wr_reg.mr = mr;
+ link->wr_reg.key = mr->rkey;
+ failed_wr = &link->wr_reg.wr;
+ rc = ib_post_send(link->roce_qp, &link->wr_reg.wr, &failed_wr);
+ WARN_ON(failed_wr != &link->wr_reg.wr);
+ if (rc)
+ return rc;
+
+ rc = wait_event_interruptible_timeout(link->wr_reg_wait,
+ (link->wr_reg_state != POSTED),
+ SMC_WR_REG_MR_WAIT_TIME);
+ if (!rc) {
+ /* timeout - terminate connections */
+ struct smc_link_group *lgr;
+
+ lgr = container_of(link, struct smc_link_group,
+ lnk[SMC_SINGLE_LINK]);
+ smc_lgr_terminate(lgr);
+ return -EPIPE;
+ }
+ if (rc == -ERESTARTSYS)
+ return -EINTR;
+ switch (link->wr_reg_state) {
+ case CONFIRMED:
+ rc = 0;
+ break;
+ case FAILED:
+ rc = -EIO;
+ break;
+ case POSTED:
+ rc = -EPIPE;
+ break;
+ }
+ return rc;
+}
+
void smc_wr_tx_dismiss_slots(struct smc_link *link, u8 wr_rx_hdr_type,
smc_wr_tx_filter filter,
smc_wr_tx_dismisser dismisser,
@@ -458,6 +514,11 @@ static void smc_wr_init_sge(struct smc_link *lnk)
lnk->wr_rx_ibs[i].sg_list = &lnk->wr_rx_sges[i];
lnk->wr_rx_ibs[i].num_sge = 1;
}
+ lnk->wr_reg.wr.next = NULL;
+ lnk->wr_reg.wr.num_sge = 0;
+ lnk->wr_reg.wr.send_flags = IB_SEND_SIGNALED;
+ lnk->wr_reg.wr.opcode = IB_WR_REG_MR;
+ lnk->wr_reg.access = IB_ACCESS_LOCAL_WRITE | IB_ACCESS_REMOTE_WRITE;
}
void smc_wr_free_link(struct smc_link *lnk)
@@ -602,6 +663,8 @@ int smc_wr_create_link(struct smc_link *lnk)
smc_wr_init_sge(lnk);
memset(lnk->wr_tx_mask, 0,
BITS_TO_LONGS(SMC_WR_BUF_CNT) * sizeof(*lnk->wr_tx_mask));
+ init_waitqueue_head(&lnk->wr_tx_wait);
+ init_waitqueue_head(&lnk->wr_reg_wait);
return rc;
dma_unmap:
diff --git a/net/smc/smc_wr.h b/net/smc/smc_wr.h
index 0b9beeda6053..45eb53833052 100644
--- a/net/smc/smc_wr.h
+++ b/net/smc/smc_wr.h
@@ -102,5 +102,6 @@ void smc_wr_tx_dismiss_slots(struct smc_link *lnk, u8 wr_rx_hdr_type,
int smc_wr_rx_register_handler(struct smc_wr_rx_handler *handler);
int smc_wr_rx_post_init(struct smc_link *link);
void smc_wr_rx_cq_handler(struct ib_cq *ib_cq, void *cq_context);
+int smc_wr_reg_send(struct smc_link *link, struct ib_mr *mr);
#endif /* SMC_WR_H */
--
2.11.2
^ permalink raw reply related
* [PATCH net-next 04/10] net/smc: use separate memory regions for RMBs
From: Ursula Braun @ 2017-07-28 11:56 UTC (permalink / raw)
To: davem
Cc: hch, netdev, linux-rdma, linux-s390, jwi, schwidefsky,
heiko.carstens, raspl, ubraun
In-Reply-To: <20170728115622.17247-1-ubraun@linux.vnet.ibm.com>
SMC currently uses the unsafe_global_rkey of the protection domain,
which exposes all memory for remote reads and writes once a connection
is established. This patch introduces separate memory regions with
separate rkeys for every RMB. Now the unsafe_global_rkey of the
protection domain is no longer needed.
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
net/smc/smc_clc.c | 4 ++--
net/smc/smc_core.c | 18 ++++++++++++++++++
net/smc/smc_core.h | 6 +++++-
net/smc/smc_ib.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
net/smc/smc_ib.h | 5 +++--
5 files changed, 71 insertions(+), 7 deletions(-)
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 15cb76019009..3934913ab835 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -204,7 +204,7 @@ int smc_clc_send_confirm(struct smc_sock *smc)
memcpy(&cclc.lcl.mac, &link->smcibdev->mac[link->ibport - 1], ETH_ALEN);
hton24(cclc.qpn, link->roce_qp->qp_num);
cclc.rmb_rkey =
- htonl(link->roce_pd->unsafe_global_rkey);
+ htonl(conn->rmb_desc->mr_rx[SMC_SINGLE_LINK]->rkey);
cclc.conn_idx = 1; /* for now: 1 RMB = 1 RMBE */
cclc.rmbe_alert_token = htonl(conn->alert_token_local);
cclc.qp_mtu = min(link->path_mtu, link->peer_mtu);
@@ -256,7 +256,7 @@ int smc_clc_send_accept(struct smc_sock *new_smc, int srv_first_contact)
memcpy(&aclc.lcl.mac, link->smcibdev->mac[link->ibport - 1], ETH_ALEN);
hton24(aclc.qpn, link->roce_qp->qp_num);
aclc.rmb_rkey =
- htonl(link->roce_pd->unsafe_global_rkey);
+ htonl(conn->rmb_desc->mr_rx[SMC_SINGLE_LINK]->rkey);
aclc.conn_idx = 1; /* as long as 1 RMB = 1 RMBE */
aclc.rmbe_alert_token = htonl(conn->alert_token_local);
aclc.qp_mtu = link->path_mtu;
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index bfdbda795f67..f1dd4e1cd3e1 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -218,6 +218,7 @@ static void smc_sndbuf_unuse(struct smc_connection *conn)
static void smc_rmb_unuse(struct smc_connection *conn)
{
if (conn->rmb_desc) {
+ conn->rmb_desc->reused = true;
conn->rmb_desc->used = 0;
conn->rmbe_size = 0;
}
@@ -274,6 +275,8 @@ static void smc_lgr_free_rmbs(struct smc_link_group *lgr)
list_for_each_entry_safe(rmb_desc, bf_desc, &lgr->rmbs[i],
list) {
list_del(&rmb_desc->list);
+ smc_ib_put_memory_region(
+ rmb_desc->mr_rx[SMC_SINGLE_LINK]);
smc_ib_buf_unmap_sg(lnk->smcibdev, rmb_desc,
DMA_FROM_DEVICE);
kfree(rmb_desc->cpu_addr);
@@ -627,6 +630,21 @@ int smc_rmb_create(struct smc_sock *smc)
rmb_desc = NULL;
continue; /* if mapping failed, try smaller one */
}
+ rc = smc_ib_get_memory_region(lgr->lnk[SMC_SINGLE_LINK].roce_pd,
+ IB_ACCESS_REMOTE_WRITE |
+ IB_ACCESS_LOCAL_WRITE,
+ rmb_desc);
+ if (rc) {
+ smc_ib_buf_unmap_sg(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
+ rmb_desc, DMA_FROM_DEVICE);
+ sg_free_table(&rmb_desc->sgt[SMC_SINGLE_LINK]);
+ free_pages((unsigned long)rmb_desc->cpu_addr,
+ rmb_desc->order);
+ kfree(rmb_desc);
+ rmb_desc = NULL;
+ continue;
+ }
+
rmb_desc->used = 1;
write_lock_bh(&lgr->rmbs_lock);
list_add(&rmb_desc->list, &lgr->rmbs[bufsize_short]);
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index 0ee450d69907..17b5fea09901 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -94,8 +94,13 @@ struct smc_buf_desc {
/* mapped address of buffer */
void *cpu_addr; /* virtual address of buffer */
struct sg_table sgt[SMC_LINKS_PER_LGR_MAX];/* virtual buffer */
+ struct ib_mr *mr_rx[SMC_LINKS_PER_LGR_MAX];
+ /* for rmb only: memory region
+ * incl. rkey provided to peer
+ */
u32 order; /* allocation order */
u32 used; /* currently used / unused */
+ bool reused; /* new created / reused */
};
struct smc_rtoken { /* address/key of remote RMB */
@@ -175,5 +180,4 @@ int smc_sndbuf_create(struct smc_sock *smc);
int smc_rmb_create(struct smc_sock *smc);
int smc_rmb_rtoken_handling(struct smc_connection *conn,
struct smc_clc_msg_accept_confirm *clc);
-
#endif
diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
index fcfeb89b05d9..08233492ec45 100644
--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c
@@ -192,8 +192,7 @@ int smc_ib_create_protection_domain(struct smc_link *lnk)
{
int rc;
- lnk->roce_pd = ib_alloc_pd(lnk->smcibdev->ibdev,
- IB_PD_UNSAFE_GLOBAL_RKEY);
+ lnk->roce_pd = ib_alloc_pd(lnk->smcibdev->ibdev, 0);
rc = PTR_ERR_OR_ZERO(lnk->roce_pd);
if (IS_ERR(lnk->roce_pd))
lnk->roce_pd = NULL;
@@ -254,6 +253,48 @@ int smc_ib_create_queue_pair(struct smc_link *lnk)
return rc;
}
+void smc_ib_put_memory_region(struct ib_mr *mr)
+{
+ ib_dereg_mr(mr);
+}
+
+static int smc_ib_map_mr_sg(struct smc_buf_desc *buf_slot)
+{
+ unsigned int offset = 0;
+ int sg_num;
+
+ /* map the largest prefix of a dma mapped SG list */
+ sg_num = ib_map_mr_sg(buf_slot->mr_rx[SMC_SINGLE_LINK],
+ buf_slot->sgt[SMC_SINGLE_LINK].sgl,
+ buf_slot->sgt[SMC_SINGLE_LINK].orig_nents,
+ &offset, PAGE_SIZE);
+
+ return sg_num;
+}
+
+/* Allocate a memory region and map the dma mapped SG list of buf_slot */
+int smc_ib_get_memory_region(struct ib_pd *pd, int access_flags,
+ struct smc_buf_desc *buf_slot)
+{
+ if (buf_slot->mr_rx[SMC_SINGLE_LINK])
+ return 0; /* already done */
+
+ buf_slot->mr_rx[SMC_SINGLE_LINK] =
+ ib_alloc_mr(pd, IB_MR_TYPE_MEM_REG, 1 << buf_slot->order);
+ if (IS_ERR(buf_slot->mr_rx[SMC_SINGLE_LINK])) {
+ int rc;
+
+ rc = PTR_ERR(buf_slot->mr_rx[SMC_SINGLE_LINK]);
+ buf_slot->mr_rx[SMC_SINGLE_LINK] = NULL;
+ return rc;
+ }
+
+ if (smc_ib_map_mr_sg(buf_slot) != 1)
+ return -EINVAL;
+
+ return 0;
+}
+
/* map a new TX or RX buffer to DMA */
int smc_ib_buf_map(struct smc_ib_device *smcibdev, int buf_size,
struct smc_buf_desc *buf_slot,
diff --git a/net/smc/smc_ib.h b/net/smc/smc_ib.h
index b30e387854b6..b57d29f29042 100644
--- a/net/smc/smc_ib.h
+++ b/net/smc/smc_ib.h
@@ -71,6 +71,7 @@ int smc_ib_ready_link(struct smc_link *lnk);
int smc_ib_modify_qp_rts(struct smc_link *lnk);
int smc_ib_modify_qp_reset(struct smc_link *lnk);
long smc_ib_setup_per_ibdev(struct smc_ib_device *smcibdev);
-
-
+int smc_ib_get_memory_region(struct ib_pd *pd, int access_flags,
+ struct smc_buf_desc *buf_slot);
+void smc_ib_put_memory_region(struct ib_mr *mr);
#endif
--
2.11.2
^ permalink raw reply related
* [PATCH net-next 03/10] net/smc: introduce sg-logic for RMBs
From: Ursula Braun @ 2017-07-28 11:56 UTC (permalink / raw)
To: davem
Cc: hch, netdev, linux-rdma, linux-s390, jwi, schwidefsky,
heiko.carstens, raspl, ubraun
In-Reply-To: <20170728115622.17247-1-ubraun@linux.vnet.ibm.com>
The follow-on patch makes use of ib_map_mr_sg() when introducing
separate memory regions for RMBs. This function is based on
scatterlists; thus this patch introduces scatterlists for RMBs.
Signed-off-by: Ursula Braun <ubraun@linux.vnet.ibm.com>
---
net/smc/smc_clc.c | 12 ++++++------
net/smc/smc_core.c | 47 +++++++++++++++++++++++++++++++----------------
net/smc/smc_core.h | 6 ++----
net/smc/smc_ib.c | 31 +++++++++++++++++++++++++++++++
net/smc/smc_ib.h | 6 ++++++
5 files changed, 76 insertions(+), 26 deletions(-)
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 03ec058d18df..15cb76019009 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -204,13 +204,13 @@ int smc_clc_send_confirm(struct smc_sock *smc)
memcpy(&cclc.lcl.mac, &link->smcibdev->mac[link->ibport - 1], ETH_ALEN);
hton24(cclc.qpn, link->roce_qp->qp_num);
cclc.rmb_rkey =
- htonl(conn->rmb_desc->rkey[SMC_SINGLE_LINK]);
+ htonl(link->roce_pd->unsafe_global_rkey);
cclc.conn_idx = 1; /* for now: 1 RMB = 1 RMBE */
cclc.rmbe_alert_token = htonl(conn->alert_token_local);
cclc.qp_mtu = min(link->path_mtu, link->peer_mtu);
cclc.rmbe_size = conn->rmbe_size_short;
- cclc.rmb_dma_addr =
- cpu_to_be64((u64)conn->rmb_desc->dma_addr[SMC_SINGLE_LINK]);
+ cclc.rmb_dma_addr = cpu_to_be64(
+ (u64)sg_dma_address(conn->rmb_desc->sgt[SMC_SINGLE_LINK].sgl));
hton24(cclc.psn, link->psn_initial);
memcpy(cclc.trl.eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER));
@@ -256,13 +256,13 @@ int smc_clc_send_accept(struct smc_sock *new_smc, int srv_first_contact)
memcpy(&aclc.lcl.mac, link->smcibdev->mac[link->ibport - 1], ETH_ALEN);
hton24(aclc.qpn, link->roce_qp->qp_num);
aclc.rmb_rkey =
- htonl(conn->rmb_desc->rkey[SMC_SINGLE_LINK]);
+ htonl(link->roce_pd->unsafe_global_rkey);
aclc.conn_idx = 1; /* as long as 1 RMB = 1 RMBE */
aclc.rmbe_alert_token = htonl(conn->alert_token_local);
aclc.qp_mtu = link->path_mtu;
aclc.rmbe_size = conn->rmbe_size_short,
- aclc.rmb_dma_addr =
- cpu_to_be64((u64)conn->rmb_desc->dma_addr[SMC_SINGLE_LINK]);
+ aclc.rmb_dma_addr = cpu_to_be64(
+ (u64)sg_dma_address(conn->rmb_desc->sgt[SMC_SINGLE_LINK].sgl));
hton24(aclc.psn, link->psn_initial);
memcpy(aclc.trl.eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER));
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 6159488cb94b..bfdbda795f67 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -266,17 +266,16 @@ static void smc_lgr_free_sndbufs(struct smc_link_group *lgr)
static void smc_lgr_free_rmbs(struct smc_link_group *lgr)
{
- struct smc_buf_desc *rmb_desc, *bf_desc;
struct smc_link *lnk = &lgr->lnk[SMC_SINGLE_LINK];
+ struct smc_buf_desc *rmb_desc, *bf_desc;
int i;
for (i = 0; i < SMC_RMBE_SIZES; i++) {
list_for_each_entry_safe(rmb_desc, bf_desc, &lgr->rmbs[i],
list) {
list_del(&rmb_desc->list);
- smc_ib_buf_unmap(lnk->smcibdev,
- smc_uncompress_bufsize(i),
- rmb_desc, DMA_FROM_DEVICE);
+ smc_ib_buf_unmap_sg(lnk->smcibdev, rmb_desc,
+ DMA_FROM_DEVICE);
kfree(rmb_desc->cpu_addr);
kfree(rmb_desc);
}
@@ -580,38 +579,54 @@ int smc_rmb_create(struct smc_sock *smc)
for (bufsize_short = smc_compress_bufsize(smc->sk.sk_rcvbuf / 2);
bufsize_short >= 0; bufsize_short--) {
bufsize = smc_uncompress_bufsize(bufsize_short);
+ if ((1 << get_order(bufsize)) > SG_MAX_SINGLE_ALLOC)
+ continue;
+
/* check for reusable rmb_slot in the link group */
rmb_desc = smc_rmb_get_slot(lgr, bufsize_short);
if (rmb_desc) {
memset(rmb_desc->cpu_addr, 0, bufsize);
break; /* found reusable slot */
}
+
/* try to alloc a new RMB */
rmb_desc = kzalloc(sizeof(*rmb_desc), GFP_KERNEL);
if (!rmb_desc)
break; /* give up with -ENOMEM */
- rmb_desc->cpu_addr = kzalloc(bufsize,
- GFP_KERNEL | __GFP_NOWARN |
- __GFP_NOMEMALLOC |
- __GFP_NORETRY);
+ rmb_desc->cpu_addr =
+ (void *)__get_free_pages(GFP_KERNEL | __GFP_NOWARN |
+ __GFP_NOMEMALLOC |
+ __GFP_NORETRY | __GFP_ZERO,
+ get_order(bufsize));
if (!rmb_desc->cpu_addr) {
kfree(rmb_desc);
rmb_desc = NULL;
- /* if RMB allocation has failed,
- * try a smaller one
- */
continue;
}
- rc = smc_ib_buf_map(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
- bufsize, rmb_desc, DMA_FROM_DEVICE);
+ rmb_desc->order = get_order(bufsize);
+
+ rc = sg_alloc_table(&rmb_desc->sgt[SMC_SINGLE_LINK], 1,
+ GFP_KERNEL);
if (rc) {
- kfree(rmb_desc->cpu_addr);
+ free_pages((unsigned long)rmb_desc->cpu_addr,
+ rmb_desc->order);
+ kfree(rmb_desc);
+ rmb_desc = NULL;
+ continue;
+ }
+ sg_set_buf(rmb_desc->sgt[SMC_SINGLE_LINK].sgl,
+ rmb_desc->cpu_addr, bufsize);
+
+ rc = smc_ib_buf_map_sg(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
+ rmb_desc, DMA_FROM_DEVICE);
+ if (rc != 1) {
+ sg_free_table(&rmb_desc->sgt[SMC_SINGLE_LINK]);
+ free_pages((unsigned long)rmb_desc->cpu_addr,
+ rmb_desc->order);
kfree(rmb_desc);
rmb_desc = NULL;
continue; /* if mapping failed, try smaller one */
}
- rmb_desc->rkey[SMC_SINGLE_LINK] =
- lgr->lnk[SMC_SINGLE_LINK].roce_pd->unsafe_global_rkey;
rmb_desc->used = 1;
write_lock_bh(&lgr->rmbs_lock);
list_add(&rmb_desc->list, &lgr->rmbs[bufsize_short]);
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index b013cb43a327..0ee450d69907 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -93,10 +93,8 @@ struct smc_buf_desc {
u64 dma_addr[SMC_LINKS_PER_LGR_MAX];
/* mapped address of buffer */
void *cpu_addr; /* virtual address of buffer */
- u32 rkey[SMC_LINKS_PER_LGR_MAX];
- /* for rmb only:
- * rkey provided to peer
- */
+ struct sg_table sgt[SMC_LINKS_PER_LGR_MAX];/* virtual buffer */
+ u32 order; /* allocation order */
u32 used; /* currently used / unused */
};
diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
index b31715505a35..fcfeb89b05d9 100644
--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c
@@ -283,6 +283,37 @@ void smc_ib_buf_unmap(struct smc_ib_device *smcibdev, int buf_size,
buf_slot->dma_addr[SMC_SINGLE_LINK] = 0;
}
+/* Map a new TX or RX buffer SG-table to DMA */
+int smc_ib_buf_map_sg(struct smc_ib_device *smcibdev,
+ struct smc_buf_desc *buf_slot,
+ enum dma_data_direction data_direction)
+{
+ int mapped_nents;
+
+ mapped_nents = ib_dma_map_sg(smcibdev->ibdev,
+ buf_slot->sgt[SMC_SINGLE_LINK].sgl,
+ buf_slot->sgt[SMC_SINGLE_LINK].orig_nents,
+ data_direction);
+ if (!mapped_nents)
+ return -ENOMEM;
+
+ return mapped_nents;
+}
+
+void smc_ib_buf_unmap_sg(struct smc_ib_device *smcibdev,
+ struct smc_buf_desc *buf_slot,
+ enum dma_data_direction data_direction)
+{
+ if (!buf_slot->sgt[SMC_SINGLE_LINK].sgl->dma_address)
+ return; /* already unmapped */
+
+ ib_dma_unmap_sg(smcibdev->ibdev,
+ buf_slot->sgt[SMC_SINGLE_LINK].sgl,
+ buf_slot->sgt[SMC_SINGLE_LINK].orig_nents,
+ data_direction);
+ buf_slot->sgt[SMC_SINGLE_LINK].sgl->dma_address = 0;
+}
+
static int smc_ib_fill_gid_and_mac(struct smc_ib_device *smcibdev, u8 ibport)
{
struct net_device *ndev;
diff --git a/net/smc/smc_ib.h b/net/smc/smc_ib.h
index b567152a526d..b30e387854b6 100644
--- a/net/smc/smc_ib.h
+++ b/net/smc/smc_ib.h
@@ -57,6 +57,12 @@ int smc_ib_buf_map(struct smc_ib_device *smcibdev, int buf_size,
void smc_ib_buf_unmap(struct smc_ib_device *smcibdev, int bufsize,
struct smc_buf_desc *buf_slot,
enum dma_data_direction data_direction);
+int smc_ib_buf_map_sg(struct smc_ib_device *smcibdev,
+ struct smc_buf_desc *buf_slot,
+ enum dma_data_direction data_direction);
+void smc_ib_buf_unmap_sg(struct smc_ib_device *smcibdev,
+ struct smc_buf_desc *buf_slot,
+ enum dma_data_direction data_direction);
void smc_ib_dealloc_protection_domain(struct smc_link *lnk);
int smc_ib_create_protection_domain(struct smc_link *lnk);
void smc_ib_destroy_queue_pair(struct smc_link *lnk);
--
2.11.2
^ permalink raw reply related
* [PATCH net-next 00/10] net/smc: get rid of unsafe_global_rkey
From: Ursula Braun @ 2017-07-28 11:56 UTC (permalink / raw)
To: davem
Cc: hch, netdev, linux-rdma, linux-s390, jwi, schwidefsky,
heiko.carstens, raspl, ubraun
Dave,
The smc code uses the unsafe_global_rkey, exposing all memory for
remote reads and writes once a connection is established.
Here is now a patch series to get rid of unsafe_global_rkey usage.
Main idea is to switch to SG-logic and separate memory regions for RMBs.
Thanks, Ursula
Ursula Braun (10):
net/smc: serialize connection creation in all cases
net/smc: shorten local bufsize variables
net/smc: introduce sg-logic for RMBs
net/smc: use separate memory regions for RMBs
net/smc: register RMB-related memory region
net/smc: remove Kconfig warning
net/smc: introduce sg-logic for send buffers
net/smc: common functions for RMBs and send buffers
net/smc: cleanup function __smc_buf_create()
net/smc: synchronize buffer usage with device
net/smc/Kconfig | 4 -
net/smc/af_smc.c | 64 ++++++---
net/smc/smc_clc.c | 12 +-
net/smc/smc_core.c | 388 +++++++++++++++++++++++++++++------------------------
net/smc/smc_core.h | 31 +++--
net/smc/smc_ib.c | 128 +++++++++++++++---
net/smc/smc_ib.h | 19 ++-
net/smc/smc_rx.c | 3 +
net/smc/smc_tx.c | 9 +-
net/smc/smc_wr.c | 63 +++++++++
net/smc/smc_wr.h | 1 +
11 files changed, 477 insertions(+), 245 deletions(-)
--
2.11.2
^ permalink raw reply
* [PATCH net-next 09/10] net/smc: cleanup function __smc_buf_create()
From: Ursula Braun @ 2017-07-28 11:56 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: hch-jcswGhMUV9g, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA,
jwi-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
schwidefsky-tA70FqPdS9bQT0dZR+AlfA,
heiko.carstens-tA70FqPdS9bQT0dZR+AlfA,
raspl-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
In-Reply-To: <20170728115622.17247-1-ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Split function __smc_buf_create() for better readability.
Signed-off-by: Ursula Braun <ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
net/smc/smc_core.c | 114 +++++++++++++++++++++++++++++------------------------
1 file changed, 63 insertions(+), 51 deletions(-)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index ab8cdac5edb3..447bd52da0e2 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -487,6 +487,64 @@ static inline int smc_rmb_wnd_update_limit(int rmbe_size)
return min_t(int, rmbe_size / 10, SOCK_MIN_SNDBUF / 2);
}
+static struct smc_buf_desc *smc_new_buf_create(struct smc_link_group *lgr,
+ bool is_rmb, int bufsize)
+{
+ struct smc_buf_desc *buf_desc;
+ struct smc_link *lnk;
+ int rc;
+
+ /* try to alloc a new buffer */
+ buf_desc = kzalloc(sizeof(*buf_desc), GFP_KERNEL);
+ if (!buf_desc)
+ return ERR_PTR(-ENOMEM);
+
+ buf_desc->cpu_addr =
+ (void *)__get_free_pages(GFP_KERNEL | __GFP_NOWARN |
+ __GFP_NOMEMALLOC |
+ __GFP_NORETRY | __GFP_ZERO,
+ get_order(bufsize));
+ if (!buf_desc->cpu_addr) {
+ kfree(buf_desc);
+ return ERR_PTR(-EAGAIN);
+ }
+ buf_desc->order = get_order(bufsize);
+
+ /* build the sg table from the pages */
+ lnk = &lgr->lnk[SMC_SINGLE_LINK];
+ rc = sg_alloc_table(&buf_desc->sgt[SMC_SINGLE_LINK], 1,
+ GFP_KERNEL);
+ if (rc) {
+ smc_buf_free(buf_desc, lnk, is_rmb);
+ return ERR_PTR(rc);
+ }
+ sg_set_buf(buf_desc->sgt[SMC_SINGLE_LINK].sgl,
+ buf_desc->cpu_addr, bufsize);
+
+ /* map sg table to DMA address */
+ rc = smc_ib_buf_map_sg(lnk->smcibdev, buf_desc,
+ is_rmb ? DMA_FROM_DEVICE : DMA_TO_DEVICE);
+ /* SMC protocol depends on mapping to one DMA address only */
+ if (rc != 1) {
+ smc_buf_free(buf_desc, lnk, is_rmb);
+ return ERR_PTR(-EAGAIN);
+ }
+
+ /* create a new memory region for the RMB */
+ if (is_rmb) {
+ rc = smc_ib_get_memory_region(lnk->roce_pd,
+ IB_ACCESS_REMOTE_WRITE |
+ IB_ACCESS_LOCAL_WRITE,
+ buf_desc);
+ if (rc) {
+ smc_buf_free(buf_desc, lnk, is_rmb);
+ return ERR_PTR(rc);
+ }
+ }
+
+ return buf_desc;
+}
+
static int __smc_buf_create(struct smc_sock *smc, bool is_rmb)
{
struct smc_connection *conn = &smc->conn;
@@ -494,12 +552,9 @@ static int __smc_buf_create(struct smc_sock *smc, bool is_rmb)
struct smc_buf_desc *buf_desc = NULL;
struct list_head *buf_list;
int bufsize, bufsize_short;
- struct smc_link *lnk;
int sk_buf_size;
rwlock_t *lock;
- int rc;
- lnk = &lgr->lnk[SMC_SINGLE_LINK];
if (is_rmb)
/* use socket recv buffer size (w/o overhead) as start value */
sk_buf_size = smc->sk.sk_rcvbuf / 2;
@@ -528,54 +583,11 @@ static int __smc_buf_create(struct smc_sock *smc, bool is_rmb)
break; /* found reusable slot */
}
- /* try to allocate the determined number of pages */
- buf_desc = kzalloc(sizeof(*buf_desc), GFP_KERNEL);
- if (!buf_desc)
- break; /* give up with -ENOMEM */
-
- buf_desc->cpu_addr =
- (void *)__get_free_pages(GFP_KERNEL | __GFP_NOWARN |
- __GFP_NOMEMALLOC |
- __GFP_NORETRY | __GFP_ZERO,
- get_order(bufsize));
- if (!buf_desc->cpu_addr) {
- kfree(buf_desc);
- buf_desc = NULL;
- continue;
- }
-
- rc = sg_alloc_table(&buf_desc->sgt[SMC_SINGLE_LINK], 1,
- GFP_KERNEL);
- if (rc) {
- smc_buf_free(buf_desc, lnk, is_rmb);
- buf_desc = NULL;
+ buf_desc = smc_new_buf_create(lgr, is_rmb, bufsize);
+ if (PTR_ERR(buf_desc) == -ENOMEM)
+ break;
+ if (IS_ERR(buf_desc))
continue;
- }
- sg_set_buf(buf_desc->sgt[SMC_SINGLE_LINK].sgl,
- buf_desc->cpu_addr, bufsize);
-
- /* map sg table to DMA address */
- rc = smc_ib_buf_map_sg(lnk->smcibdev, buf_desc, is_rmb ?
- DMA_FROM_DEVICE : DMA_TO_DEVICE);
- /* SMC protocol depends on mapping to one DMA address only */
- if (rc != 1) {
- smc_buf_free(buf_desc, lnk, is_rmb);
- buf_desc = NULL;
- continue; /* if mapping failed, try smaller one */
- }
-
- /* create a new memory region for the RMB */
- if (is_rmb) {
- rc = smc_ib_get_memory_region(lnk->roce_pd,
- IB_ACCESS_REMOTE_WRITE |
- IB_ACCESS_LOCAL_WRITE,
- buf_desc);
- if (rc) {
- smc_buf_free(buf_desc, lnk, is_rmb);
- buf_desc = NULL;
- continue;
- }
- }
buf_desc->used = 1;
write_lock_bh(lock);
@@ -584,7 +596,7 @@ static int __smc_buf_create(struct smc_sock *smc, bool is_rmb)
break; /* found */
}
- if (!buf_desc || !buf_desc->cpu_addr)
+ if (IS_ERR(buf_desc))
return -ENOMEM;
if (is_rmb) {
--
2.11.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 02/10] net/smc: shorten local bufsize variables
From: Ursula Braun @ 2017-07-28 11:56 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: hch-jcswGhMUV9g, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA,
jwi-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
schwidefsky-tA70FqPdS9bQT0dZR+AlfA,
heiko.carstens-tA70FqPdS9bQT0dZR+AlfA,
raspl-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
In-Reply-To: <20170728115622.17247-1-ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Initiate the coming rework of SMC buffer handling with this
small code cleanup. No functional changes here.
Signed-off-by: Ursula Braun <ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
net/smc/smc_core.c | 54 +++++++++++++++++++++++++-----------------------------
1 file changed, 25 insertions(+), 29 deletions(-)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 3ac09a629ea1..6159488cb94b 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -508,25 +508,25 @@ int smc_sndbuf_create(struct smc_sock *smc)
{
struct smc_connection *conn = &smc->conn;
struct smc_link_group *lgr = conn->lgr;
- int tmp_bufsize, tmp_bufsize_short;
struct smc_buf_desc *sndbuf_desc;
+ int bufsize, bufsize_short;
int rc;
/* use socket send buffer size (w/o overhead) as start value */
- for (tmp_bufsize_short = smc_compress_bufsize(smc->sk.sk_sndbuf / 2);
- tmp_bufsize_short >= 0; tmp_bufsize_short--) {
- tmp_bufsize = smc_uncompress_bufsize(tmp_bufsize_short);
+ for (bufsize_short = smc_compress_bufsize(smc->sk.sk_sndbuf / 2);
+ bufsize_short >= 0; bufsize_short--) {
+ bufsize = smc_uncompress_bufsize(bufsize_short);
/* check for reusable sndbuf_slot in the link group */
- sndbuf_desc = smc_sndbuf_get_slot(lgr, tmp_bufsize_short);
+ sndbuf_desc = smc_sndbuf_get_slot(lgr, bufsize_short);
if (sndbuf_desc) {
- memset(sndbuf_desc->cpu_addr, 0, tmp_bufsize);
+ memset(sndbuf_desc->cpu_addr, 0, bufsize);
break; /* found reusable slot */
}
/* try to alloc a new send buffer */
sndbuf_desc = kzalloc(sizeof(*sndbuf_desc), GFP_KERNEL);
if (!sndbuf_desc)
break; /* give up with -ENOMEM */
- sndbuf_desc->cpu_addr = kzalloc(tmp_bufsize,
+ sndbuf_desc->cpu_addr = kzalloc(bufsize,
GFP_KERNEL | __GFP_NOWARN |
__GFP_NOMEMALLOC |
__GFP_NORETRY);
@@ -539,8 +539,7 @@ int smc_sndbuf_create(struct smc_sock *smc)
continue;
}
rc = smc_ib_buf_map(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
- tmp_bufsize, sndbuf_desc,
- DMA_TO_DEVICE);
+ bufsize, sndbuf_desc, DMA_TO_DEVICE);
if (rc) {
kfree(sndbuf_desc->cpu_addr);
kfree(sndbuf_desc);
@@ -549,16 +548,15 @@ int smc_sndbuf_create(struct smc_sock *smc)
}
sndbuf_desc->used = 1;
write_lock_bh(&lgr->sndbufs_lock);
- list_add(&sndbuf_desc->list,
- &lgr->sndbufs[tmp_bufsize_short]);
+ list_add(&sndbuf_desc->list, &lgr->sndbufs[bufsize_short]);
write_unlock_bh(&lgr->sndbufs_lock);
break;
}
if (sndbuf_desc && sndbuf_desc->cpu_addr) {
conn->sndbuf_desc = sndbuf_desc;
- conn->sndbuf_size = tmp_bufsize;
- smc->sk.sk_sndbuf = tmp_bufsize * 2;
- atomic_set(&conn->sndbuf_space, tmp_bufsize);
+ conn->sndbuf_size = bufsize;
+ smc->sk.sk_sndbuf = bufsize * 2;
+ atomic_set(&conn->sndbuf_space, bufsize);
return 0;
} else {
return -ENOMEM;
@@ -574,25 +572,25 @@ int smc_rmb_create(struct smc_sock *smc)
{
struct smc_connection *conn = &smc->conn;
struct smc_link_group *lgr = conn->lgr;
- int tmp_bufsize, tmp_bufsize_short;
struct smc_buf_desc *rmb_desc;
+ int bufsize, bufsize_short;
int rc;
/* use socket recv buffer size (w/o overhead) as start value */
- for (tmp_bufsize_short = smc_compress_bufsize(smc->sk.sk_rcvbuf / 2);
- tmp_bufsize_short >= 0; tmp_bufsize_short--) {
- tmp_bufsize = smc_uncompress_bufsize(tmp_bufsize_short);
+ for (bufsize_short = smc_compress_bufsize(smc->sk.sk_rcvbuf / 2);
+ bufsize_short >= 0; bufsize_short--) {
+ bufsize = smc_uncompress_bufsize(bufsize_short);
/* check for reusable rmb_slot in the link group */
- rmb_desc = smc_rmb_get_slot(lgr, tmp_bufsize_short);
+ rmb_desc = smc_rmb_get_slot(lgr, bufsize_short);
if (rmb_desc) {
- memset(rmb_desc->cpu_addr, 0, tmp_bufsize);
+ memset(rmb_desc->cpu_addr, 0, bufsize);
break; /* found reusable slot */
}
/* try to alloc a new RMB */
rmb_desc = kzalloc(sizeof(*rmb_desc), GFP_KERNEL);
if (!rmb_desc)
break; /* give up with -ENOMEM */
- rmb_desc->cpu_addr = kzalloc(tmp_bufsize,
+ rmb_desc->cpu_addr = kzalloc(bufsize,
GFP_KERNEL | __GFP_NOWARN |
__GFP_NOMEMALLOC |
__GFP_NORETRY);
@@ -605,8 +603,7 @@ int smc_rmb_create(struct smc_sock *smc)
continue;
}
rc = smc_ib_buf_map(lgr->lnk[SMC_SINGLE_LINK].smcibdev,
- tmp_bufsize, rmb_desc,
- DMA_FROM_DEVICE);
+ bufsize, rmb_desc, DMA_FROM_DEVICE);
if (rc) {
kfree(rmb_desc->cpu_addr);
kfree(rmb_desc);
@@ -617,18 +614,17 @@ int smc_rmb_create(struct smc_sock *smc)
lgr->lnk[SMC_SINGLE_LINK].roce_pd->unsafe_global_rkey;
rmb_desc->used = 1;
write_lock_bh(&lgr->rmbs_lock);
- list_add(&rmb_desc->list,
- &lgr->rmbs[tmp_bufsize_short]);
+ list_add(&rmb_desc->list, &lgr->rmbs[bufsize_short]);
write_unlock_bh(&lgr->rmbs_lock);
break;
}
if (rmb_desc && rmb_desc->cpu_addr) {
conn->rmb_desc = rmb_desc;
- conn->rmbe_size = tmp_bufsize;
- conn->rmbe_size_short = tmp_bufsize_short;
- smc->sk.sk_rcvbuf = tmp_bufsize * 2;
+ conn->rmbe_size = bufsize;
+ conn->rmbe_size_short = bufsize_short;
+ smc->sk.sk_rcvbuf = bufsize * 2;
atomic_set(&conn->bytes_to_rcv, 0);
- conn->rmbe_update_limit = smc_rmb_wnd_update_limit(tmp_bufsize);
+ conn->rmbe_update_limit = smc_rmb_wnd_update_limit(bufsize);
return 0;
} else {
return -ENOMEM;
--
2.11.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH net-next 01/10] net/smc: serialize connection creation in all cases
From: Ursula Braun @ 2017-07-28 11:56 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: hch-jcswGhMUV9g, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA,
jwi-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
schwidefsky-tA70FqPdS9bQT0dZR+AlfA,
heiko.carstens-tA70FqPdS9bQT0dZR+AlfA,
raspl-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8
In-Reply-To: <20170728115622.17247-1-ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
If a link group for a new server connection exists already, the mutex
serializing the determination of link groups is given up early.
The coming registration of memory regions benefits from the serialization
as well, if the mutex is held till connection creation is finished.
This patch postpones the unlocking of the link group creation mutex.
Signed-off-by: Ursula Braun <ubraun-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
net/smc/af_smc.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 6793d7348cc8..120a7b9b4d8e 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -779,11 +779,6 @@ static void smc_listen_work(struct work_struct *work)
mutex_lock(&smc_create_lgr_pending);
local_contact = smc_conn_create(new_smc, peeraddr.sin_addr.s_addr,
smcibdev, ibport, &pclc.lcl, 0);
- if (local_contact == SMC_REUSE_CONTACT)
- /* lock no longer needed, free it due to following
- * smc_clc_wait_msg() call
- */
- mutex_unlock(&smc_create_lgr_pending);
if (local_contact < 0) {
rc = local_contact;
if (rc == -ENOMEM)
@@ -853,8 +848,7 @@ static void smc_listen_work(struct work_struct *work)
if (newsmcsk->sk_state == SMC_INIT)
newsmcsk->sk_state = SMC_ACTIVE;
enqueue:
- if (local_contact == SMC_FIRST_CONTACT)
- mutex_unlock(&smc_create_lgr_pending);
+ mutex_unlock(&smc_create_lgr_pending);
lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
if (lsmc->sk.sk_state == SMC_LISTEN) {
smc_accept_enqueue(&lsmc->sk, newsmcsk);
--
2.11.2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* RE: [PATCH V4 net-next 2/8] net: hns3: Add support of the HNAE3 framework
From: Salil Mehta @ 2017-07-28 11:52 UTC (permalink / raw)
To: Leon Romanovsky
Cc: davem@davemloft.net, Zhuangyuzeng (Yisen), huangdaode, lipeng (Y),
mehta.salil.lnk@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org,
Linuxarm
In-Reply-To: <20170728044137.GF13672@mtr-leonro.local>
Hi Leon,
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Leon Romanovsky
> Sent: Friday, July 28, 2017 5:42 AM
> To: Salil Mehta
> Cc: davem@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH V4 net-next 2/8] net: hns3: Add support of the
> HNAE3 framework
>
> On Thu, Jul 27, 2017 at 11:44:32PM +0000, Salil Mehta wrote:
> > Hi Leon
> >
> > > -----Original Message-----
> > > From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> > > owner@vger.kernel.org] On Behalf Of Leon Romanovsky
> > > Sent: Sunday, July 23, 2017 2:16 PM
> > > To: Salil Mehta
> > > Cc: davem@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng
> (Y);
> > > mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm
> > > Subject: Re: [PATCH V4 net-next 2/8] net: hns3: Add support of the
> > > HNAE3 framework
> > >
> > > On Sat, Jul 22, 2017 at 11:09:36PM +0100, Salil Mehta wrote:
> > > > This patch adds the support of the HNAE3 (Hisilicon Network
> > > > Acceleration Engine 3) framework support to the HNS3 driver.
> > > >
> > > > Framework facilitates clients like ENET(HNS3 Ethernet Driver),
> RoCE
> > > > and user-space Ethernet drivers (like ODP etc.) to register with
> > > HNAE3
> > > > devices and their associated operations.
> > > >
> > > > Signed-off-by: Daode Huang <huangdaode@hisilicon.com>
> > > > Signed-off-by: lipeng <lipeng321@huawei.com>
> > > > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > > > Signed-off-by: Yisen Zhuang <yisen.zhuang@huawei.com>
> > > > ---
> > > > Patch V4: Addressed following comments
> > > > 1. Andrew Lunn:
> > > > https://lkml.org/lkml/2017/6/17/233
> > > > https://lkml.org/lkml/2017/6/18/105
> > > > 2. Bo Yu:
> > > > https://lkml.org/lkml/2017/6/18/112
> > > > 3. Stephen Hamminger:
> > > > https://lkml.org/lkml/2017/6/19/778
> > > > Patch V3: Addressed below comments
> > > > 1. Andrew Lunn:
> > > > https://lkml.org/lkml/2017/6/13/1025
> > > > Patch V2: No change
> > > > Patch V1: Initial Submit
> > > > ---
> > > > drivers/net/ethernet/hisilicon/hns3/hnae3.c | 319
> > > ++++++++++++++++++++
> > > > drivers/net/ethernet/hisilicon/hns3/hnae3.h | 449
> > > ++++++++++++++++++++++++++++
> > > > 2 files changed, 768 insertions(+)
> > > > create mode 100644 drivers/net/ethernet/hisilicon/hns3/hnae3.c
> > > > create mode 100644 drivers/net/ethernet/hisilicon/hns3/hnae3.h
> > > >
> > > > diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.c
> > > b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
> > > > new file mode 100644
> > > > index 000000000000..7a11aaff0a23
[...]
> > > > +
> > > > +#include <linux/acpi.h>
> > > > +#include <linux/delay.h>
> > > > +#include <linux/device.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/netdevice.h>
> > > > +#include <linux/pci.h>
> > > > +#include <linux/types.h>
> > > > +
> > > > +#define HNAE_DRIVER_VERSION "1.0"
> > >
> > > Please no driver versions.
> > We need this in ethtool. Most of the driver are using it.
>
> So please, stop doing copy/paste and take a look how it was implemented
> in nfp.
>
> Related discussion about useless of your driver version.
> https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2017-
> June/004441.html
> https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2017-
> June/004428.html
Sure, thanks for this. I will have a look
>
> >
> > >
> > > > +#define HNAE_DRIVER_NAME "hns3"
> > > > +#define HNAE_COPYRIGHT "Copyright(c) 2017 Huawei Corporation."
> > > > +#define HNAE_DRIVER_STRING "Hisilicon Network Subsystem Driver"
> > > > +#define HNAE_DEFAULT_DEVICE_DESCR "Hisilicon Network Subsystem"
> > >
> > > You are not subsystem yet.
> > Hisilicon Network System is the network related hardware within
> > Hip08 SoC of Hisilicon. This does not means HNS is Linux network
> > subsystem.
>
> I understand it, so remove word "subsystem" and use more appropriate
> "core", "library", e.t.c
I understand this might look bit confusing, but if we change the name
of the driver now then we might have to change the name of the folders
and earlier driver names as well. Name of the earlier driver would be
very difficult to change at this stage since there might be people using
it.
Do you think if we rename to below it will bring more clarity?
+#define HNAE_DRIVER_STRING "Hisilicon Network Subsystem Ethernet Driver"
OR
+#define HNAE_DRIVER_STRING "HNS3 Ethernet Driver"
Please suggest? Thanks!
Best regards
Salil
>
> Thanks
^ permalink raw reply
* [PATCH v2] net: phy: Log only PHY state transitions
From: Marc Gonzalez @ 2017-07-28 11:18 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David S. Miller; +Cc: netdev, Mason
In-Reply-To: <8fc7eb8f-9d64-04c3-a1ca-0cc8eec1ee30@sigmadesigns.com>
In the current code, old and new PHY states are always logged.
>From now on, log only PHY state transitions.
Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
Fix indentation. I was trying to minimize the diff.
I also considered using a goto to jump over the phydev_dbg() call,
but that didn't feel right.
---
drivers/net/phy/phy.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index d0626bf5c540..3ff79c3d90f0 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1226,9 +1226,10 @@ void phy_state_machine(struct work_struct *work)
if (err < 0)
phy_error(phydev);
- phydev_dbg(phydev, "PHY state change %s -> %s\n",
- phy_state_to_str(old_state),
- phy_state_to_str(phydev->state));
+ if (old_state != phydev->state)
+ phydev_dbg(phydev, "PHY state change %s -> %s\n",
+ phy_state_to_str(old_state),
+ phy_state_to_str(phydev->state));
/* Only re-schedule a PHY state machine change if we are polling the
* PHY, if PHY_IGNORE_INTERRUPT is set, then we will be moving
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next v2 01/10] net: dsa: lan9303: Fixed MDIO interface
From: Egil Hjelmeland @ 2017-07-28 11:08 UTC (permalink / raw)
To: Andrew Lunn
Cc: corbet, vivien.didelot, f.fainelli, davem, kernel, linux-doc,
linux-kernel, netdev
In-Reply-To: <20170726165550.GN12049@lunn.ch>
On 26. juli 2017 18:55, Andrew Lunn wrote:
> On Tue, Jul 25, 2017 at 06:15:44PM +0200, Egil Hjelmeland wrote:
> It is better to use mdiobus_read/write or if you are nesting mdio
> busses, mdiobus_read_nested/mdiobus_write_nested. Please test this
> code with lockdep enabled.
>
I have CONFIG_DEBUG_SPINLOCK, CONFIG_DEBUG_MUTEXES. Should I enable
more?
Egil
^ permalink raw reply
* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
From: Hangbin Liu @ 2017-07-28 11:04 UTC (permalink / raw)
To: Cong Wang; +Cc: David Ahern, Roopa Prabhu, network dev
In-Reply-To: <CAM_iQpU5EZr1rtgY8sCBjD=utzmi9c6MQ2xb36iXLk++qSk3WA@mail.gmail.com>
On Thu, Jul 27, 2017 at 09:56:08PM -0700, Cong Wang wrote:
> On Wed, Jul 26, 2017 at 11:49 AM, David Ahern <dsahern@gmail.com> wrote:
> > On 7/26/17 12:27 PM, Roopa Prabhu wrote:
> >> agreed...so looks like the check in v3 should be
> >>
> >>
> >> + if ( rt == net->ipv6.ip6_null_entry ||
> >> + (rt->dst.error &&
> >> + #ifdef CONFIG_IPV6_MULTIPLE_TABLES
> >> + rt != net->ipv6.ip6_prohibit_entry &&
> >> + rt != net->ipv6.ip6_blk_hole_entry &&
> >> +#endif
> >> + )) {
> >> err = rt->dst.error;
> >> ip6_rt_put(rt);
> >> goto errout;
> >>
> >
> > I don't think so. If I add a prohibit route and use the fibmatch
> > attribute, I want to see the route from the FIB that was matched.
>
> But net->ipv6.ip6_prohibit_entry is not the prohibit route you can
> add in user-space, it is only used by rule actions. So do you really
> want to dump it?? My gut feeling is no, but I am definitely not sure.
>
> When you add a prohibit route, a new rt is allocated dynamically,
> net->ipv6.ip6_prohibit_entry is relatively static, internal and is the
> only one per netns. (Same for net->ipv6.ip6_blk_hole_entry)
>
> I think Hangbin's example doesn't have ip rules, so this case
> is not shown up.
I mixed the rule entry and route entry these days. And with your help I can
separate them now.
When first time I find the rt->dst.error return directly issue, I was testing
ip rule actually.
e.g.
+ ip netns exec client ip -6 rule add to 2003::1/64 table 100 unreachable
+ ip netns exec server ip -6 rule add to 2001::1/64 table 100 prohibit
+ ip netns exec client ip -6 route get 2003::1
RTNETLINK answers: Network is unreachable
+ ip netns exec client ip -6 route get 2001::1
RTNETLINK answers: Permission denied
After check I thought we returned net->ipv6.ip6_null_entry /
net->ipv6.ip6_blk_hole_entry in function fib6_rule_action().
That's the reason I want to delete both rt->dst.error and
net->ipv6.ip6_null_entry check in patch v2 and v3.
Then with David's comments I realise we also need to take care about ip route
entrys.
my last mail's comment:
> Thanks for your explains. Now I know where I made the mistake. I mis-looked
> FR_ACT_UNREACHABLE to RTN_UNREACHABLE and thought we return rt =
> net->ipv6.ip6_null_entry in fib6_rule_action().
But then I fall in to the code logic and get lost... And thought
FR_ACT_UNREACHABLE and RTN_UNREACHABLE are not same. Today I re-check the
code and realise RTN_UNREACHABLE is defined in user space and FR_ACT_UNREACHABLE
is in kernel. Actually they are the same.
So after PATCH v4, we fixed the route side. And part of ip rule(prohibit and
blk hole). I will think over of this.
Thanks
Hangbin
^ permalink raw reply
* Re: [PATCH net] net: phy: Run state machine to completion
From: Marc Gonzalez @ 2017-07-28 10:57 UTC (permalink / raw)
To: Florian Fainelli, David S. Miller, netdev
Cc: Russell King, Zach Brown, Nathan Sullivan, Andrew Lunn, LKML,
Mason, Mans Rullgard
In-Reply-To: <20170726192446.32045-1-f.fainelli@gmail.com>
On 26/07/2017 21:24, Florian Fainelli wrote:
> Marc reported that he was not getting the PHY library adjust_link()
> callback function to run when calling phy_stop() + phy_disconnect()
> which does not indeed happen because we don't make sure we flush the
> PHYLIB delayed work and let it run to completion. We also need to have
> a synchronous call to phy_state_machine() in order to have the state
> machine actually act on PHY_HALTED, set the PHY device's link down, turn
> the network device's carrier off and finally call the adjust_link()
> function.
>
> Reported-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> Fixes: a390d1f379cf ("phylib: convert state_queue work to delayed_work")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> David, this dates back from before the commit mentioned in Fixes but it would
> be hard to backport to earlier kernels if we flagged the original design flaw
> that used timers. Also, I am not clear on the timer API whether there was a way
> to ensure timers would run to completion before they would be cancelled.
>
> Marc, please add your Signed-off-by tag since you contributed the second line.
>
> Thanks!
>
> drivers/net/phy/phy.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index d0626bf5c540..30e7c43e0d87 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -743,12 +743,17 @@ void phy_trigger_machine(struct phy_device *phydev, bool sync)
> */
> void phy_stop_machine(struct phy_device *phydev)
> {
> + /* Run the state machine to completion */
> + flush_delayed_work(&phydev->state_queue);
> cancel_delayed_work_sync(&phydev->state_queue);
>
> mutex_lock(&phydev->lock);
> if (phydev->state > PHY_UP && phydev->state != PHY_HALTED)
> phydev->state = PHY_UP;
> mutex_unlock(&phydev->lock);
> +
> + /* Now we can run the state machine synchronously */
> + phy_state_machine(&phydev->state_queue.work);
Hello Florian,
Sorry, I was AFK for a couple of days.
Thanks for putting the patch together!
I added dump_stack() at the top of phy_state_machine() to better
understand what's going on.
Action: set link down on the command-line
Results:
1) PHY in interrupt mode
[ 136.238889] [<c0378518>] (phy_state_machine) from [<c0378a58>] (phy_stop_machine+0x4c/0x50)
[ 136.247285] [<c0378a58>] (phy_stop_machine) from [<c0379c30>] (phy_disconnect+0x20/0x34)
[ 136.255420] [<c0379c30>] (phy_disconnect) from [<c037d984>] (nb8800_stop+0x84/0xa0)
[ 136.263118] [<c037d984>] (nb8800_stop) from [<c03f52a4>] (__dev_close_many+0x88/0xd0)
[ 136.270989] [<c03f52a4>] (__dev_close_many) from [<c03f540c>] (__dev_close+0x24/0x38)
[ 136.278862] [<c03f540c>] (__dev_close) from [<c03fdd98>] (__dev_change_flags+0x94/0x144)
[ 136.286997] [<c03fdd98>] (__dev_change_flags) from [<c03fde60>] (dev_change_flags+0x18/0x48)
[ 136.295481] [<c03fde60>] (dev_change_flags) from [<c0463350>] (devinet_ioctl+0x6e0/0x7a0)
[ 136.303702] [<c0463350>] (devinet_ioctl) from [<c04657e0>] (inet_ioctl+0x194/0x1c0)
[ 136.311400] [<c04657e0>] (inet_ioctl) from [<c03de2f8>] (sock_ioctl+0x148/0x2fc)
[ 136.318837] [<c03de2f8>] (sock_ioctl) from [<c01dbe80>] (do_vfs_ioctl+0x9c/0x8e8)
[ 136.326361] [<c01dbe80>] (do_vfs_ioctl) from [<c01dc700>] (SyS_ioctl+0x34/0x5c)
[ 136.333710] [<c01dc700>] (SyS_ioctl) from [<c01076c0>] (ret_fast_syscall+0x0/0x3c)
[ 136.341357] nb8800 26000.ethernet eth0: Link is Down
Above, phy_state_machine() is called synchronously from phy_stop_machine().
2) PHY in polling mode
[ 54.045752] CPU: 3 PID: 892 Comm: kworker/3:1 Not tainted 4.13.0-rc1 #7
[ 54.056435] Workqueue: events_power_efficient phy_state_machine
[ 54.085399] [<c0378518>] (phy_state_machine) from [<c012e404>] (process_one_work+0x1d4/0x3ec)
[ 54.093969] [<c012e404>] (process_one_work) from [<c012f250>] (worker_thread+0x268/0x554)
[ 54.102189] [<c012f250>] (worker_thread) from [<c0133fd4>] (kthread+0x108/0x138)
[ 54.109623] [<c0133fd4>] (kthread) from [<c0107778>] (ret_from_fork+0x14/0x3c)
[ 54.116918] nb8800 26000.ethernet eth0: Link is Down
[ 54.122100] CPU: 2 PID: 973 Comm: ip Not tainted 4.13.0-rc1 #7
[ 54.155001] [<c0378518>] (phy_state_machine) from [<c0378a58>] (phy_stop_machine+0x4c/0x50)
[ 54.163397] [<c0378a58>] (phy_stop_machine) from [<c0379c30>] (phy_disconnect+0x20/0x34)
[ 54.171532] [<c0379c30>] (phy_disconnect) from [<c037d984>] (nb8800_stop+0x84/0xa0)
[ 54.179231] [<c037d984>] (nb8800_stop) from [<c03f52a4>] (__dev_close_many+0x88/0xd0)
[ 54.187103] [<c03f52a4>] (__dev_close_many) from [<c03f540c>] (__dev_close+0x24/0x38)
[ 54.194975] [<c03f540c>] (__dev_close) from [<c03fdd98>] (__dev_change_flags+0x94/0x144)
[ 54.203109] [<c03fdd98>] (__dev_change_flags) from [<c03fde60>] (dev_change_flags+0x18/0x48)
[ 54.211594] [<c03fde60>] (dev_change_flags) from [<c0463350>] (devinet_ioctl+0x6e0/0x7a0)
[ 54.219816] [<c0463350>] (devinet_ioctl) from [<c04657e0>] (inet_ioctl+0x194/0x1c0)
[ 54.227513] [<c04657e0>] (inet_ioctl) from [<c03de2f8>] (sock_ioctl+0x148/0x2fc)
[ 54.234950] [<c03de2f8>] (sock_ioctl) from [<c01dbe80>] (do_vfs_ioctl+0x9c/0x8e8)
[ 54.242473] [<c01dbe80>] (do_vfs_ioctl) from [<c01dc700>] (SyS_ioctl+0x34/0x5c)
[ 54.249821] [<c01dc700>] (SyS_ioctl) from [<c01076c0>] (ret_fast_syscall+0x0/0x3c)
Above() phy_state_machine() is called twice
- first supposedly after flush_delayed_work() => calls adjust_link()
- a second time by the explicit call => probably no-op
Therefore, my reasoning is that flush_delayed_work() is
unnecessary. The explicit call to phy_state_machine() at
the end of phy_stop_machine() will handle all cases.
What do you think?
Testing without flush_delayed_work() in polling mode:
[ 58.819100] CPU: 0 PID: 968 Comm: ip Not tainted 4.13.0-rc1 #10
[ 58.852098] [<c0378518>] (phy_state_machine) from [<c0378a50>] (phy_stop_machine+0x44/0x48)
[ 58.860494] [<c0378a50>] (phy_stop_machine) from [<c0379c28>] (phy_disconnect+0x20/0x34)
[ 58.868629] [<c0379c28>] (phy_disconnect) from [<c037d97c>] (nb8800_stop+0x84/0xa0)
[ 58.876328] [<c037d97c>] (nb8800_stop) from [<c03f529c>] (__dev_close_many+0x88/0xd0)
[ 58.884198] [<c03f529c>] (__dev_close_many) from [<c03f5404>] (__dev_close+0x24/0x38)
[ 58.892071] [<c03f5404>] (__dev_close) from [<c03fdd90>] (__dev_change_flags+0x94/0x144)
[ 58.900206] [<c03fdd90>] (__dev_change_flags) from [<c03fde58>] (dev_change_flags+0x18/0x48)
[ 58.908690] [<c03fde58>] (dev_change_flags) from [<c0463348>] (devinet_ioctl+0x6e0/0x7a0)
[ 58.916911] [<c0463348>] (devinet_ioctl) from [<c04657d8>] (inet_ioctl+0x194/0x1c0)
[ 58.924608] [<c04657d8>] (inet_ioctl) from [<c03de2f0>] (sock_ioctl+0x148/0x2fc)
[ 58.932046] [<c03de2f0>] (sock_ioctl) from [<c01dbe80>] (do_vfs_ioctl+0x9c/0x8e8)
[ 58.939569] [<c01dbe80>] (do_vfs_ioctl) from [<c01dc700>] (SyS_ioctl+0x34/0x5c)
[ 58.946917] [<c01dc700>] (SyS_ioctl) from [<c01076c0>] (ret_fast_syscall+0x0/0x3c)
[ 58.954609] nb8800 26000.ethernet eth0: Link is Down
Above, the call to adjust_link() occurs from the explicit call
to phy_state_machine()
Regards.
^ permalink raw reply
* Re: [PATCH 3/3] net-next: stmmac: dwmac-sun8i: choose internal PHY via compatible
From: Corentin Labbe @ 2017-07-28 9:54 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Rob Herring, Mark Rutland, Maxime Ripard, Russell King,
Giuseppe Cavallaro, alexandre.torgue-qxv4g6HH51o, Icenowy Zheng,
netdev, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi
In-Reply-To: <CAGb2v64w-KCmYExN7OqvuLj42-pHN-BN5iKpxA+KW84v+RfCDQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Jul 28, 2017 at 05:49:55PM +0800, Chen-Yu Tsai wrote:
> On Fri, Jul 28, 2017 at 5:28 PM, Corentin Labbe
> <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > The current way to find if the phy is internal is to compare DT phy-mode
> > and emac_variant/internal_phy.
> > But it will negate a possible future SoC where an external PHY use the
> > same phy mode than the internal one.
> >
> > This patch adds a new way to find if the PHY is internal, via its
> > compatible.
> >
> > Since the phy_mode of the internal PHY does need to be know, the
> > variant internal_phy member is converted to a boolean.
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> > drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 ++++++++++------
> > 1 file changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> > index fffd6d5fc907..04f7ddd802b0 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> > @@ -48,7 +48,7 @@
> > */
> > struct emac_variant {
> > u32 default_syscon_value;
> > - int internal_phy;
> > + bool internal_phy;
>
> Nit: please add a verb to the name, like "has_internal_phy",
> or even "soc_has_internal_phy". This makes it clear what this
> property represents.
>
> ChenYu
I will use soc_has_internal_phy
Thanks
^ permalink raw reply
* Re: [PATCH 0/3] net-next: stmmac: support future possible different internal phy mode
From: Chen-Yu Tsai @ 2017-07-28 9:53 UTC (permalink / raw)
To: Corentin Labbe
Cc: Rob Herring, Mark Rutland, Maxime Ripard, Chen-Yu Tsai,
Russell King, Giuseppe Cavallaro, alexandre.torgue-qxv4g6HH51o,
Icenowy Zheng, netdev, devicetree, linux-arm-kernel, linux-kernel,
linux-sunxi
In-Reply-To: <20170728092818.23419-1-clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Fri, Jul 28, 2017 at 5:28 PM, Corentin Labbe
<clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hello
>
> The current way to find if the phy is internal is to compare DT phy-mode
> and emac_variant/internal_phy.
> But it will negate a possible future SoC where an external PHY use the
> same phy mode than the internal one.
>
> This patchs series adds a new way to find if the PHY is internal, via its
> compatible.
You've already joined in on the discussion for the patch "net: stmmac:
dwmac-rk: Add internal phy support". It is pretty much the same issue.
Please wait for it to come to a proper conclusion. At least then we can
agree on something so different platforms with the same problem don't
have diverging solutions.
ChenYu
>
> Corentin Labbe (3):
> dt-bindings: net: add compatible for internal sun8i-h3/sun8i-v3s PHYs
> ARM: sunxi: h3/h5: Add sun8i-h3-ephy compatible
> net-next: stmmac: dwmac-sun8i: choose internal PHY via compatible
>
> Documentation/devicetree/bindings/net/dwmac-sun8i.txt | 4 ++--
> arch/arm/boot/dts/sunxi-h3-h5.dtsi | 3 ++-
> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 ++++++++++------
> 3 files changed, 14 insertions(+), 9 deletions(-)
>
> --
> 2.13.0
>
^ permalink raw reply
* Re: [PATCH 3/3] net-next: stmmac: dwmac-sun8i: choose internal PHY via compatible
From: Chen-Yu Tsai @ 2017-07-28 9:49 UTC (permalink / raw)
To: Corentin Labbe
Cc: Rob Herring, Mark Rutland, Maxime Ripard, Chen-Yu Tsai,
Russell King, Giuseppe Cavallaro, alexandre.torgue-qxv4g6HH51o,
Icenowy Zheng, netdev, devicetree, linux-arm-kernel, linux-kernel,
linux-sunxi
In-Reply-To: <20170728092818.23419-4-clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Fri, Jul 28, 2017 at 5:28 PM, Corentin Labbe
<clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> The current way to find if the phy is internal is to compare DT phy-mode
> and emac_variant/internal_phy.
> But it will negate a possible future SoC where an external PHY use the
> same phy mode than the internal one.
>
> This patch adds a new way to find if the PHY is internal, via its
> compatible.
>
> Since the phy_mode of the internal PHY does need to be know, the
> variant internal_phy member is converted to a boolean.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index fffd6d5fc907..04f7ddd802b0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -48,7 +48,7 @@
> */
> struct emac_variant {
> u32 default_syscon_value;
> - int internal_phy;
> + bool internal_phy;
Nit: please add a verb to the name, like "has_internal_phy",
or even "soc_has_internal_phy". This makes it clear what this
property represents.
ChenYu
^ permalink raw reply
* Re: [PATCH 2/3] ARM: sunxi: h3/h5: Add sun8i-h3-ephy compatible
From: Icenowy Zheng @ 2017-07-28 9:46 UTC (permalink / raw)
To: Chen-Yu Tsai, Corentin Labbe
Cc: Rob Herring, Mark Rutland, Maxime Ripard, Russell King,
Giuseppe Cavallaro, alexandre.torgue-qxv4g6HH51o, netdev,
devicetree, linux-arm-kernel, linux-kernel, linux-sunxi
In-Reply-To: <CAGb2v67Lt5whR4=6+SDc=Ooi6zyiHuprW+7zbPh0njnKOCiFFA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
于 2017年7月28日 GMT+08:00 下午5:44:51, Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org> 写到:
>On Fri, Jul 28, 2017 at 5:28 PM, Corentin Labbe
><clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> This patch adds the sun8i-h3-ephy compatible to the internal PHY.
>>
>> Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> arch/arm/boot/dts/sunxi-h3-h5.dtsi | 3 ++-
>
>To avoid repeating the past, this patch, if approved, will be merged
>through the sunxi tree, not netdev nor net-next.
>
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> index 4b599b5d26f6..7aaa837c2388 100644
>> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> @@ -421,7 +421,8 @@
>> #address-cells = <1>;
>> #size-cells = <0>;
>> int_mii_phy: ethernet-phy@1 {
>> - compatible =
>"ethernet-phy-ieee802.3-c22";
>> + compatible =
>"allwinner,sun8i-h3-ephy",
>> +
>"ethernet-phy-ieee802.3-c22";
>
>Are you expecting people to override this properly?
>
>As it currently is, any external phy at address 1 will simply
>reuse the same device node. And if they don't override the
>property correctly, the driver will end up trying to use
>the internal phy, while the user is expecting the external
>one to be used.
>
>Maybe you could move this to some other address, maybe the last
>valid one, or second last valid one?
Some board designers may use other address.
For example, on Nano Pi NEO2 the PHY is attached at address 0x7.
>
>ChenYu
>
>> reg = <1>;
>> clocks = <&ccu CLK_BUS_EPHY>;
>> resets = <&ccu RST_BUS_EPHY>;
>> --
>> 2.13.0
>>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/3] ARM: sunxi: h3/h5: Add sun8i-h3-ephy compatible
From: Chen-Yu Tsai @ 2017-07-28 9:44 UTC (permalink / raw)
To: Corentin Labbe
Cc: Rob Herring, Mark Rutland, Maxime Ripard, Chen-Yu Tsai,
Russell King, Giuseppe Cavallaro, alexandre.torgue, Icenowy Zheng,
netdev, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi
In-Reply-To: <20170728092818.23419-3-clabbe.montjoie@gmail.com>
On Fri, Jul 28, 2017 at 5:28 PM, Corentin Labbe
<clabbe.montjoie@gmail.com> wrote:
> This patch adds the sun8i-h3-ephy compatible to the internal PHY.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> arch/arm/boot/dts/sunxi-h3-h5.dtsi | 3 ++-
To avoid repeating the past, this patch, if approved, will be merged
through the sunxi tree, not netdev nor net-next.
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> index 4b599b5d26f6..7aaa837c2388 100644
> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> @@ -421,7 +421,8 @@
> #address-cells = <1>;
> #size-cells = <0>;
> int_mii_phy: ethernet-phy@1 {
> - compatible = "ethernet-phy-ieee802.3-c22";
> + compatible = "allwinner,sun8i-h3-ephy",
> + "ethernet-phy-ieee802.3-c22";
Are you expecting people to override this properly?
As it currently is, any external phy at address 1 will simply
reuse the same device node. And if they don't override the
property correctly, the driver will end up trying to use
the internal phy, while the user is expecting the external
one to be used.
Maybe you could move this to some other address, maybe the last
valid one, or second last valid one?
ChenYu
> reg = <1>;
> clocks = <&ccu CLK_BUS_EPHY>;
> resets = <&ccu RST_BUS_EPHY>;
> --
> 2.13.0
>
^ permalink raw reply
* [PATCH 3/3] net-next: stmmac: dwmac-sun8i: choose internal PHY via compatible
From: Corentin Labbe @ 2017-07-28 9:28 UTC (permalink / raw)
To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, peppe.cavallaro-qxv4g6HH51o,
alexandre.torgue-qxv4g6HH51o
Cc: icenowy-h8G6r0blFSE, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Corentin Labbe
In-Reply-To: <20170728092818.23419-1-clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
The current way to find if the phy is internal is to compare DT phy-mode
and emac_variant/internal_phy.
But it will negate a possible future SoC where an external PHY use the
same phy mode than the internal one.
This patch adds a new way to find if the PHY is internal, via its
compatible.
Since the phy_mode of the internal PHY does need to be know, the
variant internal_phy member is converted to a boolean.
Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index fffd6d5fc907..04f7ddd802b0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -48,7 +48,7 @@
*/
struct emac_variant {
u32 default_syscon_value;
- int internal_phy;
+ bool internal_phy;
bool support_mii;
bool support_rmii;
bool support_rgmii;
@@ -75,7 +75,7 @@ struct sunxi_priv_data {
static const struct emac_variant emac_variant_h3 = {
.default_syscon_value = 0x58000,
- .internal_phy = PHY_INTERFACE_MODE_MII,
+ .internal_phy = true,
.support_mii = true,
.support_rmii = true,
.support_rgmii = true
@@ -83,20 +83,20 @@ static const struct emac_variant emac_variant_h3 = {
static const struct emac_variant emac_variant_v3s = {
.default_syscon_value = 0x38000,
- .internal_phy = PHY_INTERFACE_MODE_MII,
+ .internal_phy = true,
.support_mii = true
};
static const struct emac_variant emac_variant_a83t = {
.default_syscon_value = 0,
- .internal_phy = 0,
+ .internal_phy = false,
.support_mii = true,
.support_rgmii = true
};
static const struct emac_variant emac_variant_a64 = {
.default_syscon_value = 0,
- .internal_phy = 0,
+ .internal_phy = false,
.support_mii = true,
.support_rmii = true,
.support_rgmii = true
@@ -889,6 +889,10 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
struct sunxi_priv_data *gmac;
struct device *dev = &pdev->dev;
int ret;
+ static const struct of_device_id internal_phys[] = {
+ { .compatible = "allwinner,sun8i-h3-ephy" },
+ { .compatible = "allwinner,sun8i-v3s-ephy" },
+ };
ret = stmmac_get_platform_resources(pdev, &stmmac_res);
if (ret)
@@ -932,7 +936,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
}
plat_dat->interface = of_get_phy_mode(dev->of_node);
- if (plat_dat->interface == gmac->variant->internal_phy) {
+ if (of_match_node(internal_phys, plat_dat->phy_node)) {
dev_info(&pdev->dev, "Will use internal PHY\n");
gmac->use_internal_phy = true;
gmac->ephy_clk = of_clk_get(plat_dat->phy_node, 0);
--
2.13.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox