Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1338322 > unrolled thread

staging: comedi: COMEDI_BUFINFO: some behavioural changes

Back to article view | Back to linux.kernel

Contents

  staging: comedi: COMEDI_BUFINFO: some behavioural changes Ian Abbott <abbotti@mev.co.uk> - 2016-02-19 17:20 +0100
    [PATCH 2/8] staging: comedi: COMEDI_BUFINFO: force bytes_read or bytes_written to 0 Ian Abbott <abbotti@mev.co.uk> - 2016-02-19 17:20 +0100
    [PATCH 5/8] staging: comedi: COMEDI_BUFINFO: return error if no active command Ian Abbott <abbotti@mev.co.uk> - 2016-02-19 17:20 +0100
    [PATCH 8/8] staging: comedi: COMEDI_BUFINFO: terminate "write" command when stopped Ian Abbott <abbotti@mev.co.uk> - 2016-02-19 17:20 +0100
    [PATCH 7/8] staging: comedi: COMEDI_BUFINFO: return -EPIPE for abnormal read Ian Abbott <abbotti@mev.co.uk> - 2016-02-19 17:20 +0100

#1338322 — staging: comedi: COMEDI_BUFINFO: some behavioural changes

These patches change the behavior of the `COMEDI_BUFINFO` ioctl, which
is used to manage buffer positions for a previously set up asynchronous
acquisition command.  It is used instead of the read and write file
operations when the buffer has been mmapped.

Patches 1 to 4 are fairly innocuous.  Patches 5 to 8 change the error
handling when the asynchronous command is no longer running, allow the
subdevice to become "non-busy" automatically in more cases (causing
subsequent calls to return `-EINVAL`), and report abnormal command
termination with return value `-EPIPE` (which also makes the subdevice
become non-busy).

1) staging: comedi: COMEDI_BUFINFO: get amount freed, not amount
   allocated
2) staging: comedi: COMEDI_BUFINFO: force bytes_read or bytes_written to
   0
3) staging: comedi: COMEDI_BUFINFO: update buffer before becoming
   non-busy
4) staging: comedi: COMEDI_BUFINFO: force bytes_written to 0 if stopped
5) staging: comedi: COMEDI_BUFINFO: return error if no active command
6) staging: comedi: COMEDI_BUFINFO: become non-busy even if bytes_read
   is 0
7) staging: comedi: COMEDI_BUFINFO: return -EPIPE for abnormal read
8) staging: comedi: COMEDI_BUFINFO: terminate "write" command when
   stopped

 drivers/staging/comedi/comedi_fops.c | 84 +++++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 40 deletions(-)

[toc] | [next] | [standalone]

#1338323 — [PATCH 2/8] staging: comedi: COMEDI_BUFINFO: force bytes_read or bytes_written to 0

The `COMEDI_BUFINFO` ioctl is used to advance the current position in
the buffer by a specified amount (which can be 0) and get the new
position.  On input, the `bytes_read` member of `struct comedi_bufinfo`
specifies the amount to advance the "read" position for an asynchronous
command in the "read" direction, and the `bytes_written` member
specifies the amount to advance the "write" position for a command in
the "write" direction.  The handler `do_bufinfo_ioctl()` may adjust
these by the amount the position is actually advanced before copying
them back to the user.  Currently, it ignores the specified `bytes_read`
value for a command in the "write" direction, and ignores the specified
`bytes_written` for a command in the "read" direction, so the values
copied back to the user are unchanged.  Change it to force the ignored
value to 0 before copying the values back to the user.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index 2cfb61e..04d6040 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1141,19 +1141,25 @@ static int do_bufinfo_ioctl(struct comedi_device *dev,
 	if (s->busy != file)
 		return -EACCES;
 
-	if (bi.bytes_read && !(async->cmd.flags & CMDF_WRITE)) {
-		comedi_buf_read_alloc(s, bi.bytes_read);
-		bi.bytes_read = comedi_buf_read_free(s, bi.bytes_read);
-
-		if (comedi_is_subdevice_idle(s) &&
-		    comedi_buf_read_n_available(s) == 0) {
-			do_become_nonbusy(dev, s);
+	if (!(async->cmd.flags & CMDF_WRITE)) {
+		/* command was set up in "read" direction */
+		if (bi.bytes_read) {
+			comedi_buf_read_alloc(s, bi.bytes_read);
+			bi.bytes_read = comedi_buf_read_free(s, bi.bytes_read);
+
+			if (comedi_is_subdevice_idle(s) &&
+			    comedi_buf_read_n_available(s) == 0)
+				do_become_nonbusy(dev, s);
 		}
-	}
-
-	if (bi.bytes_written && (async->cmd.flags & CMDF_WRITE)) {
-		comedi_buf_write_alloc(s, bi.bytes_written);
-		bi.bytes_written = comedi_buf_write_free(s, bi.bytes_written);
+		bi.bytes_written = 0;
+	} else {
+		/* command was set up in "write" direction */
+		if (bi.bytes_written) {
+			comedi_buf_write_alloc(s, bi.bytes_written);
+			bi.bytes_written =
+			    comedi_buf_write_free(s, bi.bytes_written);
+		}
+		bi.bytes_read = 0;
 	}
 
 copyback_position:
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1338325 — [PATCH 5/8] staging: comedi: COMEDI_BUFINFO: return error if no active command

The `COMEDI_BUFINFO` ioctl is used to advance the current position in
the buffer and/or get the current buffer position.  If no asynchronous
command is active (started via the file object that issued this ioctl),
this information is meaningless.  Change it to return an error
(`-EINVAL`) in this case.  Prior to this change, if a command was
started via a different file object, the ioctl returned `-EACCES`, but
now it will return `-EINVAL`, which is consistent with the current
behavior of the "read" and "write" file operation handlers.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index b7c9270..a9fabf7 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1123,24 +1123,8 @@ static int do_bufinfo_ioctl(struct comedi_device *dev,
 
 	async = s->async;
 
-	if (!async) {
-		dev_dbg(dev->class_dev,
-			"subdevice does not have async capability\n");
-		bi.buf_write_ptr = 0;
-		bi.buf_read_ptr = 0;
-		bi.buf_write_count = 0;
-		bi.buf_read_count = 0;
-		bi.bytes_read = 0;
-		bi.bytes_written = 0;
-		goto copyback;
-	}
-	if (!s->busy) {
-		bi.bytes_read = 0;
-		bi.bytes_written = 0;
-		goto copyback_position;
-	}
-	if (s->busy != file)
-		return -EACCES;
+	if (!async || s->busy != file)
+		return -EINVAL;
 
 	if (!(async->cmd.flags & CMDF_WRITE)) {
 		/* command was set up in "read" direction */
@@ -1165,7 +1149,6 @@ static int do_bufinfo_ioctl(struct comedi_device *dev,
 		bi.bytes_read = 0;
 	}
 
-copyback_position:
 	bi.buf_write_count = async->buf_write_count;
 	bi.buf_write_ptr = async->buf_write_ptr;
 	bi.buf_read_count = async->buf_read_count;
@@ -1174,7 +1157,6 @@ copyback_position:
 	if (become_nonbusy)
 		do_become_nonbusy(dev, s);
 
-copyback:
 	if (copy_to_user(arg, &bi, sizeof(bi)))
 		return -EFAULT;
 
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1338326 — [PATCH 8/8] staging: comedi: COMEDI_BUFINFO: terminate "write" command when stopped

The `COMEDI_BUFINFO` ioctl is used to advance the current position in
the buffer by a specified amount (which can be 0) and get the current
position.  An asynchronous command in the "read" direction is terminated
automatically once it has stopped and information about the final
position and error has been reported back to the user.  That is not
currently done for commands in the "write" direction.  Change it to
terminate the command in the "write" direction automatically.  If the
command stopped with an error, report an `EPIPE` error back to the user,
otherwise just report the final buffer position back to the user.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index 7cb1d06..d1cf6a1 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1121,6 +1121,7 @@ static int do_bufinfo_ioctl(struct comedi_device *dev,
 	if (!async || s->busy != file)
 		return -EINVAL;
 
+	runflags = comedi_get_subdevice_runflags(s);
 	if (!(async->cmd.flags & CMDF_WRITE)) {
 		/* command was set up in "read" direction */
 		if (bi.bytes_read) {
@@ -1132,7 +1133,6 @@ static int do_bufinfo_ioctl(struct comedi_device *dev,
 		 * {"read" position not updated or command stopped normally},
 		 * then become non-busy.
 		 */
-		runflags = comedi_get_subdevice_runflags(s);
 		if (comedi_buf_read_n_available(s) == 0 &&
 		    !comedi_is_runflags_running(runflags) &&
 		    (bi.bytes_read == 0 ||
@@ -1144,9 +1144,12 @@ static int do_bufinfo_ioctl(struct comedi_device *dev,
 		bi.bytes_written = 0;
 	} else {
 		/* command was set up in "write" direction */
-		if (!comedi_is_subdevice_running(s))
+		if (!comedi_is_runflags_running(runflags)) {
 			bi.bytes_written = 0;
-		if (bi.bytes_written) {
+			become_nonbusy = true;
+			if (comedi_is_runflags_in_error(runflags))
+				retval = -EPIPE;
+		} else if (bi.bytes_written) {
 			comedi_buf_write_alloc(s, bi.bytes_written);
 			bi.bytes_written =
 			    comedi_buf_write_free(s, bi.bytes_written);
-- 
2.7.0

[toc] | [prev] | [next] | [standalone]

#1338328 — [PATCH 7/8] staging: comedi: COMEDI_BUFINFO: return -EPIPE for abnormal read

The `COMEDI_BUFINFO` ioctl is used to advance the current position in
the buffer by a specified amount (which can be 0) and get the current
position.  If an asynchronous command in the "read" direction has
stopped normally, the command is terminated as soon as the position has
been advanced to the end of all available data.  This is not currently
done if the command terminated with an error.  Change it to allow the
command to be terminated even if it stopped with an error, but report an
`EPIPE` error to the user first.  The `EPIPE` error will not be
reported until the "read" position reported back to the user has been
advanced to the end of all available data.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index ffe58208..7cb1d06 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -686,13 +686,6 @@ static bool __comedi_is_subdevice_running(struct comedi_subdevice *s)
 	return comedi_is_runflags_running(runflags);
 }
 
-static bool comedi_is_subdevice_idle(struct comedi_subdevice *s)
-{
-	unsigned runflags = comedi_get_subdevice_runflags(s);
-
-	return !(runflags & COMEDI_SRF_BUSY_MASK);
-}
-
 bool comedi_can_auto_free_spriv(struct comedi_subdevice *s)
 {
 	unsigned runflags = __comedi_get_subdevice_runflags(s);
@@ -1111,6 +1104,8 @@ static int do_bufinfo_ioctl(struct comedi_device *dev,
 	struct comedi_bufinfo bi;
 	struct comedi_subdevice *s;
 	struct comedi_async *async;
+	unsigned int runflags;
+	int retval = 0;
 	bool become_nonbusy = false;
 
 	if (copy_from_user(&bi, arg, sizeof(bi)))
@@ -1132,9 +1127,20 @@ static int do_bufinfo_ioctl(struct comedi_device *dev,
 			comedi_buf_read_alloc(s, bi.bytes_read);
 			bi.bytes_read = comedi_buf_read_free(s, bi.bytes_read);
 		}
-		if (comedi_is_subdevice_idle(s) &&
-		    comedi_buf_read_n_available(s) == 0)
+		/*
+		 * If nothing left to read, and command has stopped, and
+		 * {"read" position not updated or command stopped normally},
+		 * then become non-busy.
+		 */
+		runflags = comedi_get_subdevice_runflags(s);
+		if (comedi_buf_read_n_available(s) == 0 &&
+		    !comedi_is_runflags_running(runflags) &&
+		    (bi.bytes_read == 0 ||
+		     !comedi_is_runflags_in_error(runflags))) {
 			become_nonbusy = true;
+			if (comedi_is_runflags_in_error(runflags))
+				retval = -EPIPE;
+		}
 		bi.bytes_written = 0;
 	} else {
 		/* command was set up in "write" direction */
@@ -1156,6 +1162,9 @@ static int do_bufinfo_ioctl(struct comedi_device *dev,
 	if (become_nonbusy)
 		do_become_nonbusy(dev, s);
 
+	if (retval)
+		return retval;
+
 	if (copy_to_user(arg, &bi, sizeof(bi)))
 		return -EFAULT;
 
-- 
2.7.0

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web