[PATCH 0/5] More SRP target patches for kernel v4.15

[PATCH 0/5] More SRP target patches for kernel v4.15

[Bart Van Assche]

Hello Doug,

Recently Mike Marciniszyn discovered that ib_srpt non-SRQ mode does not work
on Intel adapters and also that switching dynamically between non-SRQ and SRQ
mode does not work. This patch series addresses these two issues. Please
consider this patch series for kernel v4.15.

Thanks,

Bart.

Bart Van Assche (4):
  IB/srpt: Introduce helper functions for SRQ allocation and freeing
  IB/srpt: Introduce srpt_disconnect_ch_sync()
  IB/srpt: Wait until channel release has finished during module unload
  IB/srpt: Ensure that modifying the use_srq configfs attribute works

Mike Marciniszyn (1):
  IB/srpt: Post receive work requests after qp transition to INIT state

 drivers/infiniband/ulp/srpt/ib_srpt.c | 229 +++++++++++++++++++++-------------
 1 file changed, 143 insertions(+), 86 deletions(-)

-- 
2.14.3

--
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

[PATCH 1/5] IB/srpt: Post receive work requests after qp transition to INIT state

[Bart Van Assche]

From: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Not all HCAs support posting receive work requests in the RESET
state. Postpone posting receive work requests until after the
transition to the INIT state.

Fixes: commit dea262094cdf ("IB/srpt: Change default behavior from using SRQ to using RC")
Signed-off-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 304855b9b537..98b1b80e476b 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1687,10 +1687,6 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
 		goto err_destroy_cq;
 	}
 
-	if (!sdev->use_srq)
-		for (i = 0; i < ch->rq_size; i++)
-			srpt_post_recv(sdev, ch, ch->ioctx_recv_ring[i]);
-
 	atomic_set(&ch->sq_wr_avail, qp_init->cap.max_send_wr);
 
 	pr_debug("%s: max_cqe= %d max_sge= %d sq_size = %d cm_id= %p\n",
@@ -1701,6 +1697,10 @@ static int srpt_create_ch_ib(struct srpt_rdma_ch *ch)
 	if (ret)
 		goto err_destroy_qp;
 
+	if (!sdev->use_srq)
+		for (i = 0; i < ch->rq_size; i++)
+			srpt_post_recv(sdev, ch, ch->ioctx_recv_ring[i]);
+
 out:
 	kfree(qp_init);
 	return ret;
-- 
2.14.3

--
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

[PATCH 2/5] IB/srpt: Introduce helper functions for SRQ allocation and freeing

[Bart Van Assche]

The only functional change in this patch is in the srpt_add_one()
error path: if allocating the ring buffer for the SRQ fails, fall
back to non-SRQ mode instead of disabling SRP target functionality.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 115 +++++++++++++++++++++-------------
 1 file changed, 72 insertions(+), 43 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 98b1b80e476b..a5f232e9b1f3 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -2510,6 +2510,74 @@ static struct se_wwn *srpt_lookup_wwn(const char *name)
 	return wwn;
 }
 
+static void srpt_free_srq(struct srpt_device *sdev)
+{
+	if (!sdev->srq)
+		return;
+
+	ib_destroy_srq(sdev->srq);
+	srpt_free_ioctx_ring((struct srpt_ioctx **)sdev->ioctx_ring, sdev,
+			     sdev->srq_size, srp_max_req_size, DMA_FROM_DEVICE);
+	sdev->srq = NULL;
+}
+
+static int srpt_alloc_srq(struct srpt_device *sdev)
+{
+	struct ib_srq_init_attr srq_attr = {
+		.event_handler = srpt_srq_event,
+		.srq_context = (void *)sdev,
+		.attr.max_wr = sdev->srq_size,
+		.attr.max_sge = 1,
+		.srq_type = IB_SRQT_BASIC,
+	};
+	struct ib_device *device = sdev->device;
+	struct ib_srq *srq;
+	int i;
+
+	WARN_ON_ONCE(sdev->srq);
+	srq = ib_create_srq(sdev->pd, &srq_attr);
+	if (IS_ERR(srq)) {
+		pr_debug("ib_create_srq() failed: %ld\n", PTR_ERR(srq));
+		return PTR_ERR(srq);
+	}
+
+	pr_debug("create SRQ #wr= %d max_allow=%d dev= %s\n", sdev->srq_size,
+		 sdev->device->attrs.max_srq_wr, device->name);
+
+	sdev->ioctx_ring = (struct srpt_recv_ioctx **)
+		srpt_alloc_ioctx_ring(sdev, sdev->srq_size,
+				      sizeof(*sdev->ioctx_ring[0]),
+				      srp_max_req_size, DMA_FROM_DEVICE);
+	if (!sdev->ioctx_ring) {
+		ib_destroy_srq(srq);
+		return -ENOMEM;
+	}
+
+	sdev->use_srq = true;
+	sdev->srq = srq;
+
+	for (i = 0; i < sdev->srq_size; ++i)
+		srpt_post_recv(sdev, NULL, sdev->ioctx_ring[i]);
+
+	return 0;
+}
+
+static int srpt_use_srq(struct srpt_device *sdev, bool use_srq)
+{
+	struct ib_device *device = sdev->device;
+	int ret = 0;
+
+	if (!use_srq) {
+		srpt_free_srq(sdev);
+		sdev->use_srq = false;
+	} else if (use_srq && !sdev->srq) {
+		ret = srpt_alloc_srq(sdev);
+	}
+	pr_debug("%s(%s): use_srq = %d; ret = %d\n", __func__, device->name,
+		 sdev->use_srq, ret);
+	return ret;
+}
+
 /**
  * srpt_add_one() - Infiniband device addition callback function.
  */
@@ -2517,7 +2585,6 @@ static void srpt_add_one(struct ib_device *device)
 {
 	struct srpt_device *sdev;
 	struct srpt_port *sport;
-	struct ib_srq_init_attr srq_attr;
 	int i;
 
 	pr_debug("device = %p\n", device);
@@ -2539,38 +2606,7 @@ static void srpt_add_one(struct ib_device *device)
 
 	sdev->srq_size = min(srpt_srq_size, sdev->device->attrs.max_srq_wr);
 
-	srq_attr.event_handler = srpt_srq_event;
-	srq_attr.srq_context = (void *)sdev;
-	srq_attr.attr.max_wr = sdev->srq_size;
-	srq_attr.attr.max_sge = 1;
-	srq_attr.attr.srq_limit = 0;
-	srq_attr.srq_type = IB_SRQT_BASIC;
-
-	sdev->srq = sdev->port[0].port_attrib.use_srq ?
-		ib_create_srq(sdev->pd, &srq_attr) : ERR_PTR(-ENOTSUPP);
-	if (IS_ERR(sdev->srq)) {
-		pr_debug("ib_create_srq() failed: %ld\n", PTR_ERR(sdev->srq));
-
-		/* SRQ not supported. */
-		sdev->use_srq = false;
-	} else {
-		pr_debug("create SRQ #wr= %d max_allow=%d dev= %s\n",
-			 sdev->srq_size, sdev->device->attrs.max_srq_wr,
-			 device->name);
-
-		sdev->use_srq = true;
-
-		sdev->ioctx_ring = (struct srpt_recv_ioctx **)
-			srpt_alloc_ioctx_ring(sdev, sdev->srq_size,
-					      sizeof(*sdev->ioctx_ring[0]),
-					      srp_max_req_size,
-					      DMA_FROM_DEVICE);
-		if (!sdev->ioctx_ring)
-			goto err_pd;
-
-		for (i = 0; i < sdev->srq_size; ++i)
-			srpt_post_recv(sdev, NULL, sdev->ioctx_ring[i]);
-	}
+	srpt_use_srq(sdev, sdev->port[0].port_attrib.use_srq);
 
 	if (!srpt_service_guid)
 		srpt_service_guid = be64_to_cpu(device->node_guid);
@@ -2630,12 +2666,7 @@ static void srpt_add_one(struct ib_device *device)
 err_cm:
 	ib_destroy_cm_id(sdev->cm_id);
 err_ring:
-	if (sdev->use_srq)
-		ib_destroy_srq(sdev->srq);
-	srpt_free_ioctx_ring((struct srpt_ioctx **)sdev->ioctx_ring, sdev,
-			     sdev->srq_size, srp_max_req_size,
-			     DMA_FROM_DEVICE);
-err_pd:
+	srpt_free_srq(sdev);
 	ib_dealloc_pd(sdev->pd);
 free_dev:
 	kfree(sdev);
@@ -2678,10 +2709,8 @@ static void srpt_remove_one(struct ib_device *device, void *client_data)
 	spin_unlock(&srpt_dev_lock);
 	srpt_release_sdev(sdev);
 
-	if (sdev->use_srq)
-		ib_destroy_srq(sdev->srq);
-	srpt_free_ioctx_ring((struct srpt_ioctx **)sdev->ioctx_ring, sdev,
-			     sdev->srq_size, srp_max_req_size, DMA_FROM_DEVICE);
+	srpt_free_srq(sdev);
+
 	ib_dealloc_pd(sdev->pd);
 
 	kfree(sdev);
-- 
2.14.3

--
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

[PATCH 3/5] IB/srpt: Introduce srpt_disconnect_ch_sync()

[Bart Van Assche]

Except for changing a BUG_ON() call into a WARN_ON_ONCE() call, this
patch does not change any functionality.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 51 ++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index a5f232e9b1f3..1aeb8d6ae6d9 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1787,6 +1787,40 @@ static int srpt_disconnect_ch(struct srpt_rdma_ch *ch)
 	return ret;
 }
 
+/*
+ * Send DREQ and wait for DREP. Return true if and only if this function
+ * changed the state of @ch.
+ */
+static bool srpt_disconnect_ch_sync(struct srpt_rdma_ch *ch)
+	__must_hold(&sdev->mutex)
+{
+	DECLARE_COMPLETION_ONSTACK(release_done);
+	struct srpt_device *sdev = ch->sport->sdev;
+	bool wait;
+
+	lockdep_assert_held(&sdev->mutex);
+
+	pr_debug("ch %s-%d state %d\n", ch->sess_name, ch->qp->qp_num,
+		 ch->state);
+
+	WARN_ON(ch->release_done);
+	ch->release_done = &release_done;
+	wait = !list_empty(&ch->list);
+	srpt_disconnect_ch(ch);
+	mutex_unlock(&sdev->mutex);
+
+	if (!wait)
+		goto out;
+
+	while (wait_for_completion_timeout(&release_done, 180 * HZ) == 0)
+		pr_info("%s(%s-%d state %d): still waiting ...\n", __func__,
+			ch->sess_name, ch->qp->qp_num, ch->state);
+
+out:
+	mutex_lock(&sdev->mutex);
+	return wait;
+}
+
 static void __srpt_close_all_ch(struct srpt_device *sdev)
 {
 	struct srpt_rdma_ch *ch;
@@ -2791,27 +2825,12 @@ static void srpt_release_cmd(struct se_cmd *se_cmd)
  */
 static void srpt_close_session(struct se_session *se_sess)
 {
-	DECLARE_COMPLETION_ONSTACK(release_done);
 	struct srpt_rdma_ch *ch = se_sess->fabric_sess_ptr;
 	struct srpt_device *sdev = ch->sport->sdev;
-	bool wait;
-
-	pr_debug("ch %s-%d state %d\n", ch->sess_name, ch->qp->qp_num,
-		 ch->state);
 
 	mutex_lock(&sdev->mutex);
-	BUG_ON(ch->release_done);
-	ch->release_done = &release_done;
-	wait = !list_empty(&ch->list);
-	srpt_disconnect_ch(ch);
+	srpt_disconnect_ch_sync(ch);
 	mutex_unlock(&sdev->mutex);
-
-	if (!wait)
-		return;
-
-	while (wait_for_completion_timeout(&release_done, 180 * HZ) == 0)
-		pr_info("%s(%s-%d state %d): still waiting ...\n", __func__,
-			ch->sess_name, ch->qp->qp_num, ch->state);
 }
 
 /**
-- 
2.14.3

--
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

[PATCH 4/5] IB/srpt: Wait until channel release has finished during module unload

[Bart Van Assche]

Introduce the helper function srpt_set_enabled(). Protect
sport->enabled changes with sdev->mutex. Makes configfs writes
into 'enabled' wait until all channel resources have been freed.
Wait until channel release has finished during kernel module
unload.

Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 43 ++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 1aeb8d6ae6d9..ca602eb50078 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1821,19 +1821,31 @@ static bool srpt_disconnect_ch_sync(struct srpt_rdma_ch *ch)
 	return wait;
 }
 
-static void __srpt_close_all_ch(struct srpt_device *sdev)
+static void srpt_set_enabled(struct srpt_port *sport, bool enabled)
+	__must_hold(&sdev->mutex)
 {
+	struct srpt_device *sdev = sport->sdev;
 	struct srpt_rdma_ch *ch;
 
 	lockdep_assert_held(&sdev->mutex);
 
+	if (sport->enabled == enabled)
+		return;
+	sport->enabled = enabled;
+	if (sport->enabled)
+		return;
+
+again:
 	list_for_each_entry(ch, &sdev->rch_list, list) {
-		if (srpt_disconnect_ch(ch) >= 0)
-			pr_info("Closing channel %s-%d because target %s has been disabled\n",
-				ch->sess_name, ch->qp->qp_num,
-				sdev->device->name);
-		srpt_close_ch(ch);
+		if (ch->sport == sport) {
+			pr_info("%s: closing channel %s-%d\n",
+				sdev->device->name, ch->sess_name,
+				ch->qp->qp_num);
+			if (srpt_disconnect_ch_sync(ch))
+				goto again;
+		}
 	}
+
 }
 
 static void srpt_free_ch(struct kref *kref)
@@ -2496,8 +2508,7 @@ static int srpt_release_sdev(struct srpt_device *sdev)
 
 	mutex_lock(&sdev->mutex);
 	for (i = 0; i < ARRAY_SIZE(sdev->port); i++)
-		sdev->port[i].enabled = false;
-	__srpt_close_all_ch(sdev);
+		srpt_set_enabled(&sdev->port[i], false);
 	mutex_unlock(&sdev->mutex);
 
 	res = wait_event_interruptible(sdev->ch_releaseQ,
@@ -3083,7 +3094,6 @@ static ssize_t srpt_tpg_enable_store(struct config_item *item,
 	struct se_portal_group *se_tpg = to_tpg(item);
 	struct srpt_port *sport = srpt_tpg_to_sport(se_tpg);
 	struct srpt_device *sdev = sport->sdev;
-	struct srpt_rdma_ch *ch;
 	unsigned long tmp;
         int ret;
 
@@ -3097,24 +3107,11 @@ static ssize_t srpt_tpg_enable_store(struct config_item *item,
 		pr_err("Illegal value for srpt_tpg_store_enable: %lu\n", tmp);
 		return -EINVAL;
 	}
-	if (sport->enabled == tmp)
-		goto out;
-	sport->enabled = tmp;
-	if (sport->enabled)
-		goto out;
 
 	mutex_lock(&sdev->mutex);
-	list_for_each_entry(ch, &sdev->rch_list, list) {
-		if (ch->sport == sport) {
-			pr_debug("%s: ch %p %s-%d\n", __func__, ch,
-				 ch->sess_name, ch->qp->qp_num);
-			srpt_disconnect_ch(ch);
-			srpt_close_ch(ch);
-		}
-	}
+	srpt_set_enabled(sport, tmp);
 	mutex_unlock(&sdev->mutex);
 
-out:
 	return count;
 }
 
-- 
2.14.3

--
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

[PATCH 5/5] IB/srpt: Ensure that modifying the use_srq configfs attribute works

[Bart Van Assche]

The use_srq configfs attribute is created after it is read. Hence
modify srpt_tpg_attrib_use_srq_store() such that this function
switches dynamically between non-SRQ and SRQ mode.

Reported-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Bart Van Assche <bart.vanassche-Sjgp3cTcYWE@public.gmane.org>
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index ca602eb50078..fde4f6741316 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -3054,7 +3054,9 @@ static ssize_t srpt_tpg_attrib_use_srq_store(struct config_item *item,
 {
 	struct se_portal_group *se_tpg = attrib_to_tpg(item);
 	struct srpt_port *sport = srpt_tpg_to_sport(se_tpg);
+	struct srpt_device *sdev = sport->sdev;
 	unsigned long val;
+	bool enabled;
 	int ret;
 
 	ret = kstrtoul(page, 0, &val);
@@ -3062,7 +3064,17 @@ static ssize_t srpt_tpg_attrib_use_srq_store(struct config_item *item,
 		return ret;
 	if (val != !!val)
 		return -EINVAL;
+
+	ret = mutex_lock_interruptible(&sdev->mutex);
+	if (ret < 0)
+		return ret;
+	enabled = sport->enabled;
+	/* Log out all initiator systems before changing 'use_srq'. */
+	srpt_set_enabled(sport, false);
 	sport->port_attrib.use_srq = val;
+	srpt_use_srq(sdev, sport->port_attrib.use_srq);
+	srpt_set_enabled(sport, enabled);
+	mutex_unlock(&sdev->mutex);
 
 	return count;
 }
-- 
2.14.3

--
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

Re: [PATCH 1/5] IB/srpt: Post receive work requests after qp transition to INIT state

[Jason Gunthorpe]

On Fri, Nov 03, 2017 at 04:20:52PM -0700, Bart Van Assche wrote:
> From: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> Not all HCAs support posting receive work requests in the RESET
> state. Postpone posting receive work requests until after the
> transition to the INIT state.

IBA spec even says submitting a WR to a RESET QP is an error.

Jason
--
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

RE: [PATCH 1/5] IB/srpt: Post receive work requests after qp transition to INIT state

[Marciniszyn, Mike]

> > Not all HCAs support posting receive work requests in the RESET
> > state. Postpone posting receive work requests until after the
> > transition to the INIT state.
> 
> IBA spec even says submitting a WR to a RESET QP is an error.
> 

C10-97.2.1: If the QP is not associated with an SRQ, the QP shall process Work Requests submitted to the Receive Queue as described in the
rules that follow:
* Return an immediate error if the QP is in the Reset state.

So are other implementations not doing this?

Mike
--
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

Re: [PATCH 0/5] More SRP target patches for kernel v4.15

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]

On Fri, 2017-11-03 at 16:20 -0700, Bart Van Assche wrote:
> Hello Doug,
> 
> Recently Mike Marciniszyn discovered that ib_srpt non-SRQ mode does not work
> on Intel adapters and also that switching dynamically between non-SRQ and SRQ
> mode does not work. This patch series addresses these two issues. Please
> consider this patch series for kernel v4.15.
> 
> Thanks,
> 
> Bart.
> 
> Bart Van Assche (4):
>   IB/srpt: Introduce helper functions for SRQ allocation and freeing
>   IB/srpt: Introduce srpt_disconnect_ch_sync()
>   IB/srpt: Wait until channel release has finished during module unload
>   IB/srpt: Ensure that modifying the use_srq configfs attribute works
> 
> Mike Marciniszyn (1):
>   IB/srpt: Post receive work requests after qp transition to INIT state
> 
>  drivers/infiniband/ulp/srpt/ib_srpt.c | 229 +++++++++++++++++++++-------------
>  1 file changed, 143 insertions(+), 86 deletions(-)

Thanks Bart.  I reworded git message to the first patch, and then
applied the set.  Thanks.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]