[PATCH v1 0/4] fuse: fuse_dev_do_read() cleanups

[PATCH v1 0/4] fuse: fuse_dev_do_read() cleanups

[Joanne Koong]

This patchset contains some minor cleanups for the fuse_dev_do_read()
codepath.

Thanks,
Joanne

Joanne Koong (4):
  fuse: remove redundant buffer size checks for interrupt and forget
    requests
  fuse: get rid of err_unlock goto in fuse_dev_do_read()
  fuse: remove stray newline in fuse_dev_do_read()
  fuse: clean up interrupt reading

 fs/fuse/dev.c | 55 +++++++++++++++++----------------------------------
 1 file changed, 18 insertions(+), 37 deletions(-)

-- 
2.47.3

[PATCH v1 1/4] fuse: remove redundant buffer size checks for interrupt and forget requests

[Joanne Koong]

In fuse_dev_do_read(), there is already logic that ensures the buffer is
a minimum of at least FUSE_MIN_READ_BUFFER (8k) bytes.

This makes the buffer size checks for interrupt and forget requests
redundant as sizeof(struct fuse_in_header) + sizeof(struct
fuse_interrupt_in) and sizeof(struct fuse_in_header) + sizeof(struct
fuse_forget_in) are both less than FUSE_MIN_READ_BUFFER.

We can get rid of these checks altogether.

Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 fs/fuse/dev.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 2c16b94357d5..e57ede7351b9 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1250,7 +1250,7 @@ static int request_pending(struct fuse_iqueue *fiq)
  */
 static int fuse_read_interrupt(struct fuse_iqueue *fiq,
 			       struct fuse_copy_state *cs,
-			       size_t nbytes, struct fuse_req *req)
+			       struct fuse_req *req)
 __releases(fiq->lock)
 {
 	struct fuse_in_header ih;
@@ -1267,8 +1267,6 @@ __releases(fiq->lock)
 	arg.unique = req->in.h.unique;
 
 	spin_unlock(&fiq->lock);
-	if (nbytes < reqsize)
-		return -EINVAL;
 
 	err = fuse_copy_one(cs, &ih, sizeof(ih));
 	if (!err)
@@ -1301,8 +1299,7 @@ static struct fuse_forget_link *fuse_dequeue_forget(struct fuse_iqueue *fiq,
 }
 
 static int fuse_read_single_forget(struct fuse_iqueue *fiq,
-				   struct fuse_copy_state *cs,
-				   size_t nbytes)
+				   struct fuse_copy_state *cs)
 __releases(fiq->lock)
 {
 	int err;
@@ -1319,8 +1316,6 @@ __releases(fiq->lock)
 
 	spin_unlock(&fiq->lock);
 	kfree(forget);
-	if (nbytes < ih.len)
-		return -EINVAL;
 
 	err = fuse_copy_one(cs, &ih, sizeof(ih));
 	if (!err)
@@ -1348,11 +1343,6 @@ __releases(fiq->lock)
 		.len = sizeof(ih) + sizeof(arg),
 	};
 
-	if (nbytes < ih.len) {
-		spin_unlock(&fiq->lock);
-		return -EINVAL;
-	}
-
 	max_forgets = (nbytes - ih.len) / sizeof(struct fuse_forget_one);
 	head = fuse_dequeue_forget(fiq, max_forgets, &count);
 	spin_unlock(&fiq->lock);
@@ -1388,7 +1378,7 @@ static int fuse_read_forget(struct fuse_conn *fc, struct fuse_iqueue *fiq,
 __releases(fiq->lock)
 {
 	if (fc->minor < 16 || fiq->forget_list_head.next->next == NULL)
-		return fuse_read_single_forget(fiq, cs, nbytes);
+		return fuse_read_single_forget(fiq, cs);
 	else
 		return fuse_read_batch_forget(fiq, cs, nbytes);
 }
@@ -1455,7 +1445,7 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
 	if (!list_empty(&fiq->interrupts)) {
 		req = list_entry(fiq->interrupts.next, struct fuse_req,
 				 intr_entry);
-		return fuse_read_interrupt(fiq, cs, nbytes, req);
+		return fuse_read_interrupt(fiq, cs, req);
 	}
 
 	if (forget_pending(fiq)) {
-- 
2.47.3

[PATCH v1 2/4] fuse: get rid of err_unlock goto in fuse_dev_do_read()

[Joanne Koong]

There is one user of the err_unlock goto. Get rid of it and make the
code simpler.

Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 fs/fuse/dev.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index e57ede7351b9..8a0a541f3fba 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1438,8 +1438,8 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
 	}
 
 	if (!fiq->connected) {
-		err = fc->aborted ? -ECONNABORTED : -ENODEV;
-		goto err_unlock;
+		spin_unlock(&fiq->lock);
+		return fc->aborted ? -ECONNABORTED : -ENODEV;
 	}
 
 	if (!list_empty(&fiq->interrupts)) {
@@ -1524,10 +1524,6 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
 	spin_unlock(&fpq->lock);
 	fuse_request_end(req);
 	return err;
-
- err_unlock:
-	spin_unlock(&fiq->lock);
-	return err;
 }
 
 static int fuse_dev_open(struct inode *inode, struct file *file)
-- 
2.47.3

[PATCH v1 3/4] fuse: remove stray newline in fuse_dev_do_read()

[Joanne Koong]

Remove stray newline that shouldn't be there.

Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 fs/fuse/dev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 8a0a541f3fba..585677dfc82c 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1481,7 +1481,6 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
 	if (!fpq->connected) {
 		req->out.h.error = err = -ECONNABORTED;
 		goto out_end;
-
 	}
 	list_add(&req->list, &fpq->io);
 	spin_unlock(&fpq->lock);
-- 
2.47.3

[PATCH v1 4/4] fuse: clean up interrupt reading

[Joanne Koong]

Clean up interrupt reading logic. Remove passing the pointer to the fuse
request as an arg and make the header initializations more readable.

Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
 fs/fuse/dev.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 585677dfc82c..1402843c9068 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1249,23 +1249,22 @@ static int request_pending(struct fuse_iqueue *fiq)
  * Called with fiq->lock held, releases it
  */
 static int fuse_read_interrupt(struct fuse_iqueue *fiq,
-			       struct fuse_copy_state *cs,
-			       struct fuse_req *req)
+			       struct fuse_copy_state *cs)
 __releases(fiq->lock)
 {
-	struct fuse_in_header ih;
-	struct fuse_interrupt_in arg;
-	unsigned reqsize = sizeof(ih) + sizeof(arg);
+	struct fuse_req *req = list_entry(fiq->interrupts.next, struct fuse_req,
+					  intr_entry);
+	struct fuse_interrupt_in arg = {
+		.unique = req->in.h.unique,
+	};
+	struct fuse_in_header ih = {
+		.opcode = FUSE_INTERRUPT,
+		.unique = (req->in.h.unique | FUSE_INT_REQ_BIT),
+		.len = sizeof(ih) + sizeof(arg),
+	};
 	int err;
 
 	list_del_init(&req->intr_entry);
-	memset(&ih, 0, sizeof(ih));
-	memset(&arg, 0, sizeof(arg));
-	ih.len = reqsize;
-	ih.opcode = FUSE_INTERRUPT;
-	ih.unique = (req->in.h.unique | FUSE_INT_REQ_BIT);
-	arg.unique = req->in.h.unique;
-
 	spin_unlock(&fiq->lock);
 
 	err = fuse_copy_one(cs, &ih, sizeof(ih));
@@ -1273,7 +1272,7 @@ __releases(fiq->lock)
 		err = fuse_copy_one(cs, &arg, sizeof(arg));
 	fuse_copy_finish(cs);
 
-	return err ? err : reqsize;
+	return err ? err : ih.len;
 }
 
 static struct fuse_forget_link *fuse_dequeue_forget(struct fuse_iqueue *fiq,
@@ -1442,11 +1441,8 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
 		return fc->aborted ? -ECONNABORTED : -ENODEV;
 	}
 
-	if (!list_empty(&fiq->interrupts)) {
-		req = list_entry(fiq->interrupts.next, struct fuse_req,
-				 intr_entry);
-		return fuse_read_interrupt(fiq, cs, req);
-	}
+	if (!list_empty(&fiq->interrupts))
+		return fuse_read_interrupt(fiq, cs);
 
 	if (forget_pending(fiq)) {
 		if (list_empty(&fiq->pending) || fiq->forget_batch-- > 0)
-- 
2.47.3