All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
To: stefan.wahren@i2se.com, eric@anholt.net, dave.stevenson@raspberrypi.org
Cc: nsaenzjulienne@suse.de, linux-rpi-kernel@lists.infradead.org,
	gregkh@linuxfoundation.org, linux-arm-kernel@lists.infradead.org,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: [PATCH RFC 06/18] staging: vchiq_arm: rework vchiq_ioc_copy_element_data
Date: Fri, 26 Oct 2018 15:48:01 +0200	[thread overview]
Message-ID: <20181026134813.7775-7-nsaenzjulienne@suse.de> (raw)
In-Reply-To: <20181026134813.7775-1-nsaenzjulienne@suse.de>

The function is passed to vchiq_core.c for it to go trough all the
transfer elements (an array of pointers to data) and copy them into the
actual transfer memory (contiguous memory).

The logic in the function was "copy an element and return, except when
the element is empty, in which case look for the next non-empty element
and copy it. The function will be called as many times as necessary until
all the elements are copied".

Now, this approach already forces the function to loop around elements
and felt convoluted, so it was changed to a more straightforward "Copy
all the elements into memory as long as they fit".

The resulting function is shorter and simpler.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 89 +++++++------------
 1 file changed, 31 insertions(+), 58 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 34160cc3b8bd..1cdfdb714abc 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -752,74 +752,48 @@ static void close_delivered(USER_SERVICE_T *user_service)
 }
 
 struct vchiq_io_copy_callback_context {
-	struct vchiq_element *current_element;
-	size_t current_element_offset;
+	struct vchiq_element *element;
+	size_t element_offset;
 	unsigned long elements_to_go;
-	size_t current_offset;
 };
 
-static ssize_t
-vchiq_ioc_copy_element_data(
-	void *context,
-	void *dest,
-	size_t offset,
-	size_t maxsize)
+static ssize_t vchiq_ioc_copy_element_data(void *context, void *dest,
+					   size_t offset, size_t maxsize)
 {
-	long res;
+	struct vchiq_io_copy_callback_context *cc = context;
+	size_t total_bytes_copied = 0;
 	size_t bytes_this_round;
-	struct vchiq_io_copy_callback_context *copy_context =
-		(struct vchiq_io_copy_callback_context *)context;
-
-	if (offset != copy_context->current_offset)
-		return 0;
-
-	if (!copy_context->elements_to_go)
-		return 0;
-
-	/*
-	 * Complex logic here to handle the case of 0 size elements
-	 * in the middle of the array of elements.
-	 *
-	 * Need to skip over these 0 size elements.
-	 */
-	while (1) {
-		bytes_this_round = min(copy_context->current_element->size -
-				       copy_context->current_element_offset,
-				       maxsize);
-
-		if (bytes_this_round)
-			break;
 
-		copy_context->elements_to_go--;
-		copy_context->current_element++;
-		copy_context->current_element_offset = 0;
+	while (total_bytes_copied < maxsize) {
+		if (!cc->elements_to_go)
+			return total_bytes_copied;
 
-		if (!copy_context->elements_to_go)
-			return 0;
-	}
+		if (!cc->element->size) {
+			cc->elements_to_go--;
+			cc->element++;
+			cc->element_offset = 0;
+			continue;
+		}
 
-	res = copy_from_user(dest,
-			     copy_context->current_element->data +
-			     copy_context->current_element_offset,
-			     bytes_this_round);
+		bytes_this_round = min(cc->element->size - cc->element_offset,
+				       maxsize - total_bytes_copied);
 
-	if (res != 0)
-		return -EFAULT;
+		if (copy_from_user(dest + total_bytes_copied,
+				  cc->element->data + cc->element_offset,
+				  bytes_this_round))
+			return -EFAULT;
 
-	copy_context->current_element_offset += bytes_this_round;
-	copy_context->current_offset += bytes_this_round;
+		cc->element_offset += bytes_this_round;
+		total_bytes_copied += bytes_this_round;
 
-	/*
-	 * Check if done with current element, and if so advance to the next.
-	 */
-	if (copy_context->current_element_offset ==
-	    copy_context->current_element->size) {
-		copy_context->elements_to_go--;
-		copy_context->current_element++;
-		copy_context->current_element_offset = 0;
+		if (cc->element_offset == cc->element->size) {
+			cc->elements_to_go--;
+			cc->element++;
+			cc->element_offset = 0;
+		}
 	}
 
-	return bytes_this_round;
+	return maxsize;
 }
 
 /**************************************************************************
@@ -836,10 +810,9 @@ vchiq_ioc_queue_message(VCHIQ_SERVICE_HANDLE_T handle,
 	unsigned long i;
 	size_t total_size = 0;
 
-	context.current_element = elements;
-	context.current_element_offset = 0;
+	context.element = elements;
+	context.element_offset = 0;
 	context.elements_to_go = count;
-	context.current_offset = 0;
 
 	for (i = 0; i < count; i++) {
 		if (!elements[i].data && elements[i].size != 0)
-- 
2.19.1


WARNING: multiple messages have this Message-ID
From: nsaenzjulienne@suse.de (Nicolas Saenz Julienne)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH RFC 06/18] staging: vchiq_arm: rework vchiq_ioc_copy_element_data
Date: Fri, 26 Oct 2018 15:48:01 +0200	[thread overview]
Message-ID: <20181026134813.7775-7-nsaenzjulienne@suse.de> (raw)
In-Reply-To: <20181026134813.7775-1-nsaenzjulienne@suse.de>

The function is passed to vchiq_core.c for it to go trough all the
transfer elements (an array of pointers to data) and copy them into the
actual transfer memory (contiguous memory).

The logic in the function was "copy an element and return, except when
the element is empty, in which case look for the next non-empty element
and copy it. The function will be called as many times as necessary until
all the elements are copied".

Now, this approach already forces the function to loop around elements
and felt convoluted, so it was changed to a more straightforward "Copy
all the elements into memory as long as they fit".

The resulting function is shorter and simpler.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 .../interface/vchiq_arm/vchiq_arm.c           | 89 +++++++------------
 1 file changed, 31 insertions(+), 58 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 34160cc3b8bd..1cdfdb714abc 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -752,74 +752,48 @@ static void close_delivered(USER_SERVICE_T *user_service)
 }
 
 struct vchiq_io_copy_callback_context {
-	struct vchiq_element *current_element;
-	size_t current_element_offset;
+	struct vchiq_element *element;
+	size_t element_offset;
 	unsigned long elements_to_go;
-	size_t current_offset;
 };
 
-static ssize_t
-vchiq_ioc_copy_element_data(
-	void *context,
-	void *dest,
-	size_t offset,
-	size_t maxsize)
+static ssize_t vchiq_ioc_copy_element_data(void *context, void *dest,
+					   size_t offset, size_t maxsize)
 {
-	long res;
+	struct vchiq_io_copy_callback_context *cc = context;
+	size_t total_bytes_copied = 0;
 	size_t bytes_this_round;
-	struct vchiq_io_copy_callback_context *copy_context =
-		(struct vchiq_io_copy_callback_context *)context;
-
-	if (offset != copy_context->current_offset)
-		return 0;
-
-	if (!copy_context->elements_to_go)
-		return 0;
-
-	/*
-	 * Complex logic here to handle the case of 0 size elements
-	 * in the middle of the array of elements.
-	 *
-	 * Need to skip over these 0 size elements.
-	 */
-	while (1) {
-		bytes_this_round = min(copy_context->current_element->size -
-				       copy_context->current_element_offset,
-				       maxsize);
-
-		if (bytes_this_round)
-			break;
 
-		copy_context->elements_to_go--;
-		copy_context->current_element++;
-		copy_context->current_element_offset = 0;
+	while (total_bytes_copied < maxsize) {
+		if (!cc->elements_to_go)
+			return total_bytes_copied;
 
-		if (!copy_context->elements_to_go)
-			return 0;
-	}
+		if (!cc->element->size) {
+			cc->elements_to_go--;
+			cc->element++;
+			cc->element_offset = 0;
+			continue;
+		}
 
-	res = copy_from_user(dest,
-			     copy_context->current_element->data +
-			     copy_context->current_element_offset,
-			     bytes_this_round);
+		bytes_this_round = min(cc->element->size - cc->element_offset,
+				       maxsize - total_bytes_copied);
 
-	if (res != 0)
-		return -EFAULT;
+		if (copy_from_user(dest + total_bytes_copied,
+				  cc->element->data + cc->element_offset,
+				  bytes_this_round))
+			return -EFAULT;
 
-	copy_context->current_element_offset += bytes_this_round;
-	copy_context->current_offset += bytes_this_round;
+		cc->element_offset += bytes_this_round;
+		total_bytes_copied += bytes_this_round;
 
-	/*
-	 * Check if done with current element, and if so advance to the next.
-	 */
-	if (copy_context->current_element_offset ==
-	    copy_context->current_element->size) {
-		copy_context->elements_to_go--;
-		copy_context->current_element++;
-		copy_context->current_element_offset = 0;
+		if (cc->element_offset == cc->element->size) {
+			cc->elements_to_go--;
+			cc->element++;
+			cc->element_offset = 0;
+		}
 	}
 
-	return bytes_this_round;
+	return maxsize;
 }
 
 /**************************************************************************
@@ -836,10 +810,9 @@ vchiq_ioc_queue_message(VCHIQ_SERVICE_HANDLE_T handle,
 	unsigned long i;
 	size_t total_size = 0;
 
-	context.current_element = elements;
-	context.current_element_offset = 0;
+	context.element = elements;
+	context.element_offset = 0;
 	context.elements_to_go = count;
-	context.current_offset = 0;
 
 	for (i = 0; i < count; i++) {
 		if (!elements[i].data && elements[i].size != 0)
-- 
2.19.1

  parent reply	other threads:[~2018-10-26 13:49 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-26 13:47 [PATCH RFC 00/18] staging: vchiq: remove dead code & misc fixes Nicolas Saenz Julienne
2018-10-26 13:47 ` Nicolas Saenz Julienne
2018-10-26 13:47 ` [PATCH RFC 01/18] staging: vchiq_core: rework vchiq_get_config Nicolas Saenz Julienne
2018-10-26 13:47   ` Nicolas Saenz Julienne
2018-10-26 13:47 ` [PATCH RFC 02/18] staging: vchiq_arm: rework close/remove_service IOCTLS Nicolas Saenz Julienne
2018-10-26 13:47   ` Nicolas Saenz Julienne
2018-10-26 13:47 ` [PATCH RFC 03/18] staging: vchiq_shim: delete vchi_service_create Nicolas Saenz Julienne
2018-10-26 13:47   ` Nicolas Saenz Julienne
2018-10-26 13:47 ` [PATCH RFC 04/18] stagning: vchiq_arm: use list_for_each_entry when accessing bulk_waiter_list Nicolas Saenz Julienne
2018-10-26 13:47   ` Nicolas Saenz Julienne
2018-10-26 13:48 ` [PATCH RFC 05/18] staging: vchiq_arm: get rid of vchi_mh.h Nicolas Saenz Julienne
2018-10-26 13:48   ` Nicolas Saenz Julienne
2018-10-26 13:48 ` Nicolas Saenz Julienne [this message]
2018-10-26 13:48   ` [PATCH RFC 06/18] staging: vchiq_arm: rework vchiq_ioc_copy_element_data Nicolas Saenz Julienne
2018-10-26 13:48 ` [PATCH RFC 07/18] staging: vchiq-core: get rid of is_master distinction Nicolas Saenz Julienne
2018-10-26 13:48   ` Nicolas Saenz Julienne
2018-10-26 13:48 ` [PATCH RFC 08/18] staging: vchiq_core: remove unnecessary safety checks in vchiq_init_state Nicolas Saenz Julienne
2018-10-26 13:48   ` Nicolas Saenz Julienne
2018-10-26 13:48 ` [PATCH RFC 09/18] staging: vchiq_core: do not initialize semaphores twice Nicolas Saenz Julienne
2018-10-26 13:48   ` Nicolas Saenz Julienne
2018-10-28 20:45   ` Stefan Wahren
2018-10-28 20:45     ` Stefan Wahren
2018-11-06 15:41     ` Nicolas Saenz Julienne
2018-11-06 15:41       ` Nicolas Saenz Julienne
2018-11-06 16:06       ` Stefan Wahren
2018-11-06 16:06         ` Stefan Wahren
2018-11-06 18:28         ` Nicolas Saenz Julienne
2018-11-06 18:28           ` Nicolas Saenz Julienne
2018-10-26 13:48 ` [PATCH RFC 10/18] staging: vchiq_core: don't add a wmb() before remote_event_signal() Nicolas Saenz Julienne
2018-10-26 13:48   ` Nicolas Saenz Julienne
2018-10-26 13:48 ` [PATCH RFC 11/18] staging: vchiq_arm: use completions instead of semaphores Nicolas Saenz Julienne
2018-10-26 13:48   ` Nicolas Saenz Julienne
2018-10-28 21:00   ` Stefan Wahren
2018-10-28 21:00     ` Stefan Wahren
2018-10-26 13:48 ` [PATCH RFC 12/18] staging: vchiq_util: " Nicolas Saenz Julienne
2018-10-26 13:48   ` Nicolas Saenz Julienne
2018-10-26 13:48 ` [PATCH RFC 13/18] staging: vchiq_core: " Nicolas Saenz Julienne
2018-10-26 13:48   ` Nicolas Saenz Julienne
2018-10-26 13:48 ` [PATCH RFC 14/18] staging: vchiq_util: get rid of unneeded memory barriers Nicolas Saenz Julienne
2018-10-26 13:48   ` Nicolas Saenz Julienne
2018-10-26 13:48 ` [PATCH RFC 15/18] stagning: vchiq_core: fix logic redundancy in parse_open Nicolas Saenz Julienne
2018-10-26 13:48   ` Nicolas Saenz Julienne
2018-10-28 20:58   ` Stefan Wahren
2018-10-28 20:58     ` Stefan Wahren
2018-10-26 13:48 ` [PATCH RFC 16/18] staging: vchiq_arm: rework probe and init functions Nicolas Saenz Julienne
2018-10-26 13:48   ` Nicolas Saenz Julienne
2018-10-28 21:02   ` Stefan Wahren
2018-10-28 21:02     ` Stefan Wahren
2018-10-26 13:48 ` [PATCH RFC 17/18] staging: vchiq_arm: fix open/release cdev functions Nicolas Saenz Julienne
2018-10-26 13:48   ` Nicolas Saenz Julienne
2018-10-26 13:48 ` [PATCH RFC 18/18] staging: vchiq: add more tasks to the TODO list Nicolas Saenz Julienne
2018-10-26 13:48   ` Nicolas Saenz Julienne
2018-10-28 21:11   ` Stefan Wahren
2018-10-28 21:11     ` Stefan Wahren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181026134813.7775-7-nsaenzjulienne@suse.de \
    --to=nsaenzjulienne@suse.de \
    --cc=dave.stevenson@raspberrypi.org \
    --cc=devel@driverdev.osuosl.org \
    --cc=eric@anholt.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=stefan.wahren@i2se.com \
    --subject='Re: [PATCH RFC 06/18] staging: vchiq_arm: rework vchiq_ioc_copy_element_data' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.