* [PATCH 2/6] IB/hfi1: Inline sdma_txclean() for verbs pio
From: Dennis Dalessandro @ 2016-10-10 13:14 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Mike Marciniszyn,
Sebastian Sanchez
In-Reply-To: <20161010131249.17586.23789.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org>
From: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Short circuit sdma_txclean() by adding an __sdma_txclean()
that is only called when the tx has sdma mappings.
Convert internal calls to __sdma_txclean().
This removes a call from the critical path.
Reviewed-by: Sebastian Sanchez <sebastian.sanchez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/hfi1/sdma.c | 18 +++++++++---------
drivers/infiniband/hw/hfi1/sdma.h | 12 +++++++++---
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/sdma.c b/drivers/infiniband/hw/hfi1/sdma.c
index fd39bca..7102a07 100644
--- a/drivers/infiniband/hw/hfi1/sdma.c
+++ b/drivers/infiniband/hw/hfi1/sdma.c
@@ -375,7 +375,7 @@ static inline void complete_tx(struct sdma_engine *sde,
sde->head_sn, tx->sn);
sde->head_sn++;
#endif
- sdma_txclean(sde->dd, tx);
+ __sdma_txclean(sde->dd, tx);
if (complete)
(*complete)(tx, res);
if (wait && iowait_sdma_dec(wait))
@@ -1643,7 +1643,7 @@ static inline u8 ahg_mode(struct sdma_txreq *tx)
}
/**
- * sdma_txclean() - clean tx of mappings, descp *kmalloc's
+ * __sdma_txclean() - clean tx of mappings, descp *kmalloc's
* @dd: hfi1_devdata for unmapping
* @tx: tx request to clean
*
@@ -1653,7 +1653,7 @@ static inline u8 ahg_mode(struct sdma_txreq *tx)
* The code can be called multiple times without issue.
*
*/
-void sdma_txclean(
+void __sdma_txclean(
struct hfi1_devdata *dd,
struct sdma_txreq *tx)
{
@@ -3080,7 +3080,7 @@ static int _extend_sdma_tx_descs(struct hfi1_devdata *dd, struct sdma_txreq *tx)
tx->descp[i] = tx->descs[i];
return 0;
enomem:
- sdma_txclean(dd, tx);
+ __sdma_txclean(dd, tx);
return -ENOMEM;
}
@@ -3109,14 +3109,14 @@ int ext_coal_sdma_tx_descs(struct hfi1_devdata *dd, struct sdma_txreq *tx,
rval = _extend_sdma_tx_descs(dd, tx);
if (rval) {
- sdma_txclean(dd, tx);
+ __sdma_txclean(dd, tx);
return rval;
}
/* If coalesce buffer is allocated, copy data into it */
if (tx->coalesce_buf) {
if (type == SDMA_MAP_NONE) {
- sdma_txclean(dd, tx);
+ __sdma_txclean(dd, tx);
return -EINVAL;
}
@@ -3124,7 +3124,7 @@ int ext_coal_sdma_tx_descs(struct hfi1_devdata *dd, struct sdma_txreq *tx,
kvaddr = kmap(page);
kvaddr += offset;
} else if (WARN_ON(!kvaddr)) {
- sdma_txclean(dd, tx);
+ __sdma_txclean(dd, tx);
return -EINVAL;
}
@@ -3154,7 +3154,7 @@ int ext_coal_sdma_tx_descs(struct hfi1_devdata *dd, struct sdma_txreq *tx,
DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(&dd->pcidev->dev, addr))) {
- sdma_txclean(dd, tx);
+ __sdma_txclean(dd, tx);
return -ENOSPC;
}
@@ -3196,7 +3196,7 @@ int _pad_sdma_tx_descs(struct hfi1_devdata *dd, struct sdma_txreq *tx)
if ((unlikely(tx->num_desc == tx->desc_limit))) {
rval = _extend_sdma_tx_descs(dd, tx);
if (rval) {
- sdma_txclean(dd, tx);
+ __sdma_txclean(dd, tx);
return rval;
}
}
diff --git a/drivers/infiniband/hw/hfi1/sdma.h b/drivers/infiniband/hw/hfi1/sdma.h
index 56257ea..21f1e28 100644
--- a/drivers/infiniband/hw/hfi1/sdma.h
+++ b/drivers/infiniband/hw/hfi1/sdma.h
@@ -667,7 +667,13 @@ int ext_coal_sdma_tx_descs(struct hfi1_devdata *dd, struct sdma_txreq *tx,
int type, void *kvaddr, struct page *page,
unsigned long offset, u16 len);
int _pad_sdma_tx_descs(struct hfi1_devdata *, struct sdma_txreq *);
-void sdma_txclean(struct hfi1_devdata *, struct sdma_txreq *);
+void __sdma_txclean(struct hfi1_devdata *, struct sdma_txreq *);
+
+static inline void sdma_txclean(struct hfi1_devdata *dd, struct sdma_txreq *tx)
+{
+ if (tx->num_desc)
+ __sdma_txclean(dd, tx);
+}
/* helpers used by public routines */
static inline void _sdma_close_tx(struct hfi1_devdata *dd,
@@ -753,7 +759,7 @@ static inline int sdma_txadd_page(
DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(&dd->pcidev->dev, addr))) {
- sdma_txclean(dd, tx);
+ __sdma_txclean(dd, tx);
return -ENOSPC;
}
@@ -834,7 +840,7 @@ static inline int sdma_txadd_kvaddr(
DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(&dd->pcidev->dev, addr))) {
- sdma_txclean(dd, tx);
+ __sdma_txclean(dd, tx);
return -ENOSPC;
}
--
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 1/6] IB/hfi1: Add unique txwait_lock for txreq events
From: Dennis Dalessandro @ 2016-10-10 13:14 UTC (permalink / raw)
To: dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Mike Marciniszyn,
Sebastian Sanchez
In-Reply-To: <20161010131249.17586.23789.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org>
From: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Profiling suggests that the read_seqbegin() in
the txreq put logic is colliding with other uses
of the iowait lock.
The packet at a time use of this lock dictates a unique
lock to avoid reader/writer collisions when the number
of vTxWait events is low.
In order to support a unique lock the iowait struct embedded
in the QP is extended to remember the lock that protects the queue
head.
The QP destroy removes that QP from any wait list. It doesn't
need to know the head because of the linked list API, but it does
need to know the lock required to protect the head.
This also opens up the wait logic to have unique per resources locks
which needs to be in future refinement.
Reviewed-by: Sebastian Sanchez <sebastian.sanchez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/infiniband/hw/hfi1/iowait.h | 8 ++++++++
drivers/infiniband/hw/hfi1/pio.c | 1 +
drivers/infiniband/hw/hfi1/qp.c | 11 ++++++++---
drivers/infiniband/hw/hfi1/verbs.c | 4 ++++
drivers/infiniband/hw/hfi1/verbs.h | 13 +++++++------
drivers/infiniband/hw/hfi1/verbs_txreq.c | 13 +++++++------
6 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/iowait.h b/drivers/infiniband/hw/hfi1/iowait.h
index 2ec6ef3..d9740dd 100644
--- a/drivers/infiniband/hw/hfi1/iowait.h
+++ b/drivers/infiniband/hw/hfi1/iowait.h
@@ -64,6 +64,7 @@ struct sdma_engine;
/**
* struct iowait - linkage for delayed progress/waiting
* @list: used to add/insert into QP/PQ wait lists
+ * @lock: uses to record the list head lock
* @tx_head: overflow list of sdma_txreq's
* @sleep: no space callback
* @wakeup: space callback wakeup
@@ -91,6 +92,11 @@ struct sdma_engine;
* so sleeping is not allowed.
*
* The wait_dma member along with the iow
+ *
+ * The lock field is used by waiters to record
+ * the seqlock_t that guards the list head.
+ * Waiters explicity know that, but the destroy
+ * code that unwaits QPs does not.
*/
struct iowait {
@@ -103,6 +109,7 @@ struct iowait {
unsigned seq);
void (*wakeup)(struct iowait *wait, int reason);
void (*sdma_drained)(struct iowait *wait);
+ seqlock_t *lock;
struct work_struct iowork;
wait_queue_head_t wait_dma;
wait_queue_head_t wait_pio;
@@ -141,6 +148,7 @@ static inline void iowait_init(
void (*sdma_drained)(struct iowait *wait))
{
wait->count = 0;
+ wait->lock = NULL;
INIT_LIST_HEAD(&wait->list);
INIT_LIST_HEAD(&wait->tx_head);
INIT_WORK(&wait->iowork, func);
diff --git a/drivers/infiniband/hw/hfi1/pio.c b/drivers/infiniband/hw/hfi1/pio.c
index 50a3a36..385e4dc 100644
--- a/drivers/infiniband/hw/hfi1/pio.c
+++ b/drivers/infiniband/hw/hfi1/pio.c
@@ -1580,6 +1580,7 @@ static void sc_piobufavail(struct send_context *sc)
qp = iowait_to_qp(wait);
priv = qp->priv;
list_del_init(&priv->s_iowait.list);
+ priv->s_iowait.lock = NULL;
/* refcount held until actual wake up */
qps[n++] = qp;
}
diff --git a/drivers/infiniband/hw/hfi1/qp.c b/drivers/infiniband/hw/hfi1/qp.c
index 9fc75e7..d752d67 100644
--- a/drivers/infiniband/hw/hfi1/qp.c
+++ b/drivers/infiniband/hw/hfi1/qp.c
@@ -196,15 +196,18 @@ static void flush_tx_list(struct rvt_qp *qp)
static void flush_iowait(struct rvt_qp *qp)
{
struct hfi1_qp_priv *priv = qp->priv;
- struct hfi1_ibdev *dev = to_idev(qp->ibqp.device);
unsigned long flags;
+ seqlock_t *lock = priv->s_iowait.lock;
- write_seqlock_irqsave(&dev->iowait_lock, flags);
+ if (!lock)
+ return;
+ write_seqlock_irqsave(lock, flags);
if (!list_empty(&priv->s_iowait.list)) {
list_del_init(&priv->s_iowait.list);
+ priv->s_iowait.lock = NULL;
rvt_put_qp(qp);
}
- write_sequnlock_irqrestore(&dev->iowait_lock, flags);
+ write_sequnlock_irqrestore(lock, flags);
}
static inline int opa_mtu_enum_to_int(int mtu)
@@ -543,6 +546,7 @@ static int iowait_sleep(
ibp->rvp.n_dmawait++;
qp->s_flags |= RVT_S_WAIT_DMA_DESC;
list_add_tail(&priv->s_iowait.list, &sde->dmawait);
+ priv->s_iowait.lock = &dev->iowait_lock;
trace_hfi1_qpsleep(qp, RVT_S_WAIT_DMA_DESC);
rvt_get_qp(qp);
}
@@ -964,6 +968,7 @@ void notify_error_qp(struct rvt_qp *qp)
if (!list_empty(&priv->s_iowait.list) && !(qp->s_flags & RVT_S_BUSY)) {
qp->s_flags &= ~RVT_S_ANY_WAIT_IO;
list_del_init(&priv->s_iowait.list);
+ priv->s_iowait.lock = NULL;
rvt_put_qp(qp);
}
write_sequnlock(&dev->iowait_lock);
diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
index f2f6b5a..9c3c237 100644
--- a/drivers/infiniband/hw/hfi1/verbs.c
+++ b/drivers/infiniband/hw/hfi1/verbs.c
@@ -694,6 +694,7 @@ static void mem_timer(unsigned long data)
qp = iowait_to_qp(wait);
priv = qp->priv;
list_del_init(&priv->s_iowait.list);
+ priv->s_iowait.lock = NULL;
/* refcount held until actual wake up */
if (!list_empty(list))
mod_timer(&dev->mem_timer, jiffies + 1);
@@ -769,6 +770,7 @@ static int wait_kmem(struct hfi1_ibdev *dev,
mod_timer(&dev->mem_timer, jiffies + 1);
qp->s_flags |= RVT_S_WAIT_KMEM;
list_add_tail(&priv->s_iowait.list, &dev->memwait);
+ priv->s_iowait.lock = &dev->iowait_lock;
trace_hfi1_qpsleep(qp, RVT_S_WAIT_KMEM);
rvt_get_qp(qp);
}
@@ -980,6 +982,7 @@ static int pio_wait(struct rvt_qp *qp,
qp->s_flags |= flag;
was_empty = list_empty(&sc->piowait);
list_add_tail(&priv->s_iowait.list, &sc->piowait);
+ priv->s_iowait.lock = &dev->iowait_lock;
trace_hfi1_qpsleep(qp, RVT_S_WAIT_PIO);
rvt_get_qp(qp);
/* counting: only call wantpiobuf_intr if first user */
@@ -1631,6 +1634,7 @@ int hfi1_register_ib_device(struct hfi1_devdata *dd)
setup_timer(&dev->mem_timer, mem_timer, (unsigned long)dev);
seqlock_init(&dev->iowait_lock);
+ seqlock_init(&dev->txwait_lock);
INIT_LIST_HEAD(&dev->txwait);
INIT_LIST_HEAD(&dev->memwait);
diff --git a/drivers/infiniband/hw/hfi1/verbs.h b/drivers/infiniband/hw/hfi1/verbs.h
index 1c3815d..7a8af39 100644
--- a/drivers/infiniband/hw/hfi1/verbs.h
+++ b/drivers/infiniband/hw/hfi1/verbs.h
@@ -180,18 +180,19 @@ struct hfi1_ibdev {
struct rvt_dev_info rdi; /* Must be first */
/* QP numbers are shared by all IB ports */
- /* protect wait lists */
- seqlock_t iowait_lock;
+ /* protect txwait list */
+ seqlock_t txwait_lock ____cacheline_aligned_in_smp;
struct list_head txwait; /* list for wait verbs_txreq */
struct list_head memwait; /* list for wait kernel memory */
- struct list_head txreq_free;
struct kmem_cache *verbs_txreq_cache;
- struct timer_list mem_timer;
+ u64 n_txwait;
+ u64 n_kmem_wait;
+ /* protect iowait lists */
+ seqlock_t iowait_lock ____cacheline_aligned_in_smp;
u64 n_piowait;
u64 n_piodrain;
- u64 n_txwait;
- u64 n_kmem_wait;
+ struct timer_list mem_timer;
#ifdef CONFIG_DEBUG_FS
/* per HFI debugfs */
diff --git a/drivers/infiniband/hw/hfi1/verbs_txreq.c b/drivers/infiniband/hw/hfi1/verbs_txreq.c
index 094ab82..5d23172 100644
--- a/drivers/infiniband/hw/hfi1/verbs_txreq.c
+++ b/drivers/infiniband/hw/hfi1/verbs_txreq.c
@@ -72,22 +72,22 @@ void hfi1_put_txreq(struct verbs_txreq *tx)
kmem_cache_free(dev->verbs_txreq_cache, tx);
do {
- seq = read_seqbegin(&dev->iowait_lock);
+ seq = read_seqbegin(&dev->txwait_lock);
if (!list_empty(&dev->txwait)) {
struct iowait *wait;
- write_seqlock_irqsave(&dev->iowait_lock, flags);
+ write_seqlock_irqsave(&dev->txwait_lock, flags);
wait = list_first_entry(&dev->txwait, struct iowait,
list);
qp = iowait_to_qp(wait);
priv = qp->priv;
list_del_init(&priv->s_iowait.list);
/* refcount held until actual wake up */
- write_sequnlock_irqrestore(&dev->iowait_lock, flags);
+ write_sequnlock_irqrestore(&dev->txwait_lock, flags);
hfi1_qp_wakeup(qp, RVT_S_WAIT_TX);
break;
}
- } while (read_seqretry(&dev->iowait_lock, seq));
+ } while (read_seqretry(&dev->txwait_lock, seq));
}
struct verbs_txreq *__get_txreq(struct hfi1_ibdev *dev,
@@ -96,7 +96,7 @@ struct verbs_txreq *__get_txreq(struct hfi1_ibdev *dev,
{
struct verbs_txreq *tx = ERR_PTR(-EBUSY);
- write_seqlock(&dev->iowait_lock);
+ write_seqlock(&dev->txwait_lock);
if (ib_rvt_state_ops[qp->state] & RVT_PROCESS_RECV_OK) {
struct hfi1_qp_priv *priv;
@@ -108,13 +108,14 @@ struct verbs_txreq *__get_txreq(struct hfi1_ibdev *dev,
dev->n_txwait++;
qp->s_flags |= RVT_S_WAIT_TX;
list_add_tail(&priv->s_iowait.list, &dev->txwait);
+ priv->s_iowait.lock = &dev->txwait_lock;
trace_hfi1_qpsleep(qp, RVT_S_WAIT_TX);
rvt_get_qp(qp);
}
qp->s_flags &= ~RVT_S_BUSY;
}
out:
- write_sequnlock(&dev->iowait_lock);
+ write_sequnlock(&dev->txwait_lock);
return tx;
}
--
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 0/6] IB/hfi1,rdmavt: Few more for 4.9 incl stable fix
From: Dennis Dalessandro @ 2016-10-10 13:14 UTC (permalink / raw)
To: dledford
Cc: Mike Marciniszyn, Dean Luick, Jakub Pawlak, Tadeusz Struk,
linux-rdma, Stable, Sebastian Sanchez, Jianxin Xiong
Doug,
Here are a couple more patches we have collected for 4.9. There is a stable fix
here that fixes a breakage in IPoIB introduced by a change in 4.8.
Patches apply on your k.o/for-4.9-hfi1 branch.
Can also be found in GitHub repo at:
https://github.com/ddalessa/kernel/tree/for-4.9
---
Dennis Dalessandro (1):
IB/rdmavt: rdmavt can handle non aligned page maps
Jakub Pawlak (1):
IB/hfi1: Fix integrity check flags default values
Mike Marciniszyn (3):
IB/hfi1: Add unique txwait_lock for txreq events
IB/hfi1: Inline sdma_txclean() for verbs pio
IB/hfi1: Optimize lkey validation structures
Tadeusz Struk (1):
IB/hfi1: Remove redundant sysfs irq affinity entry
drivers/infiniband/hw/hfi1/affinity.c | 72 ------------------------------
drivers/infiniband/hw/hfi1/affinity.h | 4 --
drivers/infiniband/hw/hfi1/hfi.h | 40 +++++++++++------
drivers/infiniband/hw/hfi1/iowait.h | 8 +++
drivers/infiniband/hw/hfi1/pio.c | 14 ++----
drivers/infiniband/hw/hfi1/qp.c | 11 +++--
drivers/infiniband/hw/hfi1/sdma.c | 37 +++++----------
drivers/infiniband/hw/hfi1/sdma.h | 12 ++++-
drivers/infiniband/hw/hfi1/sysfs.c | 25 ----------
drivers/infiniband/hw/hfi1/verbs.c | 4 ++
drivers/infiniband/hw/hfi1/verbs.h | 13 +++--
drivers/infiniband/hw/hfi1/verbs_txreq.c | 13 +++--
drivers/infiniband/sw/rdmavt/dma.c | 3 -
drivers/infiniband/sw/rdmavt/mr.c | 10 ++--
include/rdma/rdmavt_mr.h | 10 +++-
15 files changed, 97 insertions(+), 179 deletions(-)
--
-Denny
^ permalink raw reply
* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Parav Pandit @ 2016-10-10 13:13 UTC (permalink / raw)
To: Tejun Heo
Cc: Leon Romanovsky, cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma,
Li Zefan, Johannes Weiner, Doug Ledford, Christoph Hellwig,
Liran Liss, Hefty, Sean, Jason Gunthorpe, Haggai Eran,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz, Matan Barak
In-Reply-To: <20161010122545.GA27360-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>
Hi Tejun,
On Mon, Oct 10, 2016 at 5:55 PM, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> Hello, Parav.
>
> On Mon, Oct 10, 2016 at 11:59:45AM +0530, Parav Pandit wrote:
>> >> Weight can be in range of 1 to 10,000 similar to cpu cgroup.
>> >
>> > This is exactly what I don't like, the percentage will remove from the
>> > user the translation needs between weight and actual limitation.
>> >
>> > IMHO CPU used weights because everything there is in weights :).
>>
>> I admit weight are not very intuitive, I was aligning to the existing
>> other cgroup interfaces which achieves similar functionality.
>> I will let Tejun approve the "percentage" or "ratio" new file
>> interface as its little different than weight.
>
> So, if there is gonna be a proportional control mechanism, it should
> use the same interface convention as other proportional controls.
Thats what I was suggesting to Leon.
> Also, I don't get what you mean by using percentage and when people
> brought up this idea, it always has been stemming from
> misunderstanding. Can you please elaborate how percentage based
> proportional control would work? What would 100% mean when cgroups
> can come and go?
When 100% is given to one cgroup, all resources of all type can be
charged by processes of that cgroup.
Resources are stateful resource. So when cgroup goes away, they go
back to global pool (or hw).
Giving 100% to two cgroups is configuration error anyway (or without config).
>
> If you're suggesting expressing absolute limits in terms of
> percentage, that is not proportional control. That's just using a
> different unit for absolute resource limits and it must not be called
> weight.
>
>> weight or percentage helps in abstracting as starting point. So I like
>> to add it too.
>
> Way back, when rdmacg was proposed first, I asked the same question -
> whether there can be a higher level abstraction for rdma resources,
> and, IIRC, the collective answer was that the there can be no
> universal measure of resources in the kernel because a large part of
> resource management actually takes place in userspace.
>
Yes. This still holds true. I don't think anything changed.
proportional knob by kernel provides constrained know of equally
distribute all type of resources.
This works only on one class of application. Thats the primary reason
we had knob for verb level well defined resource.
(Which is subset of what patchv12 had to offer, where such
configuration can be done by the user space).
What is not part of patchv12 is - currently max limit is configured as
"max", instead of real max value.
Due to which user space tool is unable to configure exact value to
program to achieve functionality of ratio.
This new functionality can be done post this patch as this is
incremental functionality for user space tools.
As you know weight configuration allows automatic increase/decrease of
resource to other cgroups when one of them go away, as opposed to
absolute value. How this is going to work in exact terms for stateful
resource, we don't know yet. I haven't though through the design
either from kernel side. So just started exploring if weight interface
can be leveraged.
> If I misunderstood, please correct me but what's being suggested here
> seems to be implementing the knob in rdamcg and letting the specific
> drivers worry about the actual resource provisioning and even then
> there doesn't seem to be a clear way of semantically defining what
> ratios would mean.
Nop. Thats not true.
(a) Every new resource has to be defined in cgroup_rdma.h
(b) charge()/uncharge() has to happen by the cgroup for each.
(c) Letting drivers do will make things fall apart. There are no APIs
exposed either to let drivers know process cgroup either. There is no
intention either.
(d) ratio means -if adapter has
100 resources of type - A,
80 resource of type - B,
10% for cgroup-1 means,
10 resource of type - A
8 resource of type - B
>
> Let's please first establish what the resource control would exactly
> mean.
>
> Thanks.
>
> --
> tejun
^ permalink raw reply
* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Tejun Heo @ 2016-10-10 12:25 UTC (permalink / raw)
To: Parav Pandit
Cc: Leon Romanovsky, cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma,
Li Zefan, Johannes Weiner, Doug Ledford, Christoph Hellwig,
Liran Liss, Hefty, Sean, Jason Gunthorpe, Haggai Eran,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz, Matan Barak
In-Reply-To: <CAG53R5UM6nSTZ7=0S9reKGX45CpNBi8soSDVZyXkN-z0_XXWWQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hello, Parav.
On Mon, Oct 10, 2016 at 11:59:45AM +0530, Parav Pandit wrote:
> >> Weight can be in range of 1 to 10,000 similar to cpu cgroup.
> >
> > This is exactly what I don't like, the percentage will remove from the
> > user the translation needs between weight and actual limitation.
> >
> > IMHO CPU used weights because everything there is in weights :).
>
> I admit weight are not very intuitive, I was aligning to the existing
> other cgroup interfaces which achieves similar functionality.
> I will let Tejun approve the "percentage" or "ratio" new file
> interface as its little different than weight.
So, if there is gonna be a proportional control mechanism, it should
use the same interface convention as other proportional controls.
Also, I don't get what you mean by using percentage and when people
brought up this idea, it always has been stemming from
misunderstanding. Can you please elaborate how percentage based
proportional control would work? What would 100% mean when cgroups
can come and go?
If you're suggesting expressing absolute limits in terms of
percentage, that is not proportional control. That's just using a
different unit for absolute resource limits and it must not be called
weight.
> weight or percentage helps in abstracting as starting point. So I like
> to add it too.
Way back, when rdmacg was proposed first, I asked the same question -
whether there can be a higher level abstraction for rdma resources,
and, IIRC, the collective answer was that the there can be no
universal measure of resources in the kernel because a large part of
resource management actually takes place in userspace.
If I misunderstood, please correct me but what's being suggested here
seems to be implementing the knob in rdamcg and letting the specific
drivers worry about the actual resource provisioning and even then
there doesn't seem to be a clear way of semantically defining what
ratios would mean.
Let's please first establish what the resource control would exactly
mean.
Thanks.
--
tejun
--
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
* Re: [PATCH 26/28] Avoid gcc 5.4 warning -Wunused-result
From: Yishai Hadas @ 2016-10-10 10:06 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Devesh Sharma,
Hal Rosenstock, Mike Marciniszyn, Moni Shoua, Sean Hefty,
Steve Wise, Tatyana Nikolova, Vladimir Sokolovsky, Yishai Hadas
In-Reply-To: <20161009231936.GA24139-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On 10/10/2016 2:19 AM, Jason Gunthorpe wrote:
> On Sun, Oct 09, 2016 at 04:31:17PM +0300, Yishai Hadas wrote:
>
>>> - if (write(connfd, msg, sizeof msg) != sizeof msg) {
>>> - fprintf(stderr, "Couldn't send local address\n");
>>> + if (write(connfd, msg, sizeof msg) != sizeof msg ||
>>> + read(connfd, msg, sizeof msg) != sizeof msg) {
>>> + fprintf(stderr, "Couldn't send/recv local address\n");
>>
>> At that step the server expects to read a "done" response from its client,
>> checking whether the read was done for sizeof msg which is much larger will
>> fail.
>
> This OK?
Yes, below patch solves the issue.
> From db525af53140c3b7604ab45406ed8845cb6171e1 Mon Sep 17 00:00:00 2001
> From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Date: Sun, 9 Oct 2016 17:17:54 -0600
> Subject: [PATCH] verbs: Fix read error check in pingpong
>
> Turns out these reads rely on the short readed caused by EOS.
>
> Fixes: f3912df771db (Avoid gcc 5.4 warning -Wunused-result)
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
> libibverbs/examples/rc_pingpong.c | 2 +-
> libibverbs/examples/srq_pingpong.c | 2 +-
> libibverbs/examples/uc_pingpong.c | 2 +-
> libibverbs/examples/ud_pingpong.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c
> index c92e551c00e6..aca7bac4491b 100644
> --- a/libibverbs/examples/rc_pingpong.c
> +++ b/libibverbs/examples/rc_pingpong.c
> @@ -309,7 +309,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
> sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
> my_dest->psn, gid);
> if (write(connfd, msg, sizeof msg) != sizeof msg ||
> - read(connfd, msg, sizeof msg) != sizeof msg) {
> + read(connfd, msg, sizeof msg) != sizeof "done") {
> fprintf(stderr, "Couldn't send/recv local address\n");
> free(rem_dest);
> rem_dest = NULL;
> diff --git a/libibverbs/examples/srq_pingpong.c b/libibverbs/examples/srq_pingpong.c
> index 863ff10dd0f4..8f55d78e87d4 100644
> --- a/libibverbs/examples/srq_pingpong.c
> +++ b/libibverbs/examples/srq_pingpong.c
> @@ -333,7 +333,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
> }
> }
>
> - if (read(connfd, msg, sizeof msg) != sizeof msg) {
> + if (read(connfd, msg, sizeof msg) != sizeof "done") {
> perror("client write");
> free(rem_dest);
> rem_dest = NULL;
> diff --git a/libibverbs/examples/uc_pingpong.c b/libibverbs/examples/uc_pingpong.c
> index 2b105b947cf3..b565bacaff2a 100644
> --- a/libibverbs/examples/uc_pingpong.c
> +++ b/libibverbs/examples/uc_pingpong.c
> @@ -283,7 +283,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
> sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
> my_dest->psn, gid);
> if (write(connfd, msg, sizeof msg) != sizeof msg ||
> - read(connfd, msg, sizeof msg) != sizeof msg) {
> + read(connfd, msg, sizeof msg) != sizeof "done") {
> fprintf(stderr, "Couldn't send/recv local address\n");
> free(rem_dest);
> rem_dest = NULL;
> diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c
> index d0cd73cc0fae..ddb68cf8624b 100644
> --- a/libibverbs/examples/ud_pingpong.c
> +++ b/libibverbs/examples/ud_pingpong.c
> @@ -280,7 +280,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
> sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
> my_dest->psn, gid);
> if (write(connfd, msg, sizeof msg) != sizeof msg ||
> - read(connfd, msg, sizeof msg) != sizeof msg) {
> + read(connfd, msg, sizeof msg) != sizeof "done") {
> fprintf(stderr, "Couldn't send/recv local address\n");
> free(rem_dest);
> rem_dest = NULL;
>
--
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
* [PATCH 3/3] iwpmd: use ccan list.h for mapped_ports
From: Christoph Hellwig @ 2016-10-10 9:41 UTC (permalink / raw)
To: Tatyana.E.Nikolova-ral2JQCrhuEAvxtiuMwx3w,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
robert.o.sharp-ral2JQCrhuEAvxtiuMwx3w
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476092514-18188-1-git-send-email-hch-jcswGhMUV9g@public.gmane.org>
Also ѕtop passing mappped_ports ports as an argument to some function
and always use the global, which allows making mapped_ports static
in iwarp_pm_helper.c.
Now that no user of the iwpm_list infrastructure are left remove it.
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
iwpmd/iwarp_pm.h | 26 ++++----------
iwpmd/iwarp_pm_helper.c | 92 +++++++++----------------------------------------
iwpmd/iwarp_pm_server.c | 27 +++++++--------
3 files changed, 35 insertions(+), 110 deletions(-)
diff --git a/iwpmd/iwarp_pm.h b/iwpmd/iwarp_pm.h
index 48c9f80..d282029 100644
--- a/iwpmd/iwarp_pm.h
+++ b/iwpmd/iwarp_pm.h
@@ -132,18 +132,8 @@ typedef union sockaddr_union {
struct sockaddr_nl nl_sockaddr;
} sockaddr_union;
-enum {
- IWPM_LIST_MAPPED_PORTS,
-};
-
-typedef struct iwpm_list {
- struct iwpm_list * next;
- struct iwpm_list * prev;
-} iwpm_list;
-
typedef struct iwpm_mapped_port {
- struct iwpm_mapped_port * next;
- struct iwpm_mapped_port * prev;
+ struct list_node entry;
int owner_client;
int sd;
struct sockaddr_storage local_addr;
@@ -252,15 +242,15 @@ iwpm_mapped_port *create_iwpm_mapped_port(struct sockaddr_storage *, int);
iwpm_mapped_port *reopen_iwpm_mapped_port(struct sockaddr_storage *, struct sockaddr_storage *, int);
-void add_iwpm_mapped_port(iwpm_mapped_port **, iwpm_mapped_port *);
+void add_iwpm_mapped_port(iwpm_mapped_port *);
-iwpm_mapped_port *find_iwpm_mapping(iwpm_mapped_port *, struct sockaddr_storage *, int);
+iwpm_mapped_port *find_iwpm_mapping(struct sockaddr_storage *, int);
-iwpm_mapped_port *find_iwpm_same_mapping(iwpm_mapped_port *, struct sockaddr_storage *, int);
+iwpm_mapped_port *find_iwpm_same_mapping(struct sockaddr_storage *, int);
-void remove_iwpm_mapped_port(iwpm_mapped_port **, iwpm_mapped_port *);
+void remove_iwpm_mapped_port(iwpm_mapped_port *);
-void print_iwpm_mapped_ports(iwpm_mapped_port *);
+void print_iwpm_mapped_ports(void);
void free_iwpm_port(iwpm_mapped_port *);
@@ -284,10 +274,6 @@ int add_iwpm_pending_msg(iwpm_send_msg *);
int check_same_sockaddr(struct sockaddr_storage *, struct sockaddr_storage *);
-void add_list_element(iwpm_list **, iwpm_list **, int);
-
-void remove_list_element(iwpm_list **, iwpm_list *, int);
-
void free_iwpm_mapped_ports(void);
extern struct list_head pending_messages;
diff --git a/iwpmd/iwarp_pm_helper.c b/iwpmd/iwarp_pm_helper.c
index 8aa8e40..dcd1ca9 100644
--- a/iwpmd/iwarp_pm_helper.c
+++ b/iwpmd/iwarp_pm_helper.c
@@ -33,8 +33,7 @@
#include "iwarp_pm.h"
-extern iwpm_mapped_port *mapped_ports;
-extern iwpm_mapped_port *pending_ports;
+static LIST_HEAD(mapped_ports); /* list of mapped ports */
extern pthread_cond_t cond_req_complete;
extern pthread_mutex_t map_req_mutex;
@@ -417,10 +416,9 @@ reopen_mapped_port_error:
/**
* add_iwpm_mapped_port - Add mapping to a global list
- * @iwpm_ports: list where to save the mapping
* @iwpm_port: mapping to be saved
*/
-void add_iwpm_mapped_port(iwpm_mapped_port **iwpm_ports, iwpm_mapped_port *iwpm_port)
+void add_iwpm_mapped_port(iwpm_mapped_port *iwpm_port)
{
static int dbg_idx = 1;
iwpm_debug(IWARP_PM_ALL_DBG, "add_iwpm_mapped_port: Adding a new mapping #%d\n", dbg_idx++);
@@ -429,8 +427,7 @@ void add_iwpm_mapped_port(iwpm_mapped_port **iwpm_ports, iwpm_mapped_port *iwpm_
if (iwpm_port->ref_cnt > 1)
return;
}
- add_list_element((iwpm_list **)iwpm_ports, (iwpm_list **)&iwpm_port,
- IWPM_LIST_MAPPED_PORTS);
+ list_add(&mapped_ports, &iwpm_port->entry);
}
/**
@@ -474,7 +471,6 @@ int check_same_sockaddr(struct sockaddr_storage *sockaddr_a, struct sockaddr_sto
/**
* find_iwpm_mapping - Find saved mapped port object
- * @iwpm_ports: list of mapped port object
* @search_addr: IP address and port to search for in the list
* @not_mapped: if set, compare local addresses, otherwise compare mapped addresses
*
@@ -482,13 +478,13 @@ int check_same_sockaddr(struct sockaddr_storage *sockaddr_a, struct sockaddr_sto
* to find a saved port object with the sockaddr or
* a wild card address with the same tcp port
*/
-iwpm_mapped_port *find_iwpm_mapping(iwpm_mapped_port *iwpm_ports,
- struct sockaddr_storage *search_addr, int not_mapped)
+iwpm_mapped_port *find_iwpm_mapping(struct sockaddr_storage *search_addr,
+ int not_mapped)
{
iwpm_mapped_port *iwpm_port, *saved_iwpm_port = NULL;
struct sockaddr_storage *current_addr;
- for (iwpm_port = iwpm_ports; iwpm_port != NULL; iwpm_port = iwpm_port->next) {
+ list_for_each(&mapped_ports, iwpm_port, entry) {
current_addr = (not_mapped)? &iwpm_port->local_addr : &iwpm_port->mapped_addr;
if (get_sockaddr_port(search_addr) == get_sockaddr_port(current_addr)) {
@@ -505,20 +501,19 @@ find_mapping_exit:
/**
* find_iwpm_same_mapping - Find saved mapped port object
- * @iwpm_ports: list of mapped port object
* @search_addr: IP address and port to search for in the list
* @not_mapped: if set, compare local addresses, otherwise compare mapped addresses
*
* Compares the search_sockaddr to the addresses in the list,
* to find a saved port object with the same sockaddr
*/
-iwpm_mapped_port *find_iwpm_same_mapping(iwpm_mapped_port *iwpm_ports,
- struct sockaddr_storage *search_addr, int not_mapped)
+iwpm_mapped_port *find_iwpm_same_mapping(struct sockaddr_storage *search_addr,
+ int not_mapped)
{
iwpm_mapped_port *iwpm_port, *saved_iwpm_port = NULL;
struct sockaddr_storage *current_addr;
- for (iwpm_port = iwpm_ports; iwpm_port != NULL; iwpm_port = iwpm_port->next) {
+ list_for_each(&mapped_ports, iwpm_port, entry) {
current_addr = (not_mapped)? &iwpm_port->local_addr : &iwpm_port->mapped_addr;
if (check_same_sockaddr(search_addr, current_addr)) {
saved_iwpm_port = iwpm_port;
@@ -556,29 +551,27 @@ void free_iwpm_port(iwpm_mapped_port *iwpm_port)
/**
* remove_iwpm_mapped_port - Remove a mapping from a global list
- * @iwpm_ports: list from which the mapping needs to be removed
* @iwpm_port: mapping to be removed
*
* Called only by the main iwarp port mapper thread
*/
-void remove_iwpm_mapped_port(iwpm_mapped_port **iwpm_ports, iwpm_mapped_port *iwpm_port)
+void remove_iwpm_mapped_port(iwpm_mapped_port *iwpm_port)
{
static int dbg_idx = 1;
iwpm_debug(IWARP_PM_ALL_DBG, "remove_iwpm_mapped_port: index = %d\n", dbg_idx++);
- remove_list_element((iwpm_list **)iwpm_ports, (iwpm_list *)iwpm_port,
- IWPM_LIST_MAPPED_PORTS);
+ list_del(&iwpm_port->entry);
}
-void print_iwpm_mapped_ports(iwpm_mapped_port *iwpm_ports)
+void print_iwpm_mapped_ports(void)
{
iwpm_mapped_port *iwpm_port;
- int i;
+ int i = 0;
syslog(LOG_WARNING, "print_iwpm_mapped_ports:\n");
- for (iwpm_port = iwpm_ports, i = 0; iwpm_port != NULL; iwpm_port = iwpm_port->next, i++) {
- syslog(LOG_WARNING, "Mapping #%d\n", i);
+ list_for_each(&mapped_ports, iwpm_port, entry) {
+ syslog(LOG_WARNING, "Mapping #%d\n", i++);
print_iwpm_sockaddr(&iwpm_port->local_addr, "Local address", IWARP_PM_DEBUG);
print_iwpm_sockaddr(&iwpm_port->mapped_addr, "Mapped address", IWARP_PM_DEBUG);
}
@@ -616,55 +609,6 @@ int add_iwpm_pending_msg(iwpm_send_msg *send_msg)
return 0;
}
-/*
- * assign_list_head - Make list_element the first element in the list
- * (i.e. *list = *list_element)
- */
-static void assign_list_head(iwpm_list **list, iwpm_list *list_element, int list_type)
-{
- iwpm_mapped_port **ports;
-
- switch (list_type) {
- case IWPM_LIST_MAPPED_PORTS:
- ports = (iwpm_mapped_port **)list;
- *ports = (iwpm_mapped_port *)list_element;
- break;
- default:
- break;
- }
-}
-
-/**
- * add_list_element - Add element to a doubly linked list
- */
-void add_list_element(iwpm_list **list, iwpm_list **list_element, int list_type)
-{
- /* add element to the beginning of the list*/
- (*list_element)->next = *list;
- if (*list)
- (*list)->prev = *list_element;
- (*list_element)->prev = NULL;
- assign_list_head(list, *list_element, list_type);
-}
-
-/**
- * remove_list_element - Remove element from a doubly linked list
- */
-void remove_list_element(iwpm_list **list, iwpm_list *list_element, int list_type)
-{
- /* remove element from the list */
- if (list_element->prev) {
- list_element->prev->next = list_element->next;
- if (list_element->next)
- list_element->next->prev = list_element->prev;
- } else {
- /* remove first element */
- assign_list_head(list, list_element->next, list_type);
- if (*list)
- (*list)->prev = NULL;
- }
-}
-
/**
* free_iwpm_mapped_ports - Free all iwpm mapped port objects
*/
@@ -672,10 +616,6 @@ void free_iwpm_mapped_ports(void)
{
iwpm_mapped_port *iwpm_port;
- while (mapped_ports) {
- iwpm_port = mapped_ports;
- mapped_ports = mapped_ports->next;
+ while ((iwpm_port = list_pop(&mapped_ports, iwpm_mapped_port, entry)))
free_iwpm_port(iwpm_port);
- }
- mapped_ports = NULL;
}
diff --git a/iwpmd/iwarp_pm_server.c b/iwpmd/iwarp_pm_server.c
index db73c6f..a9ca685 100644
--- a/iwpmd/iwarp_pm_server.c
+++ b/iwpmd/iwarp_pm_server.c
@@ -37,7 +37,6 @@
const char iwpm_ulib_name [] = "iWarpPortMapperUser";
int iwpm_version = 3;
-iwpm_mapped_port *mapped_ports = NULL; /* list of mapped ports */
LIST_HEAD(mapping_reqs); /* list of map tracking objects */
LIST_HEAD(pending_messages); /* list of pending wire messages */
iwpm_client client_list[IWARP_PM_MAX_CLIENTS];/* list of iwarp port mapper clients */
@@ -325,7 +324,7 @@ static int process_iwpm_add_mapping(struct nlmsghdr *req_nlh, int client_idx, in
}
local_addr = (struct sockaddr_storage *)nla_data(nltb[IWPM_NLA_MANAGE_ADDR]);
- iwpm_port = find_iwpm_mapping(mapped_ports, local_addr, not_mapped);
+ iwpm_port = find_iwpm_mapping(local_addr, not_mapped);
if (iwpm_port) {
if (check_same_sockaddr(local_addr, &iwpm_port->local_addr) && iwpm_port->wcard) {
iwpm_port->ref_cnt++;
@@ -366,7 +365,7 @@ static int process_iwpm_add_mapping(struct nlmsghdr *req_nlh, int client_idx, in
goto add_mapping_free_error;
}
/* add the new mapping to the list */
- add_iwpm_mapped_port(&mapped_ports, iwpm_port);
+ add_iwpm_mapped_port(iwpm_port);
nlmsg_free(resp_nlmsg);
return 0;
@@ -432,7 +431,7 @@ static int process_iwpm_query_mapping(struct nlmsghdr *req_nlh, int client_idx,
local_addr = (struct sockaddr_storage *)nla_data(nltb[IWPM_NLA_QUERY_LOCAL_ADDR]);
remote_addr = (struct sockaddr_storage *)nla_data(nltb[IWPM_NLA_QUERY_REMOTE_ADDR]);
- iwpm_port = find_iwpm_mapping(mapped_ports, local_addr, not_mapped);
+ iwpm_port = find_iwpm_mapping(local_addr, not_mapped);
if (iwpm_port) {
err_code = IWPM_DUPLICATE_MAPPING_ERR;
str_err = "Duplicate mapped port";
@@ -497,7 +496,7 @@ static int process_iwpm_query_mapping(struct nlmsghdr *req_nlh, int client_idx,
form_iwpm_send_msg(pm_client_sock, &dest_addr.s_sockaddr, msg_parms.msize, send_msg);
add_iwpm_map_request(iwpm_map_req);
- add_iwpm_mapped_port(&mapped_ports, iwpm_port);
+ add_iwpm_mapped_port(iwpm_port);
return send_iwpm_msg(form_iwpm_request, &msg_parms, &dest_addr.s_sockaddr, pm_client_sock);
query_mapping_free_error:
if (iwpm_port)
@@ -542,7 +541,7 @@ static int process_iwpm_remove_mapping(struct nlmsghdr *req_nlh, int client_idx,
iwpm_debug(IWARP_PM_NETLINK_DBG, "process_remove_mapping: Going to remove mapping"
" (client idx = %d)\n", client_idx);
- iwpm_port = find_iwpm_same_mapping(mapped_ports, local_addr, not_mapped);
+ iwpm_port = find_iwpm_same_mapping(local_addr, not_mapped);
if (!iwpm_port) {
iwpm_debug(IWARP_PM_NETLINK_DBG, "process_remove_mapping: Unable to find mapped port object\n");
print_iwpm_sockaddr(local_addr, "process_remove_mapping: Local address", IWARP_PM_ALL_DBG);
@@ -560,7 +559,7 @@ static int process_iwpm_remove_mapping(struct nlmsghdr *req_nlh, int client_idx,
if (ref_cnt)
goto remove_mapping_exit;
}
- remove_iwpm_mapped_port(&mapped_ports, iwpm_port);
+ remove_iwpm_mapped_port(iwpm_port);
free_iwpm_port(iwpm_port);
remove_mapping_exit:
return ret;
@@ -641,7 +640,7 @@ static int process_iwpm_wire_request(iwpm_msg_parms *msg_parms, int nl_sock,
copy_iwpm_sockaddr(msg_parms->address_family, NULL, &local_addr,
&msg_parms->apipaddr[0], NULL, &msg_parms->apport);
- iwpm_port = find_iwpm_mapping(mapped_ports, &local_addr, not_mapped);
+ iwpm_port = find_iwpm_mapping(&local_addr, not_mapped);
if (!iwpm_port) {
/* could not find mapping for the requested address */
iwpm_debug(IWARP_PM_WIRE_DBG, "process_wire_request: "
@@ -737,7 +736,7 @@ static int process_iwpm_wire_accept(iwpm_msg_parms *msg_parms, int nl_sock,
copy_iwpm_sockaddr(msg_parms->address_family, NULL, &remote_mapped_addr,
&msg_parms->apipaddr[0], NULL, &msg_parms->apport);
ret = -EINVAL;
- iwpm_port = find_iwpm_same_mapping(mapped_ports, &local_addr, not_mapped);
+ iwpm_port = find_iwpm_same_mapping(&local_addr, not_mapped);
if (!iwpm_port) {
iwpm_debug(IWARP_PM_WIRE_DBG, "process_wire_accept: "
"Received accept for unknown mapping.\n");
@@ -812,7 +811,7 @@ static int process_iwpm_wire_reject(iwpm_msg_parms *msg_parms, int nl_sock)
print_iwpm_sockaddr(&remote_addr, "process_wire_reject: Remote address",
IWARP_PM_ALL_DBG);
ret = -EINVAL;
- iwpm_port = find_iwpm_same_mapping(mapped_ports, &local_addr, not_mapped);
+ iwpm_port = find_iwpm_same_mapping(&local_addr, not_mapped);
if (!iwpm_port) {
syslog(LOG_WARNING, "process_wire_reject: Received reject for unknown mapping.\n");
return 0;
@@ -857,7 +856,7 @@ static int process_iwpm_wire_ack(iwpm_msg_parms *msg_parms)
copy_iwpm_sockaddr(msg_parms->address_family, NULL, &local_mapped_addr,
&msg_parms->apipaddr[0], NULL, &msg_parms->apport);
- iwpm_port = find_iwpm_mapping(mapped_ports, &local_mapped_addr, not_mapped);
+ iwpm_port = find_iwpm_mapping(&local_mapped_addr, not_mapped);
if (!iwpm_port) {
iwpm_debug(IWARP_PM_WIRE_DBG, "process_wire_ack: Received ack for unknown mapping.\n");
return 0;
@@ -905,7 +904,7 @@ static int process_iwpm_mapinfo(struct nlmsghdr *req_nlh, int client_idx, int nl
local_addr = (struct sockaddr_storage *)nla_data(nltb[IWPM_NLA_MAPINFO_LOCAL_ADDR]);
local_mapped_addr = (struct sockaddr_storage *)nla_data(nltb[IWPM_NLA_MAPINFO_MAPPED_ADDR]);
- iwpm_port = find_iwpm_mapping(mapped_ports, local_addr, not_mapped);
+ iwpm_port = find_iwpm_mapping(local_addr, not_mapped);
if (iwpm_port) {
/* Can be safely ignored, if the mapinfo is exactly the same,
* because the client will provide all the port information it has and
@@ -926,7 +925,7 @@ static int process_iwpm_mapinfo(struct nlmsghdr *req_nlh, int client_idx, int nl
goto process_mapinfo_error;
}
/* add the new mapping to the list */
- add_iwpm_mapped_port(&mapped_ports, iwpm_port);
+ add_iwpm_mapped_port(iwpm_port);
process_mapinfo_exit:
mapinfo_num_list[client_idx]++;
return 0;
@@ -1301,7 +1300,7 @@ static int iwarp_port_mapper()
do {
do {
if (print_mappings) {
- print_iwpm_mapped_ports(mapped_ports);
+ print_iwpm_mapped_ports();
print_mappings = 0;
}
/* initialize the file sets for select */
--
2.10.1.382.ga23ca1b
--
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 2/3] iwpmd: use ccan list.h for mapping_reqs
From: Christoph Hellwig @ 2016-10-10 9:41 UTC (permalink / raw)
To: Tatyana.E.Nikolova-ral2JQCrhuEAvxtiuMwx3w,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
robert.o.sharp-ral2JQCrhuEAvxtiuMwx3w
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476092514-18188-1-git-send-email-hch-jcswGhMUV9g@public.gmane.org>
Trivial conversion, nothing to see here.
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
iwpmd/iwarp_pm.h | 5 ++---
iwpmd/iwarp_pm_helper.c | 14 +++-----------
iwpmd/iwarp_pm_server.c | 11 ++++-------
3 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/iwpmd/iwarp_pm.h b/iwpmd/iwarp_pm.h
index 57153cc..48c9f80 100644
--- a/iwpmd/iwarp_pm.h
+++ b/iwpmd/iwarp_pm.h
@@ -134,7 +134,6 @@ typedef union sockaddr_union {
enum {
IWPM_LIST_MAPPED_PORTS,
- IWPM_LIST_MAP_REQUESTS,
};
typedef struct iwpm_list {
@@ -174,8 +173,7 @@ typedef struct iwpm_send_msg {
} iwpm_send_msg;
typedef struct iwpm_mapping_request {
- struct iwpm_mapping_request * next;
- struct iwpm_mapping_request * prev;
+ struct list_node entry;
struct sockaddr_storage src_addr;
struct sockaddr_storage remote_addr;
__u16 nlmsg_type; /* Message content */
@@ -293,5 +291,6 @@ void remove_list_element(iwpm_list **, iwpm_list *, int);
void free_iwpm_mapped_ports(void);
extern struct list_head pending_messages;
+extern struct list_head mapping_reqs;
#endif
diff --git a/iwpmd/iwarp_pm_helper.c b/iwpmd/iwarp_pm_helper.c
index ebcff58..8aa8e40 100644
--- a/iwpmd/iwarp_pm_helper.c
+++ b/iwpmd/iwarp_pm_helper.c
@@ -35,7 +35,6 @@
extern iwpm_mapped_port *mapped_ports;
extern iwpm_mapped_port *pending_ports;
-extern iwpm_mapping_request *mapping_reqs;
extern pthread_cond_t cond_req_complete;
extern pthread_mutex_t map_req_mutex;
@@ -96,8 +95,7 @@ iwpm_mapping_request *create_iwpm_map_request(struct nlmsghdr *req_nlh,
void add_iwpm_map_request(iwpm_mapping_request *iwpm_map_req)
{
pthread_mutex_lock(&map_req_mutex);
- add_list_element((iwpm_list **)&mapping_reqs, (iwpm_list **)&iwpm_map_req,
- IWPM_LIST_MAP_REQUESTS);
+ list_add(&mapping_reqs, &iwpm_map_req->entry);
/* if not wake, signal the thread that a new request has been posted */
if (!wake)
pthread_cond_signal(&cond_req_complete);
@@ -117,8 +115,7 @@ void remove_iwpm_map_request(iwpm_mapping_request *iwpm_map_req)
"Timeout for request (type = %u pid = %d)\n",
iwpm_map_req->msg_type, iwpm_map_req->nlmsg_pid);
}
- remove_list_element((iwpm_list **)&mapping_reqs, (iwpm_list *)iwpm_map_req,
- IWPM_LIST_MAP_REQUESTS);
+ list_del(&iwpm_map_req->entry);
if (iwpm_map_req->send_msg)
free(iwpm_map_req->send_msg);
free(iwpm_map_req);
@@ -140,7 +137,7 @@ int update_iwpm_map_request(__u64 assochandle, struct sockaddr_storage *src_addr
pthread_mutex_lock(&map_req_mutex);
/* look for a matching entry in the list */
- for (iwpm_map_req = mapping_reqs; iwpm_map_req != NULL; iwpm_map_req = iwpm_map_req->next) {
+ list_for_each(&mapping_reqs, iwpm_map_req, entry) {
if (assochandle == iwpm_map_req->assochandle &&
(msg_type & iwpm_map_req->msg_type) &&
check_same_sockaddr(src_addr, &iwpm_map_req->src_addr)) {
@@ -626,17 +623,12 @@ int add_iwpm_pending_msg(iwpm_send_msg *send_msg)
static void assign_list_head(iwpm_list **list, iwpm_list *list_element, int list_type)
{
iwpm_mapped_port **ports;
- iwpm_mapping_request **requests;
switch (list_type) {
case IWPM_LIST_MAPPED_PORTS:
ports = (iwpm_mapped_port **)list;
*ports = (iwpm_mapped_port *)list_element;
break;
- case IWPM_LIST_MAP_REQUESTS:
- requests = (iwpm_mapping_request **)list;
- *requests = (iwpm_mapping_request *)list_element;
- break;
default:
break;
}
diff --git a/iwpmd/iwarp_pm_server.c b/iwpmd/iwarp_pm_server.c
index a45ccbd..db73c6f 100644
--- a/iwpmd/iwarp_pm_server.c
+++ b/iwpmd/iwarp_pm_server.c
@@ -38,7 +38,7 @@ const char iwpm_ulib_name [] = "iWarpPortMapperUser";
int iwpm_version = 3;
iwpm_mapped_port *mapped_ports = NULL; /* list of mapped ports */
-volatile iwpm_mapping_request *mapping_reqs = NULL; /* list of map tracking objects */
+LIST_HEAD(mapping_reqs); /* list of map tracking objects */
LIST_HEAD(pending_messages); /* list of pending wire messages */
iwpm_client client_list[IWARP_PM_MAX_CLIENTS];/* list of iwarp port mapper clients */
int mapinfo_num_list[IWARP_PM_MAX_CLIENTS]; /* list of iwarp port mapper clients */
@@ -96,7 +96,7 @@ void *iwpm_mapping_reqs_handler()
while (1) {
pthread_mutex_lock(&map_req_mutex);
wake = 0;
- if (!mapping_reqs) {
+ if (list_empty(&mapping_reqs)) {
/* wait until a new mapping request is posted */
ret = pthread_cond_wait(&cond_req_complete, &map_req_mutex);
if (ret) {
@@ -111,9 +111,7 @@ void *iwpm_mapping_reqs_handler()
do {
pthread_mutex_lock(&map_req_mutex);
wake = 1;
- iwpm_map_req = (iwpm_mapping_request *)mapping_reqs;
- while (iwpm_map_req) {
- next_map_req = iwpm_map_req->next;
+ list_for_each_safe(&mapping_reqs, iwpm_map_req, next_map_req, entry) {
if (iwpm_map_req->timeout > 0) {
if (iwpm_map_req->timeout < IWPM_MAP_REQ_TIMEOUT &&
iwpm_map_req->msg_type != IWARP_PM_REQ_ACK) {
@@ -130,11 +128,10 @@ void *iwpm_mapping_reqs_handler()
} else {
remove_iwpm_map_request(iwpm_map_req);
}
- iwpm_map_req = next_map_req;
}
pthread_mutex_unlock(&map_req_mutex);
sleep(1);
- } while (mapping_reqs);
+ } while (!list_empty(&mapping_reqs));
}
mapping_reqs_handler_exit:
return NULL;
--
2.10.1.382.ga23ca1b
--
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 1/3] iwpmd: use ccan list.h for pending_messages
From: Christoph Hellwig @ 2016-10-10 9:41 UTC (permalink / raw)
To: Tatyana.E.Nikolova-ral2JQCrhuEAvxtiuMwx3w,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
robert.o.sharp-ral2JQCrhuEAvxtiuMwx3w
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476092514-18188-1-git-send-email-hch-jcswGhMUV9g@public.gmane.org>
The only change in semantics is that we remove the entry from the list
earlier in iwpm_pending_msgs_handler due to using the list_pop helper,
but given that we won't exist the loop early ever this should not change
behavior.
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
iwpmd/iwarp_pm.h | 9 ++++-----
iwpmd/iwarp_pm_helper.c | 22 +---------------------
iwpmd/iwarp_pm_server.c | 9 +++++----
3 files changed, 10 insertions(+), 30 deletions(-)
diff --git a/iwpmd/iwarp_pm.h b/iwpmd/iwarp_pm.h
index 5b7c613..57153cc 100644
--- a/iwpmd/iwarp_pm.h
+++ b/iwpmd/iwarp_pm.h
@@ -52,6 +52,7 @@
#include <pthread.h>
#include <syslog.h>
#include <netlink/msg.h>
+#include <ccan/list.h>
#include "iwpm_netlink.h"
#define IWARP_PM_PORT 3935
@@ -134,7 +135,6 @@ typedef union sockaddr_union {
enum {
IWPM_LIST_MAPPED_PORTS,
IWPM_LIST_MAP_REQUESTS,
- IWPM_LIST_PENDING_MSGS
};
typedef struct iwpm_list {
@@ -189,8 +189,7 @@ typedef struct iwpm_mapping_request {
} iwpm_mapping_request;
typedef struct iwpm_pending_msg {
- struct iwpm_send_msg * next;
- struct iwpm_send_msg * prev;
+ struct list_node entry;
iwpm_send_msg send_msg;
} iwpm_pending_msg;
@@ -285,8 +284,6 @@ int send_iwpm_msg(void (*form_msg_type)(iwpm_wire_msg *, iwpm_msg_parms *),
int add_iwpm_pending_msg(iwpm_send_msg *);
-void remove_iwpm_pending_msg(iwpm_pending_msg *);
-
int check_same_sockaddr(struct sockaddr_storage *, struct sockaddr_storage *);
void add_list_element(iwpm_list **, iwpm_list **, int);
@@ -295,4 +292,6 @@ void remove_list_element(iwpm_list **, iwpm_list *, int);
void free_iwpm_mapped_ports(void);
+extern struct list_head pending_messages;
+
#endif
diff --git a/iwpmd/iwarp_pm_helper.c b/iwpmd/iwarp_pm_helper.c
index 89d2b6c..ebcff58 100644
--- a/iwpmd/iwarp_pm_helper.c
+++ b/iwpmd/iwarp_pm_helper.c
@@ -36,7 +36,6 @@
extern iwpm_mapped_port *mapped_ports;
extern iwpm_mapped_port *pending_ports;
extern iwpm_mapping_request *mapping_reqs;
-extern iwpm_send_msg *pending_messages;
extern pthread_cond_t cond_req_complete;
extern pthread_mutex_t map_req_mutex;
@@ -613,27 +612,13 @@ int add_iwpm_pending_msg(iwpm_send_msg *send_msg)
memcpy(&pending_msg->send_msg, send_msg, sizeof(iwpm_send_msg));
pthread_mutex_lock(&pending_msg_mutex);
- add_list_element((iwpm_list **)&pending_messages, (iwpm_list **)&pending_msg,
- IWPM_LIST_PENDING_MSGS);
+ list_add(&pending_messages, &pending_msg->entry);
pthread_mutex_unlock(&pending_msg_mutex);
/* signal the thread that a new message has been posted */
pthread_cond_signal(&cond_pending_msg);
return 0;
}
-/**
- * remove_iwpm_pending_msg - Free wire message buffer
- * @pending_msg: message to be removed
- *
- * Routine must be called within lock context
- */
-void remove_iwpm_pending_msg(iwpm_pending_msg *pending_msg)
-{
- remove_list_element((iwpm_list **)&pending_messages, (iwpm_list *)pending_msg,
- IWPM_LIST_PENDING_MSGS);
- free(pending_msg);
-}
-
/*
* assign_list_head - Make list_element the first element in the list
* (i.e. *list = *list_element)
@@ -642,7 +627,6 @@ static void assign_list_head(iwpm_list **list, iwpm_list *list_element, int list
{
iwpm_mapped_port **ports;
iwpm_mapping_request **requests;
- iwpm_send_msg **messages;
switch (list_type) {
case IWPM_LIST_MAPPED_PORTS:
@@ -653,10 +637,6 @@ static void assign_list_head(iwpm_list **list, iwpm_list *list_element, int list
requests = (iwpm_mapping_request **)list;
*requests = (iwpm_mapping_request *)list_element;
break;
- case IWPM_LIST_PENDING_MSGS:
- messages = (iwpm_send_msg **)list;
- *messages = (iwpm_send_msg *)list_element;
- break;
default:
break;
}
diff --git a/iwpmd/iwarp_pm_server.c b/iwpmd/iwarp_pm_server.c
index 39e0aa2..a45ccbd 100644
--- a/iwpmd/iwarp_pm_server.c
+++ b/iwpmd/iwarp_pm_server.c
@@ -39,7 +39,7 @@ int iwpm_version = 3;
iwpm_mapped_port *mapped_ports = NULL; /* list of mapped ports */
volatile iwpm_mapping_request *mapping_reqs = NULL; /* list of map tracking objects */
-volatile iwpm_pending_msg *pending_messages = NULL; /* list of pending wire messages */
+LIST_HEAD(pending_messages); /* list of pending wire messages */
iwpm_client client_list[IWARP_PM_MAX_CLIENTS];/* list of iwarp port mapper clients */
int mapinfo_num_list[IWARP_PM_MAX_CLIENTS]; /* list of iwarp port mapper clients */
@@ -160,9 +160,10 @@ void *iwpm_pending_msgs_handler()
pthread_mutex_unlock(&pending_msg_mutex);
goto pending_msgs_handler_exit;
}
+
/* try sending out each pending message and remove it from the list */
- while (pending_messages) {
- pending_msg = (iwpm_pending_msg *)pending_messages;
+ while ((pending_msg = list_pop(&pending_messages,
+ iwpm_pending_msg, entry))) {
retries = IWPM_SEND_MSG_RETRIES;
while (retries) {
send_msg = &pending_msg->send_msg;
@@ -179,7 +180,7 @@ void *iwpm_pending_msgs_handler()
} else
retries = 0; /* no need to retry */
}
- remove_iwpm_pending_msg(pending_msg);
+ free(pending_msg);
}
}
pthread_mutex_unlock(&pending_msg_mutex);
--
2.10.1.382.ga23ca1b
--
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
* switch iwpm to ccan list.h
From: Christoph Hellwig @ 2016-10-10 9:41 UTC (permalink / raw)
To: Tatyana.E.Nikolova-ral2JQCrhuEAvxtiuMwx3w,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW,
robert.o.sharp-ral2JQCrhuEAvxtiuMwx3w
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
I've started running sparse on the rdma-core code, and one of the
worst offenders is the iwpmd list code, so this (untested) patch
set converts it over to the ccan list.h helpers. Reviews and
testing appreciated.
--
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
* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Parav Pandit @ 2016-10-10 9:22 UTC (permalink / raw)
To: Leon Romanovsky
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma, Tejun Heo, Li Zefan,
Johannes Weiner, Doug Ledford, Christoph Hellwig, Liran Liss,
Hefty, Sean, Jason Gunthorpe, Haggai Eran,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz, Matan Barak
In-Reply-To: <20161010085241.GL9282-2ukJVAZIZ/Y@public.gmane.org>
On Mon, Oct 10, 2016 at 2:22 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Oct 10, 2016 at 02:05:27PM +0530, Parav Pandit wrote:
>> On Mon, Oct 10, 2016 at 1:03 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> > On Mon, Oct 10, 2016 at 11:59:45AM +0530, Parav Pandit wrote:
>> >> Hi Leon,
>> >>
>> >> On Mon, Oct 10, 2016 at 10:16 AM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> >> > On Thu, Oct 06, 2016 at 07:19:24PM +0530, Parav Pandit wrote:
>> >> >> Hi Leon,
>> >> >>
>> >> >> On Wed, Oct 5, 2016 at 4:52 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> >> >> > On Wed, Aug 31, 2016 at 02:07:24PM +0530, Parav Pandit wrote:
>> >> >> >> rdmacg: IB/core: rdma controller support
>> >> >> >>
>> >> >> >> Overview:
>> >> >> >> Currently user space applications can easily take away all the rdma
>> >> >> >> device specific resources such as AH, CQ, QP, MR etc. Due to which other
>> >> >> >> applications in other cgroup or kernel space ULPs may not even get chance
>> >> >> >> to allocate any rdma resources. This results into service unavailibility.
>> >> >> >>
>> >> >> >> RDMA cgroup addresses this issue by allowing resource accounting,
>> >> >> >> limit enforcement on per cgroup, per rdma device basis.
>> >> >> >>
>> >> >> >> RDMA uverbs layer will enforce limits on well defined RDMA verb
>> >> >> >> resources without any HCA vendor device driver involvement.
>> >> >> >>
>> >> >> >> RDMA uverbs layer will not do limit enforcement of HCA hw vendor
>> >> >> >> specific resources. Instead rdma cgroup provides set of APIs
>> >> >> >> through which vendor specific drivers can do resource accounting
>> >> >> >> by making use of rdma cgroup.
>> >> >> >
>> >> >> > Hi Parav,
>> >> >> > I want to propose an extension to the RDMA cgroup which can be done as
>> >> >> > follow-up patches.
>> >> >> >
>> >> >> > Let's add new global type, which will control whole HCA (for example in percentages). It will
>> >> >> > allow natively define new objects without need to introduce them to the user.
>> >> >> >
>> >> >> In other cgroup such as CPU, this is done using cpu.weight API. Where
>> >> >> percentage or weight is configured by the user.
>> >> >> In this mode, resources taken away from other cgroup proportionately.
>> >> >> It works for cpu because its mainly stateless resource unlike rdma
>> >> >> resources.
>> >> >> So if we want to simplify user configuration similarly,
>> >> >> percentage/weight configuration can be extended.
>> >> >> This way they need not be introduced to users.
>> >> >> I hope your definition of "user" is actual end-user and not rdma cgroup. Right?
>> >> >
>> >> > Yes, "user" -> "admin".
>> >> > I think that percentage is more intuitive to them and will be much easier to
>> >> > explain how to use it. I always have in mind "swappiness" field and the
>> >> > numerous questions on how to configure it.
>> >> >
>> >> >> In other words, new object should be still added as new enum value in
>> >> >> rdma_cgroup.h?
>> >> >
>> >> > Yes, I had in mind something like IB_CGROUP_HCA, this is why it can be
>> >> > done as a future work after accepting current patches.
>> >> >
>> >> What I meant is,
>> >> today we have RDMACG_VERB_RESOURCE_QP etc,
>> >> We will additionally have RDMACG_VERB_RESOURCE_INDIRECT_TBL etc in
>> >> cgroup_rdma.h.
>> >> So that its available for admin to override it.
>> >
>> > IMHO, we are talking about the same. My global HCA object will be
>> > overwritten by more granular VERBS objects in case they exists.
>> >
>> >>
>> >> >> Only than it can be overwritten by specific UVERBs type as you
>> >> >> described below. I think thats what you meant as you described below.
>> >> >
>> >> > Exactly.
>> >> >
>> >> >>
>> >> >> Otherwise charging/uncharging this new percentage resource can get messy.
>> >> >
>> >> > Agree
>> >> >
>> >> >>
>> >> >> > This HCA share will be overwritten by specific UVERBS types which you
>> >> >> > already defined.
>> >> >> >
>> >> >> > What do you think?
>> >> >>
>> >> >> So to refine your proposal from cgroup perspective, instead of adding
>> >> >> new resource type in rdma_cgroup.h for percentage, I prefer to have
>> >> >>
>> >> >> Existing
>> >> >> 1. rdma.max
>> >> >> 2. rdma.current
>> >> >> New,
>> >> >> 3. rdma.weight
>> >> >> This ABI will have similar API to say
>> >> >> echo "mlx4_0 50" > rdma.weight.
>> >> >> Where 50 is weight of the resources.
>> >> >> For example,
>> >> >> for one cgroup instance weight=sum=100% resource for a given cgroup.
>> >> >> for three cgroup instances percentage=(weight/sum)% = 50/(50+50+50) = 33%.
>> >> >> One cgroup gets 33% resource.
>> >> >>
>> >> >> Weight can be in range of 1 to 10,000 similar to cpu cgroup.
>> >> >
>> >> > This is exactly what I don't like, the percentage will remove from the
>> >> > user the translation needs between weight and actual limitation.
>> >> >
>> >> > IMHO CPU used weights because everything there is in weights :).
>> >> >
>> >> I admit weight are not very intuitive, I was aligning to the existing
>> >> other cgroup interfaces which achieves similar functionality.
>> >> I will let Tejun approve the "percentage" or "ratio" new file
>> >> interface as its little different than weight.
>> >
>> > Sure, let's close the main idea first and see if it makes sense for
>> > other participants.
>> >
>> >>
>> >> >>
>> >> >> This might work if applications running in all cgroups are similar.
>> >> >> But weight doesn't do justice, when there are different type of
>> >> >> applications running in each cgroup. Such as few running libfabric
>> >> >> based apps, few running MPI, others directly using ibverbs.
>> >> >> So as you said rdma.max configuration would be required for management
>> >> >> plane to override weight (percentage) for certain resources.
>> >> >
>> >> > Why?
>> >> > The device exposes max values during initialization and if user asked
>> >> > for 20% percent of HCA, he will get max*0.2.
>> >>
>> >> Because every application may not be equivalent of other application.
>> >> For example, some require one to one QP and PD mapping.
>> >> Some share single PD across multiple QPs.
>> >> Some have ratio of 100 MRs per QP, as factor of memory size and operations.
>> >> some servers like to have 1K MRs per QP.
>> >> So if we have just weight, it will equally distributes MRs per QP in
>> >> all cgroup and that either leads to unused resource per cgroup or,
>> >> lesser number of cg instances.
>> >> So fine tuning required for individual one, which we already have.
>> >
>> > I afraid that it is over complicating which can be done by curious user
>> > in his user-space scripts: limit the global HCA -> read max values ->
>> > overwrite with specific mapping.
>> >
>> >>
>> >> weight or percentage helps in abstracting as starting point. So I like
>> >> to add it too.
>> >
>> > Let's start simple
>>
>> Yes. I will rebase and test my patch today and see if requires resending.
>
> It is worth to wait till -rc1. Doug didn't finish his pull requests yet.
>
Alright. I will wait.
> Thanks
--
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
* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Leon Romanovsky @ 2016-10-10 8:52 UTC (permalink / raw)
To: Parav Pandit
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma, Tejun Heo, Li Zefan,
Johannes Weiner, Doug Ledford, Christoph Hellwig, Liran Liss,
Hefty, Sean, Jason Gunthorpe, Haggai Eran,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz, Matan Barak
In-Reply-To: <CAG53R5WeWSrJ5-Gtt-cXpUr0r73zh3bqQM_G5zTue27tPtVEXA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 6847 bytes --]
On Mon, Oct 10, 2016 at 02:05:27PM +0530, Parav Pandit wrote:
> On Mon, Oct 10, 2016 at 1:03 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Mon, Oct 10, 2016 at 11:59:45AM +0530, Parav Pandit wrote:
> >> Hi Leon,
> >>
> >> On Mon, Oct 10, 2016 at 10:16 AM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >> > On Thu, Oct 06, 2016 at 07:19:24PM +0530, Parav Pandit wrote:
> >> >> Hi Leon,
> >> >>
> >> >> On Wed, Oct 5, 2016 at 4:52 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >> >> > On Wed, Aug 31, 2016 at 02:07:24PM +0530, Parav Pandit wrote:
> >> >> >> rdmacg: IB/core: rdma controller support
> >> >> >>
> >> >> >> Overview:
> >> >> >> Currently user space applications can easily take away all the rdma
> >> >> >> device specific resources such as AH, CQ, QP, MR etc. Due to which other
> >> >> >> applications in other cgroup or kernel space ULPs may not even get chance
> >> >> >> to allocate any rdma resources. This results into service unavailibility.
> >> >> >>
> >> >> >> RDMA cgroup addresses this issue by allowing resource accounting,
> >> >> >> limit enforcement on per cgroup, per rdma device basis.
> >> >> >>
> >> >> >> RDMA uverbs layer will enforce limits on well defined RDMA verb
> >> >> >> resources without any HCA vendor device driver involvement.
> >> >> >>
> >> >> >> RDMA uverbs layer will not do limit enforcement of HCA hw vendor
> >> >> >> specific resources. Instead rdma cgroup provides set of APIs
> >> >> >> through which vendor specific drivers can do resource accounting
> >> >> >> by making use of rdma cgroup.
> >> >> >
> >> >> > Hi Parav,
> >> >> > I want to propose an extension to the RDMA cgroup which can be done as
> >> >> > follow-up patches.
> >> >> >
> >> >> > Let's add new global type, which will control whole HCA (for example in percentages). It will
> >> >> > allow natively define new objects without need to introduce them to the user.
> >> >> >
> >> >> In other cgroup such as CPU, this is done using cpu.weight API. Where
> >> >> percentage or weight is configured by the user.
> >> >> In this mode, resources taken away from other cgroup proportionately.
> >> >> It works for cpu because its mainly stateless resource unlike rdma
> >> >> resources.
> >> >> So if we want to simplify user configuration similarly,
> >> >> percentage/weight configuration can be extended.
> >> >> This way they need not be introduced to users.
> >> >> I hope your definition of "user" is actual end-user and not rdma cgroup. Right?
> >> >
> >> > Yes, "user" -> "admin".
> >> > I think that percentage is more intuitive to them and will be much easier to
> >> > explain how to use it. I always have in mind "swappiness" field and the
> >> > numerous questions on how to configure it.
> >> >
> >> >> In other words, new object should be still added as new enum value in
> >> >> rdma_cgroup.h?
> >> >
> >> > Yes, I had in mind something like IB_CGROUP_HCA, this is why it can be
> >> > done as a future work after accepting current patches.
> >> >
> >> What I meant is,
> >> today we have RDMACG_VERB_RESOURCE_QP etc,
> >> We will additionally have RDMACG_VERB_RESOURCE_INDIRECT_TBL etc in
> >> cgroup_rdma.h.
> >> So that its available for admin to override it.
> >
> > IMHO, we are talking about the same. My global HCA object will be
> > overwritten by more granular VERBS objects in case they exists.
> >
> >>
> >> >> Only than it can be overwritten by specific UVERBs type as you
> >> >> described below. I think thats what you meant as you described below.
> >> >
> >> > Exactly.
> >> >
> >> >>
> >> >> Otherwise charging/uncharging this new percentage resource can get messy.
> >> >
> >> > Agree
> >> >
> >> >>
> >> >> > This HCA share will be overwritten by specific UVERBS types which you
> >> >> > already defined.
> >> >> >
> >> >> > What do you think?
> >> >>
> >> >> So to refine your proposal from cgroup perspective, instead of adding
> >> >> new resource type in rdma_cgroup.h for percentage, I prefer to have
> >> >>
> >> >> Existing
> >> >> 1. rdma.max
> >> >> 2. rdma.current
> >> >> New,
> >> >> 3. rdma.weight
> >> >> This ABI will have similar API to say
> >> >> echo "mlx4_0 50" > rdma.weight.
> >> >> Where 50 is weight of the resources.
> >> >> For example,
> >> >> for one cgroup instance weight=sum=100% resource for a given cgroup.
> >> >> for three cgroup instances percentage=(weight/sum)% = 50/(50+50+50) = 33%.
> >> >> One cgroup gets 33% resource.
> >> >>
> >> >> Weight can be in range of 1 to 10,000 similar to cpu cgroup.
> >> >
> >> > This is exactly what I don't like, the percentage will remove from the
> >> > user the translation needs between weight and actual limitation.
> >> >
> >> > IMHO CPU used weights because everything there is in weights :).
> >> >
> >> I admit weight are not very intuitive, I was aligning to the existing
> >> other cgroup interfaces which achieves similar functionality.
> >> I will let Tejun approve the "percentage" or "ratio" new file
> >> interface as its little different than weight.
> >
> > Sure, let's close the main idea first and see if it makes sense for
> > other participants.
> >
> >>
> >> >>
> >> >> This might work if applications running in all cgroups are similar.
> >> >> But weight doesn't do justice, when there are different type of
> >> >> applications running in each cgroup. Such as few running libfabric
> >> >> based apps, few running MPI, others directly using ibverbs.
> >> >> So as you said rdma.max configuration would be required for management
> >> >> plane to override weight (percentage) for certain resources.
> >> >
> >> > Why?
> >> > The device exposes max values during initialization and if user asked
> >> > for 20% percent of HCA, he will get max*0.2.
> >>
> >> Because every application may not be equivalent of other application.
> >> For example, some require one to one QP and PD mapping.
> >> Some share single PD across multiple QPs.
> >> Some have ratio of 100 MRs per QP, as factor of memory size and operations.
> >> some servers like to have 1K MRs per QP.
> >> So if we have just weight, it will equally distributes MRs per QP in
> >> all cgroup and that either leads to unused resource per cgroup or,
> >> lesser number of cg instances.
> >> So fine tuning required for individual one, which we already have.
> >
> > I afraid that it is over complicating which can be done by curious user
> > in his user-space scripts: limit the global HCA -> read max values ->
> > overwrite with specific mapping.
> >
> >>
> >> weight or percentage helps in abstracting as starting point. So I like
> >> to add it too.
> >
> > Let's start simple
>
> Yes. I will rebase and test my patch today and see if requires resending.
It is worth to wait till -rc1. Doug didn't finish his pull requests yet.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH infiniband-diags v2] ibportstate: Fixed switch peer port probing when using DR routing
From: Knut Omang @ 2016-10-10 8:50 UTC (permalink / raw)
To: Ira Weiny
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Line Holen, Knut Omang,
Dag Moxnes
From: Dag Moxnes <dag.moxnes-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
ibportstate queries to a remote peer port on a switch using direct
routing would result in timeouts. The reason for this is that the
DR path was not correctly constructed.
Signed-off-by: Dag Moxnes <dag.moxnes-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Line Holen <line.holen-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
src/ibportstate.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/src/ibportstate.c b/src/ibportstate.c
index cb47aa9..7046f47 100644
--- a/src/ibportstate.c
+++ b/src/ibportstate.c
@@ -1,6 +1,8 @@
/*
* Copyright (c) 2004-2009 Voltaire Inc. All rights reserved.
* Copyright (c) 2010,2011 Mellanox Technologies LTD. All rights reserved.
+ * Copyright (c) 2011, 2016, Oracle and/or its affiliates. All rights reserved.
+ *
*
* This software is available to you under a choice of one of two
* licenses. You may choose to be licensed under the terms of the GNU
@@ -655,15 +657,22 @@ int main(int argc, char **argv)
/* Setup portid for peer port */
memcpy(&peerportid, &portid, sizeof(peerportid));
- peerportid.drpath.cnt = 1;
- peerportid.drpath.p[1] = (uint8_t) portnum;
-
- /* Set DrSLID to local lid */
- if (resolve_self(ibd_ca, ibd_ca_port, &selfportid,
- &selfport, 0) < 0)
- IBEXIT("could not resolve self");
- peerportid.drpath.drslid = (uint16_t) selfportid.lid;
- peerportid.drpath.drdlid = 0xffff;
+ if (portid.lid == 0) {
+ peerportid.drpath.cnt++;
+ if (peerportid.drpath.cnt == IB_SUBNET_PATH_HOPS_MAX) {
+ IBEXIT("Too many hops");
+ }
+ } else {
+ peerportid.drpath.cnt = 1;
+
+ /* Set DrSLID to local lid */
+ if (resolve_self(ibd_ca, ibd_ca_port, &selfportid, argv[0],
+ &selfportid, 0) < 0)
+ IBEXIT("could not resolve self");
+ peerportid.drpath.drslid = selfportid.lid;
+ peerportid.drpath.drdlid = 0xffff;
+ }
+ peerportid.drpath.p[peerportid.drpath.cnt] = (uint8_t) portnum;
/* Get peer port NodeInfo to obtain peer port number */
is_peer_switch = get_node_info(&peerportid, data);
base-commit: 2937cf99350a2e423b705e8b8dd10499796a7b41
--
git-series 0.8.10
--
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: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Parav Pandit @ 2016-10-10 8:35 UTC (permalink / raw)
To: Leon Romanovsky
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma, Tejun Heo, Li Zefan,
Johannes Weiner, Doug Ledford, Christoph Hellwig, Liran Liss,
Hefty, Sean, Jason Gunthorpe, Haggai Eran,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz, Matan Barak
In-Reply-To: <20161010073343.GK9282-2ukJVAZIZ/Y@public.gmane.org>
On Mon, Oct 10, 2016 at 1:03 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Oct 10, 2016 at 11:59:45AM +0530, Parav Pandit wrote:
>> Hi Leon,
>>
>> On Mon, Oct 10, 2016 at 10:16 AM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> > On Thu, Oct 06, 2016 at 07:19:24PM +0530, Parav Pandit wrote:
>> >> Hi Leon,
>> >>
>> >> On Wed, Oct 5, 2016 at 4:52 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> >> > On Wed, Aug 31, 2016 at 02:07:24PM +0530, Parav Pandit wrote:
>> >> >> rdmacg: IB/core: rdma controller support
>> >> >>
>> >> >> Overview:
>> >> >> Currently user space applications can easily take away all the rdma
>> >> >> device specific resources such as AH, CQ, QP, MR etc. Due to which other
>> >> >> applications in other cgroup or kernel space ULPs may not even get chance
>> >> >> to allocate any rdma resources. This results into service unavailibility.
>> >> >>
>> >> >> RDMA cgroup addresses this issue by allowing resource accounting,
>> >> >> limit enforcement on per cgroup, per rdma device basis.
>> >> >>
>> >> >> RDMA uverbs layer will enforce limits on well defined RDMA verb
>> >> >> resources without any HCA vendor device driver involvement.
>> >> >>
>> >> >> RDMA uverbs layer will not do limit enforcement of HCA hw vendor
>> >> >> specific resources. Instead rdma cgroup provides set of APIs
>> >> >> through which vendor specific drivers can do resource accounting
>> >> >> by making use of rdma cgroup.
>> >> >
>> >> > Hi Parav,
>> >> > I want to propose an extension to the RDMA cgroup which can be done as
>> >> > follow-up patches.
>> >> >
>> >> > Let's add new global type, which will control whole HCA (for example in percentages). It will
>> >> > allow natively define new objects without need to introduce them to the user.
>> >> >
>> >> In other cgroup such as CPU, this is done using cpu.weight API. Where
>> >> percentage or weight is configured by the user.
>> >> In this mode, resources taken away from other cgroup proportionately.
>> >> It works for cpu because its mainly stateless resource unlike rdma
>> >> resources.
>> >> So if we want to simplify user configuration similarly,
>> >> percentage/weight configuration can be extended.
>> >> This way they need not be introduced to users.
>> >> I hope your definition of "user" is actual end-user and not rdma cgroup. Right?
>> >
>> > Yes, "user" -> "admin".
>> > I think that percentage is more intuitive to them and will be much easier to
>> > explain how to use it. I always have in mind "swappiness" field and the
>> > numerous questions on how to configure it.
>> >
>> >> In other words, new object should be still added as new enum value in
>> >> rdma_cgroup.h?
>> >
>> > Yes, I had in mind something like IB_CGROUP_HCA, this is why it can be
>> > done as a future work after accepting current patches.
>> >
>> What I meant is,
>> today we have RDMACG_VERB_RESOURCE_QP etc,
>> We will additionally have RDMACG_VERB_RESOURCE_INDIRECT_TBL etc in
>> cgroup_rdma.h.
>> So that its available for admin to override it.
>
> IMHO, we are talking about the same. My global HCA object will be
> overwritten by more granular VERBS objects in case they exists.
>
>>
>> >> Only than it can be overwritten by specific UVERBs type as you
>> >> described below. I think thats what you meant as you described below.
>> >
>> > Exactly.
>> >
>> >>
>> >> Otherwise charging/uncharging this new percentage resource can get messy.
>> >
>> > Agree
>> >
>> >>
>> >> > This HCA share will be overwritten by specific UVERBS types which you
>> >> > already defined.
>> >> >
>> >> > What do you think?
>> >>
>> >> So to refine your proposal from cgroup perspective, instead of adding
>> >> new resource type in rdma_cgroup.h for percentage, I prefer to have
>> >>
>> >> Existing
>> >> 1. rdma.max
>> >> 2. rdma.current
>> >> New,
>> >> 3. rdma.weight
>> >> This ABI will have similar API to say
>> >> echo "mlx4_0 50" > rdma.weight.
>> >> Where 50 is weight of the resources.
>> >> For example,
>> >> for one cgroup instance weight=sum=100% resource for a given cgroup.
>> >> for three cgroup instances percentage=(weight/sum)% = 50/(50+50+50) = 33%.
>> >> One cgroup gets 33% resource.
>> >>
>> >> Weight can be in range of 1 to 10,000 similar to cpu cgroup.
>> >
>> > This is exactly what I don't like, the percentage will remove from the
>> > user the translation needs between weight and actual limitation.
>> >
>> > IMHO CPU used weights because everything there is in weights :).
>> >
>> I admit weight are not very intuitive, I was aligning to the existing
>> other cgroup interfaces which achieves similar functionality.
>> I will let Tejun approve the "percentage" or "ratio" new file
>> interface as its little different than weight.
>
> Sure, let's close the main idea first and see if it makes sense for
> other participants.
>
>>
>> >>
>> >> This might work if applications running in all cgroups are similar.
>> >> But weight doesn't do justice, when there are different type of
>> >> applications running in each cgroup. Such as few running libfabric
>> >> based apps, few running MPI, others directly using ibverbs.
>> >> So as you said rdma.max configuration would be required for management
>> >> plane to override weight (percentage) for certain resources.
>> >
>> > Why?
>> > The device exposes max values during initialization and if user asked
>> > for 20% percent of HCA, he will get max*0.2.
>>
>> Because every application may not be equivalent of other application.
>> For example, some require one to one QP and PD mapping.
>> Some share single PD across multiple QPs.
>> Some have ratio of 100 MRs per QP, as factor of memory size and operations.
>> some servers like to have 1K MRs per QP.
>> So if we have just weight, it will equally distributes MRs per QP in
>> all cgroup and that either leads to unused resource per cgroup or,
>> lesser number of cg instances.
>> So fine tuning required for individual one, which we already have.
>
> I afraid that it is over complicating which can be done by curious user
> in his user-space scripts: limit the global HCA -> read max values ->
> overwrite with specific mapping.
>
>>
>> weight or percentage helps in abstracting as starting point. So I like
>> to add it too.
>
> Let's start simple
Yes. I will rebase and test my patch today and see if requires resending.
--
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
* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Leon Romanovsky @ 2016-10-10 7:33 UTC (permalink / raw)
To: Parav Pandit
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma, Tejun Heo, Li Zefan,
Johannes Weiner, Doug Ledford, Christoph Hellwig, Liran Liss,
Hefty, Sean, Jason Gunthorpe, Haggai Eran,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz, Matan Barak
In-Reply-To: <CAG53R5UM6nSTZ7=0S9reKGX45CpNBi8soSDVZyXkN-z0_XXWWQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 6486 bytes --]
On Mon, Oct 10, 2016 at 11:59:45AM +0530, Parav Pandit wrote:
> Hi Leon,
>
> On Mon, Oct 10, 2016 at 10:16 AM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Thu, Oct 06, 2016 at 07:19:24PM +0530, Parav Pandit wrote:
> >> Hi Leon,
> >>
> >> On Wed, Oct 5, 2016 at 4:52 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >> > On Wed, Aug 31, 2016 at 02:07:24PM +0530, Parav Pandit wrote:
> >> >> rdmacg: IB/core: rdma controller support
> >> >>
> >> >> Overview:
> >> >> Currently user space applications can easily take away all the rdma
> >> >> device specific resources such as AH, CQ, QP, MR etc. Due to which other
> >> >> applications in other cgroup or kernel space ULPs may not even get chance
> >> >> to allocate any rdma resources. This results into service unavailibility.
> >> >>
> >> >> RDMA cgroup addresses this issue by allowing resource accounting,
> >> >> limit enforcement on per cgroup, per rdma device basis.
> >> >>
> >> >> RDMA uverbs layer will enforce limits on well defined RDMA verb
> >> >> resources without any HCA vendor device driver involvement.
> >> >>
> >> >> RDMA uverbs layer will not do limit enforcement of HCA hw vendor
> >> >> specific resources. Instead rdma cgroup provides set of APIs
> >> >> through which vendor specific drivers can do resource accounting
> >> >> by making use of rdma cgroup.
> >> >
> >> > Hi Parav,
> >> > I want to propose an extension to the RDMA cgroup which can be done as
> >> > follow-up patches.
> >> >
> >> > Let's add new global type, which will control whole HCA (for example in percentages). It will
> >> > allow natively define new objects without need to introduce them to the user.
> >> >
> >> In other cgroup such as CPU, this is done using cpu.weight API. Where
> >> percentage or weight is configured by the user.
> >> In this mode, resources taken away from other cgroup proportionately.
> >> It works for cpu because its mainly stateless resource unlike rdma
> >> resources.
> >> So if we want to simplify user configuration similarly,
> >> percentage/weight configuration can be extended.
> >> This way they need not be introduced to users.
> >> I hope your definition of "user" is actual end-user and not rdma cgroup. Right?
> >
> > Yes, "user" -> "admin".
> > I think that percentage is more intuitive to them and will be much easier to
> > explain how to use it. I always have in mind "swappiness" field and the
> > numerous questions on how to configure it.
> >
> >> In other words, new object should be still added as new enum value in
> >> rdma_cgroup.h?
> >
> > Yes, I had in mind something like IB_CGROUP_HCA, this is why it can be
> > done as a future work after accepting current patches.
> >
> What I meant is,
> today we have RDMACG_VERB_RESOURCE_QP etc,
> We will additionally have RDMACG_VERB_RESOURCE_INDIRECT_TBL etc in
> cgroup_rdma.h.
> So that its available for admin to override it.
IMHO, we are talking about the same. My global HCA object will be
overwritten by more granular VERBS objects in case they exists.
>
> >> Only than it can be overwritten by specific UVERBs type as you
> >> described below. I think thats what you meant as you described below.
> >
> > Exactly.
> >
> >>
> >> Otherwise charging/uncharging this new percentage resource can get messy.
> >
> > Agree
> >
> >>
> >> > This HCA share will be overwritten by specific UVERBS types which you
> >> > already defined.
> >> >
> >> > What do you think?
> >>
> >> So to refine your proposal from cgroup perspective, instead of adding
> >> new resource type in rdma_cgroup.h for percentage, I prefer to have
> >>
> >> Existing
> >> 1. rdma.max
> >> 2. rdma.current
> >> New,
> >> 3. rdma.weight
> >> This ABI will have similar API to say
> >> echo "mlx4_0 50" > rdma.weight.
> >> Where 50 is weight of the resources.
> >> For example,
> >> for one cgroup instance weight=sum=100% resource for a given cgroup.
> >> for three cgroup instances percentage=(weight/sum)% = 50/(50+50+50) = 33%.
> >> One cgroup gets 33% resource.
> >>
> >> Weight can be in range of 1 to 10,000 similar to cpu cgroup.
> >
> > This is exactly what I don't like, the percentage will remove from the
> > user the translation needs between weight and actual limitation.
> >
> > IMHO CPU used weights because everything there is in weights :).
> >
> I admit weight are not very intuitive, I was aligning to the existing
> other cgroup interfaces which achieves similar functionality.
> I will let Tejun approve the "percentage" or "ratio" new file
> interface as its little different than weight.
Sure, let's close the main idea first and see if it makes sense for
other participants.
>
> >>
> >> This might work if applications running in all cgroups are similar.
> >> But weight doesn't do justice, when there are different type of
> >> applications running in each cgroup. Such as few running libfabric
> >> based apps, few running MPI, others directly using ibverbs.
> >> So as you said rdma.max configuration would be required for management
> >> plane to override weight (percentage) for certain resources.
> >
> > Why?
> > The device exposes max values during initialization and if user asked
> > for 20% percent of HCA, he will get max*0.2.
>
> Because every application may not be equivalent of other application.
> For example, some require one to one QP and PD mapping.
> Some share single PD across multiple QPs.
> Some have ratio of 100 MRs per QP, as factor of memory size and operations.
> some servers like to have 1K MRs per QP.
> So if we have just weight, it will equally distributes MRs per QP in
> all cgroup and that either leads to unused resource per cgroup or,
> lesser number of cg instances.
> So fine tuning required for individual one, which we already have.
I afraid that it is over complicating which can be done by curious user
in his user-space scripts: limit the global HCA -> read max values ->
overwrite with specific mapping.
>
> weight or percentage helps in abstracting as starting point. So I like
> to add it too.
Let's start simple
Thanks.
>
>
>
> >
> >>
> >>
> >> >
> >> > Except this proposal,
> >> > Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >> >
> >> > Thanks.
> >> --
> >> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH qedr 04/10] qedr: Add support for PD,PKEY and CQ verbs
From: Leon Romanovsky @ 2016-10-10 7:24 UTC (permalink / raw)
To: Amrani, Ram
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, Elior, Ariel,
Kalderon, Michal, Mintz, Yuval, Borundia, Rajesh,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <SN1PR07MB22070099EE40A67B81D14C75F8DB0-mikhvbZlbf8TSoR2DauN2+FPX92sqiQdvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]
On Mon, Oct 10, 2016 at 06:34:43AM +0000, Amrani, Ram wrote:
> > > +/* * CQE requester status enumeration */ enum
> > > +rdma_cqe_requester_status_enum {
> > > + RDMA_CQE_REQ_STS_OK,
> > > + RDMA_CQE_REQ_STS_BAD_RESPONSE_ERR,
> > > + RDMA_CQE_REQ_STS_LOCAL_LENGTH_ERR,
> > > + RDMA_CQE_REQ_STS_LOCAL_QP_OPERATION_ERR,
> > > + RDMA_CQE_REQ_STS_LOCAL_PROTECTION_ERR,
> > > + RDMA_CQE_REQ_STS_MEMORY_MGT_OPERATION_ERR,
> > > + RDMA_CQE_REQ_STS_REMOTE_INVALID_REQUEST_ERR,
> > > + RDMA_CQE_REQ_STS_REMOTE_ACCESS_ERR,
> > > + RDMA_CQE_REQ_STS_REMOTE_OPERATION_ERR,
> > > + RDMA_CQE_REQ_STS_RNR_NAK_RETRY_CNT_ERR,
> > > + RDMA_CQE_REQ_STS_TRANSPORT_RETRY_CNT_ERR,
> > > + RDMA_CQE_REQ_STS_WORK_REQUEST_FLUSHED_ERR,
> > > + MAX_RDMA_CQE_REQUESTER_STATUS_ENUM
> >
> > Please add "," at the last line of enums, it will allow future changes to these
> > enums with minimal churn.
>
> This is a good idea and I've applied it to several enums in the patch.
> However, I think that this specific enum should left be as is since the last element should always remain last as it is a MAX_*
It makes sense,
Thanks
>
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [PATCH qedr 04/10] qedr: Add support for PD,PKEY and CQ verbs
From: Amrani, Ram @ 2016-10-10 6:34 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, Elior, Ariel,
Kalderon, Michal, Mintz, Yuval, Borundia, Rajesh,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20161006133357.GP9282-2ukJVAZIZ/Y@public.gmane.org>
> > +/* * CQE requester status enumeration */ enum
> > +rdma_cqe_requester_status_enum {
> > + RDMA_CQE_REQ_STS_OK,
> > + RDMA_CQE_REQ_STS_BAD_RESPONSE_ERR,
> > + RDMA_CQE_REQ_STS_LOCAL_LENGTH_ERR,
> > + RDMA_CQE_REQ_STS_LOCAL_QP_OPERATION_ERR,
> > + RDMA_CQE_REQ_STS_LOCAL_PROTECTION_ERR,
> > + RDMA_CQE_REQ_STS_MEMORY_MGT_OPERATION_ERR,
> > + RDMA_CQE_REQ_STS_REMOTE_INVALID_REQUEST_ERR,
> > + RDMA_CQE_REQ_STS_REMOTE_ACCESS_ERR,
> > + RDMA_CQE_REQ_STS_REMOTE_OPERATION_ERR,
> > + RDMA_CQE_REQ_STS_RNR_NAK_RETRY_CNT_ERR,
> > + RDMA_CQE_REQ_STS_TRANSPORT_RETRY_CNT_ERR,
> > + RDMA_CQE_REQ_STS_WORK_REQUEST_FLUSHED_ERR,
> > + MAX_RDMA_CQE_REQUESTER_STATUS_ENUM
>
> Please add "," at the last line of enums, it will allow future changes to these
> enums with minimal churn.
This is a good idea and I've applied it to several enums in the patch.
However, I think that this specific enum should left be as is since the last element should always remain last as it is a MAX_*
--
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
* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Parav Pandit @ 2016-10-10 6:29 UTC (permalink / raw)
To: Leon Romanovsky
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma, Tejun Heo, Li Zefan,
Johannes Weiner, Doug Ledford, Christoph Hellwig, Liran Liss,
Hefty, Sean, Jason Gunthorpe, Haggai Eran,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz, Matan Barak
In-Reply-To: <20161010044623.GI9282-2ukJVAZIZ/Y@public.gmane.org>
Hi Leon,
On Mon, Oct 10, 2016 at 10:16 AM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Oct 06, 2016 at 07:19:24PM +0530, Parav Pandit wrote:
>> Hi Leon,
>>
>> On Wed, Oct 5, 2016 at 4:52 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> > On Wed, Aug 31, 2016 at 02:07:24PM +0530, Parav Pandit wrote:
>> >> rdmacg: IB/core: rdma controller support
>> >>
>> >> Overview:
>> >> Currently user space applications can easily take away all the rdma
>> >> device specific resources such as AH, CQ, QP, MR etc. Due to which other
>> >> applications in other cgroup or kernel space ULPs may not even get chance
>> >> to allocate any rdma resources. This results into service unavailibility.
>> >>
>> >> RDMA cgroup addresses this issue by allowing resource accounting,
>> >> limit enforcement on per cgroup, per rdma device basis.
>> >>
>> >> RDMA uverbs layer will enforce limits on well defined RDMA verb
>> >> resources without any HCA vendor device driver involvement.
>> >>
>> >> RDMA uverbs layer will not do limit enforcement of HCA hw vendor
>> >> specific resources. Instead rdma cgroup provides set of APIs
>> >> through which vendor specific drivers can do resource accounting
>> >> by making use of rdma cgroup.
>> >
>> > Hi Parav,
>> > I want to propose an extension to the RDMA cgroup which can be done as
>> > follow-up patches.
>> >
>> > Let's add new global type, which will control whole HCA (for example in percentages). It will
>> > allow natively define new objects without need to introduce them to the user.
>> >
>> In other cgroup such as CPU, this is done using cpu.weight API. Where
>> percentage or weight is configured by the user.
>> In this mode, resources taken away from other cgroup proportionately.
>> It works for cpu because its mainly stateless resource unlike rdma
>> resources.
>> So if we want to simplify user configuration similarly,
>> percentage/weight configuration can be extended.
>> This way they need not be introduced to users.
>> I hope your definition of "user" is actual end-user and not rdma cgroup. Right?
>
> Yes, "user" -> "admin".
> I think that percentage is more intuitive to them and will be much easier to
> explain how to use it. I always have in mind "swappiness" field and the
> numerous questions on how to configure it.
>
>> In other words, new object should be still added as new enum value in
>> rdma_cgroup.h?
>
> Yes, I had in mind something like IB_CGROUP_HCA, this is why it can be
> done as a future work after accepting current patches.
>
What I meant is,
today we have RDMACG_VERB_RESOURCE_QP etc,
We will additionally have RDMACG_VERB_RESOURCE_INDIRECT_TBL etc in
cgroup_rdma.h.
So that its available for admin to override it.
>> Only than it can be overwritten by specific UVERBs type as you
>> described below. I think thats what you meant as you described below.
>
> Exactly.
>
>>
>> Otherwise charging/uncharging this new percentage resource can get messy.
>
> Agree
>
>>
>> > This HCA share will be overwritten by specific UVERBS types which you
>> > already defined.
>> >
>> > What do you think?
>>
>> So to refine your proposal from cgroup perspective, instead of adding
>> new resource type in rdma_cgroup.h for percentage, I prefer to have
>>
>> Existing
>> 1. rdma.max
>> 2. rdma.current
>> New,
>> 3. rdma.weight
>> This ABI will have similar API to say
>> echo "mlx4_0 50" > rdma.weight.
>> Where 50 is weight of the resources.
>> For example,
>> for one cgroup instance weight=sum=100% resource for a given cgroup.
>> for three cgroup instances percentage=(weight/sum)% = 50/(50+50+50) = 33%.
>> One cgroup gets 33% resource.
>>
>> Weight can be in range of 1 to 10,000 similar to cpu cgroup.
>
> This is exactly what I don't like, the percentage will remove from the
> user the translation needs between weight and actual limitation.
>
> IMHO CPU used weights because everything there is in weights :).
>
I admit weight are not very intuitive, I was aligning to the existing
other cgroup interfaces which achieves similar functionality.
I will let Tejun approve the "percentage" or "ratio" new file
interface as its little different than weight.
>>
>> This might work if applications running in all cgroups are similar.
>> But weight doesn't do justice, when there are different type of
>> applications running in each cgroup. Such as few running libfabric
>> based apps, few running MPI, others directly using ibverbs.
>> So as you said rdma.max configuration would be required for management
>> plane to override weight (percentage) for certain resources.
>
> Why?
> The device exposes max values during initialization and if user asked
> for 20% percent of HCA, he will get max*0.2.
Because every application may not be equivalent of other application.
For example, some require one to one QP and PD mapping.
Some share single PD across multiple QPs.
Some have ratio of 100 MRs per QP, as factor of memory size and operations.
some servers like to have 1K MRs per QP.
So if we have just weight, it will equally distributes MRs per QP in
all cgroup and that either leads to unused resource per cgroup or,
lesser number of cg instances.
So fine tuning required for individual one, which we already have.
weight or percentage helps in abstracting as starting point. So I like
to add it too.
>
>>
>>
>> >
>> > Except this proposal,
>> > Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> >
>> > Thanks.
>> --
>> 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
--
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
* Re: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Leon Romanovsky @ 2016-10-10 4:46 UTC (permalink / raw)
To: Parav Pandit
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
Tejun Heo, Li Zefan, Johannes Weiner, Doug Ledford,
Christoph Hellwig, Liran Liss, Hefty, Sean, Jason Gunthorpe,
Haggai Eran, james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz,
Matan Barak
In-Reply-To: <CAG53R5VNVb=8-LJbDRqjtOZG347ucPuc420bcfnDgBKMoKqU-w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4430 bytes --]
On Thu, Oct 06, 2016 at 07:19:24PM +0530, Parav Pandit wrote:
> Hi Leon,
>
> On Wed, Oct 5, 2016 at 4:52 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Wed, Aug 31, 2016 at 02:07:24PM +0530, Parav Pandit wrote:
> >> rdmacg: IB/core: rdma controller support
> >>
> >> Overview:
> >> Currently user space applications can easily take away all the rdma
> >> device specific resources such as AH, CQ, QP, MR etc. Due to which other
> >> applications in other cgroup or kernel space ULPs may not even get chance
> >> to allocate any rdma resources. This results into service unavailibility.
> >>
> >> RDMA cgroup addresses this issue by allowing resource accounting,
> >> limit enforcement on per cgroup, per rdma device basis.
> >>
> >> RDMA uverbs layer will enforce limits on well defined RDMA verb
> >> resources without any HCA vendor device driver involvement.
> >>
> >> RDMA uverbs layer will not do limit enforcement of HCA hw vendor
> >> specific resources. Instead rdma cgroup provides set of APIs
> >> through which vendor specific drivers can do resource accounting
> >> by making use of rdma cgroup.
> >
> > Hi Parav,
> > I want to propose an extension to the RDMA cgroup which can be done as
> > follow-up patches.
> >
> > Let's add new global type, which will control whole HCA (for example in percentages). It will
> > allow natively define new objects without need to introduce them to the user.
> >
> In other cgroup such as CPU, this is done using cpu.weight API. Where
> percentage or weight is configured by the user.
> In this mode, resources taken away from other cgroup proportionately.
> It works for cpu because its mainly stateless resource unlike rdma
> resources.
> So if we want to simplify user configuration similarly,
> percentage/weight configuration can be extended.
> This way they need not be introduced to users.
> I hope your definition of "user" is actual end-user and not rdma cgroup. Right?
Yes, "user" -> "admin".
I think that percentage is more intuitive to them and will be much easier to
explain how to use it. I always have in mind "swappiness" field and the
numerous questions on how to configure it.
> In other words, new object should be still added as new enum value in
> rdma_cgroup.h?
Yes, I had in mind something like IB_CGROUP_HCA, this is why it can be
done as a future work after accepting current patches.
> Only than it can be overwritten by specific UVERBs type as you
> described below. I think thats what you meant as you described below.
Exactly.
>
> Otherwise charging/uncharging this new percentage resource can get messy.
Agree
>
> > This HCA share will be overwritten by specific UVERBS types which you
> > already defined.
> >
> > What do you think?
>
> So to refine your proposal from cgroup perspective, instead of adding
> new resource type in rdma_cgroup.h for percentage, I prefer to have
>
> Existing
> 1. rdma.max
> 2. rdma.current
> New,
> 3. rdma.weight
> This ABI will have similar API to say
> echo "mlx4_0 50" > rdma.weight.
> Where 50 is weight of the resources.
> For example,
> for one cgroup instance weight=sum=100% resource for a given cgroup.
> for three cgroup instances percentage=(weight/sum)% = 50/(50+50+50) = 33%.
> One cgroup gets 33% resource.
>
> Weight can be in range of 1 to 10,000 similar to cpu cgroup.
This is exactly what I don't like, the percentage will remove from the
user the translation needs between weight and actual limitation.
IMHO CPU used weights because everything there is in weights :).
>
> This might work if applications running in all cgroups are similar.
> But weight doesn't do justice, when there are different type of
> applications running in each cgroup. Such as few running libfabric
> based apps, few running MPI, others directly using ibverbs.
> So as you said rdma.max configuration would be required for management
> plane to override weight (percentage) for certain resources.
Why?
The device exposes max values during initialization and if user asked
for 20% percent of HCA, he will get max*0.2.
>
>
> >
> > Except this proposal,
> > Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > Thanks.
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH V2] i40iw: Add Quality of Service support
From: Leon Romanovsky @ 2016-10-10 4:27 UTC (permalink / raw)
To: Henry Orosco
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
e1000-rdma-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <20161007204747.6420-1-henry.orosco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 785 bytes --]
On Fri, Oct 07, 2016 at 03:47:47PM -0500, Henry Orosco wrote:
<...>
>
> switch (iwdev->init_state) {
> case RDMA_DEV_REGISTERED:
> @@ -1628,6 +1642,7 @@ static int i40iw_open(struct i40e_info *ldev, struct i40e_client *client)
> iwdev->init_state = RDMA_DEV_REGISTERED;
> iwdev->iw_status = 1;
> i40iw_port_ibevent(iwdev);
> + iwdev->param_wq = create_singlethread_workqueue("l2params");
> i40iw_pr_info("i40iw_open completed\n");
No, please don't use this interface, For reference, see latest work from Bhaktipriya
Shridhar to eliminate this call. Doug and Dave sent a pull request with it to Linus in
last merge cycle.
Also, 1) you should check if this call succeeded and 2) ask yourself if
special workqueue is needed.
> return 0;
> } while (0);
Thanks.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH rdma-core 1/5] Pull uninitialized_var into util/compiler.h
From: Leon Romanovsky @ 2016-10-10 4:09 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161009230150.GB12551-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 957 bytes --]
On Sun, Oct 09, 2016 at 05:01:51PM -0600, Jason Gunthorpe wrote:
> On Sun, Oct 09, 2016 at 08:46:01AM +0300, Leon Romanovsky wrote:
>
> > 1. Can we remove this uninitialized_var at all and simply replace by
> > default values?
>
> I think people would complain about the extra stores. gcc 6 will
> eliminate them, but older compilers will not.
This unintialized_var(x) adds extra store too (... x = x ...).
>
> But yes, we could do this.
>
> > 2. Do we really want additional folder to ccan with such common headers?
> > What about rename ccan to be util and put this new file there (in case
> > you proceed with it)?
>
> I see the ccan directory as stuf from ccan that we are not going to
> change, while util is for new stuff specific to this tree.
And I see this folder as a location of shared code with different utils
(ccan, custom, e.t.c), but it doesn't really matter now and I will be fine
with any directory structure layout.
Thanks
>
> Jason
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 0/5 for rdma-core] Five rxe_cfg patches
From: Leon Romanovsky @ 2016-10-10 4:02 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Doug Ledford, Jason Gunthorpe, Moni Shoua, linux-rdma
In-Reply-To: <bf5dba39-7e14-23ee-4aa1-2a276d629fe9-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 994 bytes --]
On Fri, Oct 07, 2016 at 11:37:43AM -0700, Bart Van Assche wrote:
> Hello Doug, Jason and Moni,
>
> These five patches is what I came up with after having had a look at the
> rxe_cfg source code. It would be appreciated if these patches would be
> considered for inclusion in the rdma-core repository. The filenames of these
> patches are:
>
> 0001-rxe_cfg-Use-Perl-functions-instead-of-parsing-the-ou.patch
> 0002-rxe_cfg-Initialize-rxe_mtu-even-if-ibv_definfo-fails.patch
> 0003-rxe_cfg-Do-not-suppress-stderr.patch
> 0004-rxe_cfg-Remove-dead-code-from-show_module_status.patch
> 0005-rxe_cfg-Use-printf-instead-of-using-a-loop.patch
Hi Bart,
Thank you for investing time, Moni started to look/test them and update
us on the result.
Thanks.
>
> Thanks,
>
> Bart.
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PULL REQUEST] Please pull rdma.git (tag for-linus)
From: Doug Ledford @ 2016-10-10 3:07 UTC (permalink / raw)
To: Linus Torvalds, Steve Wise; +Cc: linux-rdma
In-Reply-To: <CA+55aFw5tftp1Da5Hs8ks+=hiodZ_NTCH3Z9efPxDVo7h75RAw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 847 bytes --]
On 10/9/2016 8:14 PM, Linus Torvalds wrote:
> On Fri, Oct 7, 2016 at 3:24 PM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>
>> 1) cxgb4 - keep the head version in your tree, my tree and the common
>> ancestor were prior to a refactoring in your tree that moved uld_attach
>> from cxgb4_main.c to cxgb4_uld.c
>
> Hmm. As far as I can tell, a proper merge also involves adding the line
>
> lld->fr_nsmr_tpte_wr_support = adap->params.fr_nsmr_tpte_wr_support;
>
> to uld_init() in cxgb4_uld.c.
>
> I did so, and it all seems to build find for me, but I obviously
> cannot do any testing. Somebody with the proper cxgb4 hardware should
> verify.
Thanks for that catch, we'll get it tested.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [PULL REQUEST] Please pull rdma.git (tag for-linus)
From: Linus Torvalds @ 2016-10-10 0:14 UTC (permalink / raw)
To: Doug Ledford, Steve Wise; +Cc: linux-rdma
In-Reply-To: <00af6acf-ec90-ea0e-68d6-f9e962c89bd3-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Fri, Oct 7, 2016 at 3:24 PM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> 1) cxgb4 - keep the head version in your tree, my tree and the common
> ancestor were prior to a refactoring in your tree that moved uld_attach
> from cxgb4_main.c to cxgb4_uld.c
Hmm. As far as I can tell, a proper merge also involves adding the line
lld->fr_nsmr_tpte_wr_support = adap->params.fr_nsmr_tpte_wr_support;
to uld_init() in cxgb4_uld.c.
I did so, and it all seems to build find for me, but I obviously
cannot do any testing. Somebody with the proper cxgb4 hardware should
verify.
Of course, it only affects the very special IB_WR_REG_MR fastpath
case, so maybe nobody much cares. Adding Steve Wise to the cc, to make
sure somebody actually tests this.
(Side note: current git has what appears to be a use-after-free
netfilter buglet, but it shouldn't trigger problems often enough to
keep people from testing. It kept me from doing my merge window pulls
while I investigated, though).
Linus
--
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
* Re: [PATCH 26/28] Avoid gcc 5.4 warning -Wunused-result
From: Jason Gunthorpe @ 2016-10-09 23:19 UTC (permalink / raw)
To: Yishai Hadas
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Devesh Sharma,
Hal Rosenstock, Mike Marciniszyn, Moni Shoua, Sean Hefty,
Steve Wise, Tatyana Nikolova, Vladimir Sokolovsky, Yishai Hadas
In-Reply-To: <9ae84345-4427-4689-e6d1-0bfa3eb19630-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
On Sun, Oct 09, 2016 at 04:31:17PM +0300, Yishai Hadas wrote:
> >- if (write(connfd, msg, sizeof msg) != sizeof msg) {
> >- fprintf(stderr, "Couldn't send local address\n");
> >+ if (write(connfd, msg, sizeof msg) != sizeof msg ||
> >+ read(connfd, msg, sizeof msg) != sizeof msg) {
> >+ fprintf(stderr, "Couldn't send/recv local address\n");
>
> At that step the server expects to read a "done" response from its client,
> checking whether the read was done for sizeof msg which is much larger will
> fail.
This OK?
>From db525af53140c3b7604ab45406ed8845cb6171e1 Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Date: Sun, 9 Oct 2016 17:17:54 -0600
Subject: [PATCH] verbs: Fix read error check in pingpong
Turns out these reads rely on the short readed caused by EOS.
Fixes: f3912df771db (Avoid gcc 5.4 warning -Wunused-result)
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
libibverbs/examples/rc_pingpong.c | 2 +-
libibverbs/examples/srq_pingpong.c | 2 +-
libibverbs/examples/uc_pingpong.c | 2 +-
libibverbs/examples/ud_pingpong.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c
index c92e551c00e6..aca7bac4491b 100644
--- a/libibverbs/examples/rc_pingpong.c
+++ b/libibverbs/examples/rc_pingpong.c
@@ -309,7 +309,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
my_dest->psn, gid);
if (write(connfd, msg, sizeof msg) != sizeof msg ||
- read(connfd, msg, sizeof msg) != sizeof msg) {
+ read(connfd, msg, sizeof msg) != sizeof "done") {
fprintf(stderr, "Couldn't send/recv local address\n");
free(rem_dest);
rem_dest = NULL;
diff --git a/libibverbs/examples/srq_pingpong.c b/libibverbs/examples/srq_pingpong.c
index 863ff10dd0f4..8f55d78e87d4 100644
--- a/libibverbs/examples/srq_pingpong.c
+++ b/libibverbs/examples/srq_pingpong.c
@@ -333,7 +333,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
}
}
- if (read(connfd, msg, sizeof msg) != sizeof msg) {
+ if (read(connfd, msg, sizeof msg) != sizeof "done") {
perror("client write");
free(rem_dest);
rem_dest = NULL;
diff --git a/libibverbs/examples/uc_pingpong.c b/libibverbs/examples/uc_pingpong.c
index 2b105b947cf3..b565bacaff2a 100644
--- a/libibverbs/examples/uc_pingpong.c
+++ b/libibverbs/examples/uc_pingpong.c
@@ -283,7 +283,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
my_dest->psn, gid);
if (write(connfd, msg, sizeof msg) != sizeof msg ||
- read(connfd, msg, sizeof msg) != sizeof msg) {
+ read(connfd, msg, sizeof msg) != sizeof "done") {
fprintf(stderr, "Couldn't send/recv local address\n");
free(rem_dest);
rem_dest = NULL;
diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c
index d0cd73cc0fae..ddb68cf8624b 100644
--- a/libibverbs/examples/ud_pingpong.c
+++ b/libibverbs/examples/ud_pingpong.c
@@ -280,7 +280,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx,
sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn,
my_dest->psn, gid);
if (write(connfd, msg, sizeof msg) != sizeof msg ||
- read(connfd, msg, sizeof msg) != sizeof msg) {
+ read(connfd, msg, sizeof msg) != sizeof "done") {
fprintf(stderr, "Couldn't send/recv local address\n");
free(rem_dest);
rem_dest = NULL;
--
2.7.4
--
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
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