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

Groups > linux.kernel > #1538875 > unrolled thread

[PATCH 0/5] async requests support for 9pfs

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] async requests support for 9pfs Stefano Stabellini <sstabellini@kernel.org> - 2016-12-08 22:00 +0100
    [PATCH 2/5] 9p: store req details and callback in struct p9_req_t Stefano Stabellini <sstabellini@kernel.org> - 2016-12-08 22:00 +0100
      Re: [V9fs-developer] [PATCH 2/5] 9p: store req details and callback  in struct p9_req_t Dominique Martinet <dominique.martinet@cea.fr> - 2016-12-09 08:30 +0100
        Re: [V9fs-developer] [PATCH 2/5] 9p: store req details and callback  in struct p9_req_t Stefano Stabellini <sstabellini@kernel.org> - 2016-12-10 00:30 +0100
    [PATCH 3/5] 9p: introduce p9_client_get_req Stefano Stabellini <sstabellini@kernel.org> - 2016-12-08 22:10 +0100
    [PATCH 1/5] 9p: add iocb parameter to p9_client_read and p9_client_write Stefano Stabellini <sstabellini@kernel.org> - 2016-12-08 22:10 +0100

#1538875 — [PATCH 0/5] async requests support for 9pfs

Hi all,

This patch series introduces async requests for read and write
operations. If the read, or the write, is an async operation to begin
with (aio), we can avoid waiting for the server response.

This is my first contribution to 9p, so feedback and suggestions are
welcome!


Stefano Stabellini (5):
      9p: add iocb parameter to p9_client_read and p9_client_write
      9p: store req details and callback in struct p9_req_t
      9p: introduce p9_client_get_req
      9p: introduce async read requests
      9p: introduce async write requests

 fs/9p/vfs_addr.c        |   8 +-
 fs/9p/vfs_dir.c         |   2 +-
 fs/9p/vfs_file.c        |   4 +-
 fs/9p/xattr.c           |   4 +-
 include/net/9p/client.h |  15 +++-
 net/9p/client.c         | 195 ++++++++++++++++++++++++++++++++++++++++++++++--
 6 files changed, 211 insertions(+), 17 deletions(-)

[toc] | [next] | [standalone]

#1538877 — [PATCH 2/5] 9p: store req details and callback in struct p9_req_t

Add a few fields to struct p9_req_t. Callback is the function which will
be called upon requestion completion. offset, rsize, pagevec and kiocb
store important information regarding the read or write request,
essential to complete the request.

Currently not utilized, but they will be used in a later patch.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
 include/net/9p/client.h | 8 ++++++++
 net/9p/client.c         | 9 ++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/net/9p/client.h b/include/net/9p/client.h
index aef19c6..69fc2f0 100644
--- a/include/net/9p/client.h
+++ b/include/net/9p/client.h
@@ -110,6 +110,7 @@ enum p9_req_status_t {
  *
  */
 
+struct p9_client;
 struct p9_req_t {
 	int status;
 	int t_err;
@@ -118,6 +119,13 @@ struct p9_req_t {
 	struct p9_fcall *rc;
 	void *aux;
 
+    /* Used for async requests */
+	void (*callback)(struct p9_client *c, struct p9_req_t *req, int status);
+	size_t offset;
+	u64 rsize;
+	struct page **pagevec;
+	struct kiocb *kiocb;
+
 	struct list_head req_list;
 };
 
diff --git a/net/9p/client.c b/net/9p/client.c
index b5ea9a3..bfe1715 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -405,6 +405,10 @@ static void p9_free_req(struct p9_client *c, struct p9_req_t *r)
 	int tag = r->tc->tag;
 	p9_debug(P9_DEBUG_MUX, "clnt %p req %p tag: %d\n", c, r, tag);
 
+	r->offset = 0;
+	r->rsize = 0;
+	r->kiocb = NULL;
+	r->callback = NULL;
 	r->status = REQ_STATUS_IDLE;
 	if (tag != P9_NOTAG && p9_idpool_check(tag, c->tagpool))
 		p9_idpool_put(tag, c->tagpool);
@@ -427,7 +431,10 @@ void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status)
 	smp_wmb();
 	req->status = status;
 
-	wake_up(req->wq);
+	if (req->callback != NULL)
+		req->callback(c, req, status);
+	else
+		wake_up(req->wq);
 	p9_debug(P9_DEBUG_MUX, "wakeup: %d\n", req->tc->tag);
 }
 EXPORT_SYMBOL(p9_client_cb);
-- 
1.9.1

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

#1539152 — Re: [V9fs-developer] [PATCH 2/5] 9p: store req details and callback in struct p9_req_t

Nice. I like the idea of async I/Os :)

Stefano Stabellini wrote on Thu, Dec 08, 2016:
> Add a few fields to struct p9_req_t. Callback is the function which will
> be called upon requestion completion. offset, rsize, pagevec and kiocb
> store important information regarding the read or write request,
> essential to complete the request.
> 
> Currently not utilized, but they will be used in a later patch.
> 
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
>  include/net/9p/client.h | 8 ++++++++
>  net/9p/client.c         | 9 ++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/9p/client.h b/include/net/9p/client.h
> index aef19c6..69fc2f0 100644
> --- a/include/net/9p/client.h
> +++ b/include/net/9p/client.h
> @@ -110,6 +110,7 @@ enum p9_req_status_t {
>   *
>   */
>  
> +struct p9_client;
>  struct p9_req_t {
>  	int status;
>  	int t_err;
> @@ -118,6 +119,13 @@ struct p9_req_t {
>  	struct p9_fcall *rc;
>  	void *aux;
>  
> +    /* Used for async requests */
> +	void (*callback)(struct p9_client *c, struct p9_req_t *req, int status);
> +	size_t offset;
> +	u64 rsize;
> +	struct page **pagevec;
> +	struct kiocb *kiocb;
> +
>  	struct list_head req_list;
>  };
>  
> diff --git a/net/9p/client.c b/net/9p/client.c
> index b5ea9a3..bfe1715 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -405,6 +405,10 @@ static void p9_free_req(struct p9_client *c, struct p9_req_t *r)
>  	int tag = r->tc->tag;
>  	p9_debug(P9_DEBUG_MUX, "clnt %p req %p tag: %d\n", c, r, tag);
>  
> +	r->offset = 0;
> +	r->rsize = 0;
> +	r->kiocb = NULL;
> +	r->callback = NULL;

Probably want to cleanup r->pagevec here too, even if that doesn't seem
to have any implication short-term (e.g. only looked at if callback is
not empty from what I've seen)

>  	r->status = REQ_STATUS_IDLE;
>  	if (tag != P9_NOTAG && p9_idpool_check(tag, c->tagpool))
>  		p9_idpool_put(tag, c->tagpool);
> @@ -427,7 +431,10 @@ void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status)
>  	smp_wmb();
>  	req->status = status;
>  
> -	wake_up(req->wq);
> +	if (req->callback != NULL)
> +		req->callback(c, req, status);
> +	else
> +		wake_up(req->wq);
>  	p9_debug(P9_DEBUG_MUX, "wakeup: %d\n", req->tc->tag);
>  }
>  EXPORT_SYMBOL(p9_client_cb);

Mostly a warning here, but p9_client_cb is called from an interrupt
context in 9P/RDMA.
This has been working up till now because we only do a wake_up and
there's no waiting, but (looking at later patches),
p9_client_read_complete for example does allocations and possibly other
unsafe operations from an interrupt context.

I don't know if the way forward is to move p9_client_cb from that
context or to have the callback be scheduled in a work queue instead;
but we'll need to fix that later.

-- 
Dominique Martinet

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

#1539730 — Re: [V9fs-developer] [PATCH 2/5] 9p: store req details and callback in struct p9_req_t

On Fri, 9 Dec 2016, Dominique Martinet wrote:
> Nice. I like the idea of async I/Os :)
> 
> Stefano Stabellini wrote on Thu, Dec 08, 2016:
> > Add a few fields to struct p9_req_t. Callback is the function which will
> > be called upon requestion completion. offset, rsize, pagevec and kiocb
> > store important information regarding the read or write request,
> > essential to complete the request.
> > 
> > Currently not utilized, but they will be used in a later patch.
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
> >  include/net/9p/client.h | 8 ++++++++
> >  net/9p/client.c         | 9 ++++++++-
> >  2 files changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/net/9p/client.h b/include/net/9p/client.h
> > index aef19c6..69fc2f0 100644
> > --- a/include/net/9p/client.h
> > +++ b/include/net/9p/client.h
> > @@ -110,6 +110,7 @@ enum p9_req_status_t {
> >   *
> >   */
> >  
> > +struct p9_client;
> >  struct p9_req_t {
> >  	int status;
> >  	int t_err;
> > @@ -118,6 +119,13 @@ struct p9_req_t {
> >  	struct p9_fcall *rc;
> >  	void *aux;
> >  
> > +    /* Used for async requests */
> > +	void (*callback)(struct p9_client *c, struct p9_req_t *req, int status);
> > +	size_t offset;
> > +	u64 rsize;
> > +	struct page **pagevec;
> > +	struct kiocb *kiocb;
> > +
> >  	struct list_head req_list;
> >  };
> >  
> > diff --git a/net/9p/client.c b/net/9p/client.c
> > index b5ea9a3..bfe1715 100644
> > --- a/net/9p/client.c
> > +++ b/net/9p/client.c
> > @@ -405,6 +405,10 @@ static void p9_free_req(struct p9_client *c, struct p9_req_t *r)
> >  	int tag = r->tc->tag;
> >  	p9_debug(P9_DEBUG_MUX, "clnt %p req %p tag: %d\n", c, r, tag);
> >  
> > +	r->offset = 0;
> > +	r->rsize = 0;
> > +	r->kiocb = NULL;
> > +	r->callback = NULL;
> 
> Probably want to cleanup r->pagevec here too, even if that doesn't seem
> to have any implication short-term (e.g. only looked at if callback is
> not empty from what I've seen)

Thanks, I missed it.


> >  	r->status = REQ_STATUS_IDLE;
> >  	if (tag != P9_NOTAG && p9_idpool_check(tag, c->tagpool))
> >  		p9_idpool_put(tag, c->tagpool);
> > @@ -427,7 +431,10 @@ void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status)
> >  	smp_wmb();
> >  	req->status = status;
> >  
> > -	wake_up(req->wq);
> > +	if (req->callback != NULL)
> > +		req->callback(c, req, status);
> > +	else
> > +		wake_up(req->wq);
> >  	p9_debug(P9_DEBUG_MUX, "wakeup: %d\n", req->tc->tag);
> >  }
> >  EXPORT_SYMBOL(p9_client_cb);
> 
> Mostly a warning here, but p9_client_cb is called from an interrupt
> context in 9P/RDMA.
> This has been working up till now because we only do a wake_up and
> there's no waiting, but (looking at later patches),
> p9_client_read_complete for example does allocations and possibly other
> unsafe operations from an interrupt context.
> 
> I don't know if the way forward is to move p9_client_cb from that
> context or to have the callback be scheduled in a work queue instead;
> but we'll need to fix that later.

Either would work. It might be simpler to have the callback run as a
work queue. I'll make the change. Maybe I'll use kiocb to figure out if
we have to schedule_work.

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

#1538883 — [PATCH 3/5] 9p: introduce p9_client_get_req

Introduce a simple helper function to only prepare a p9 client request,
without any waiting involved.

Currently not utilized, but it will be used by a later patch.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
 net/9p/client.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/9p/client.c b/net/9p/client.c
index bfe1715..eb589ef 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -719,6 +719,18 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c,
 	return ERR_PTR(err);
 }
 
+static struct p9_req_t *
+p9_client_get_req(struct p9_client *c, int8_t type, const char *fmt, ...)
+{
+	va_list ap;
+	struct p9_req_t *req;
+
+	va_start(ap, fmt);
+	req = p9_client_prepare_req(c, type, c->msize, fmt, ap);
+	va_end(ap);
+	return req;
+}
+
 /**
  * p9_client_rpc - issue a request and wait for a response
  * @c: client session
-- 
1.9.1

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

#1538885 — [PATCH 1/5] 9p: add iocb parameter to p9_client_read and p9_client_write

The parameter can be NULL.
Currently not utilized, but it will be used in later patches.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
---
 fs/9p/vfs_addr.c        | 8 ++++----
 fs/9p/vfs_dir.c         | 2 +-
 fs/9p/vfs_file.c        | 4 ++--
 fs/9p/xattr.c           | 4 ++--
 include/net/9p/client.h | 7 +++++--
 net/9p/client.c         | 6 ++++--
 6 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
index 6181ad7..99ba284 100644
--- a/fs/9p/vfs_addr.c
+++ b/fs/9p/vfs_addr.c
@@ -66,7 +66,7 @@ static int v9fs_fid_readpage(struct p9_fid *fid, struct page *page)
 
 	iov_iter_bvec(&to, ITER_BVEC | READ, &bvec, 1, PAGE_SIZE);
 
-	retval = p9_client_read(fid, page_offset(page), &to, &err);
+	retval = p9_client_read(fid, NULL, page_offset(page), &to, &err);
 	if (err) {
 		v9fs_uncache_page(inode, page);
 		retval = err;
@@ -181,7 +181,7 @@ static int v9fs_vfs_writepage_locked(struct page *page)
 
 	set_page_writeback(page);
 
-	p9_client_write(v9inode->writeback_fid, page_offset(page), &from, &err);
+	p9_client_write(v9inode->writeback_fid, NULL, page_offset(page), &from, &err);
 
 	end_page_writeback(page);
 	return err;
@@ -251,7 +251,7 @@ static int v9fs_launder_page(struct page *page)
 	ssize_t n;
 	int err = 0;
 	if (iov_iter_rw(iter) == WRITE) {
-		n = p9_client_write(file->private_data, pos, iter, &err);
+		n = p9_client_write(file->private_data, iocb, pos, iter, &err);
 		if (n) {
 			struct inode *inode = file_inode(file);
 			loff_t i_size = i_size_read(inode);
@@ -259,7 +259,7 @@ static int v9fs_launder_page(struct page *page)
 				inode_add_bytes(inode, pos + n - i_size);
 		}
 	} else {
-		n = p9_client_read(file->private_data, pos, iter, &err);
+		n = p9_client_read(file->private_data, iocb, pos, iter, &err);
 	}
 	return n ? n : err;
 }
diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c
index b0405d6..68557e0 100644
--- a/fs/9p/vfs_dir.c
+++ b/fs/9p/vfs_dir.c
@@ -134,7 +134,7 @@ static int v9fs_dir_readdir(struct file *file, struct dir_context *ctx)
 			struct iov_iter to;
 			int n;
 			iov_iter_kvec(&to, READ | ITER_KVEC, &kvec, 1, buflen);
-			n = p9_client_read(file->private_data, ctx->pos, &to,
+			n = p9_client_read(file->private_data, NULL, ctx->pos, &to,
 					   &err);
 			if (err)
 				return err;
diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
index d7b78d5..79e8c7d 100644
--- a/fs/9p/vfs_file.c
+++ b/fs/9p/vfs_file.c
@@ -387,7 +387,7 @@ static int v9fs_file_flock_dotl(struct file *filp, int cmd,
 	p9_debug(P9_DEBUG_VFS, "count %zu offset %lld\n",
 		 iov_iter_count(to), iocb->ki_pos);
 
-	ret = p9_client_read(fid, iocb->ki_pos, to, &err);
+	ret = p9_client_read(fid, iocb, iocb->ki_pos, to, &err);
 	if (!ret)
 		return err;
 
@@ -416,7 +416,7 @@ static int v9fs_file_flock_dotl(struct file *filp, int cmd,
 		return retval;
 
 	origin = iocb->ki_pos;
-	retval = p9_client_write(file->private_data, iocb->ki_pos, from, &err);
+	retval = p9_client_write(file->private_data, iocb, iocb->ki_pos, from, &err);
 	if (retval > 0) {
 		struct inode *inode = file_inode(file);
 		loff_t i_size;
diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
index f329eee..5e137c0 100644
--- a/fs/9p/xattr.c
+++ b/fs/9p/xattr.c
@@ -48,7 +48,7 @@ ssize_t v9fs_fid_xattr_get(struct p9_fid *fid, const char *name,
 			retval = -ERANGE;
 	} else {
 		iov_iter_truncate(&to, attr_size);
-		retval = p9_client_read(attr_fid, 0, &to, &err);
+		retval = p9_client_read(attr_fid, NULL, 0, &to, &err);
 		if (err)
 			retval = err;
 	}
@@ -125,7 +125,7 @@ int v9fs_fid_xattr_set(struct p9_fid *fid, const char *name,
 		p9_debug(P9_DEBUG_VFS, "p9_client_xattrcreate failed %d\n",
 			 retval);
 	else
-		p9_client_write(fid, 0, &from, &retval);
+		p9_client_write(fid, NULL, 0, &from, &retval);
 	p9_client_clunk(fid);
 	return retval;
 }
diff --git a/include/net/9p/client.h b/include/net/9p/client.h
index c6b97e5..aef19c6 100644
--- a/include/net/9p/client.h
+++ b/include/net/9p/client.h
@@ -26,6 +26,7 @@
 #ifndef NET_9P_CLIENT_H
 #define NET_9P_CLIENT_H
 
+#include <linux/fs.h>
 #include <linux/utsname.h>
 
 /* Number of requests per row */
@@ -238,8 +239,10 @@ int p9_client_create_dotl(struct p9_fid *ofid, char *name, u32 flags, u32 mode,
 int p9_client_fsync(struct p9_fid *fid, int datasync);
 int p9_client_remove(struct p9_fid *fid);
 int p9_client_unlinkat(struct p9_fid *dfid, const char *name, int flags);
-int p9_client_read(struct p9_fid *fid, u64 offset, struct iov_iter *to, int *err);
-int p9_client_write(struct p9_fid *fid, u64 offset, struct iov_iter *from, int *err);
+int p9_client_read(struct p9_fid *fid, struct kiocb *iocb, u64 offset,
+                    struct iov_iter *to, int *err);
+int p9_client_write(struct p9_fid *fid, struct kiocb *iocb, u64 offset,
+                    struct iov_iter *from, int *err);
 int p9_client_readdir(struct p9_fid *fid, char *data, u32 count, u64 offset);
 int p9dirent_read(struct p9_client *clnt, char *buf, int len,
 		  struct p9_dirent *dirent);
diff --git a/net/9p/client.c b/net/9p/client.c
index 3fc94a4..b5ea9a3 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -1536,7 +1536,8 @@ int p9_client_unlinkat(struct p9_fid *dfid, const char *name, int flags)
 EXPORT_SYMBOL(p9_client_unlinkat);
 
 int
-p9_client_read(struct p9_fid *fid, u64 offset, struct iov_iter *to, int *err)
+p9_client_read(struct p9_fid *fid, struct kiocb *iocb, u64 offset,
+				struct iov_iter *to, int *err)
 {
 	struct p9_client *clnt = fid->clnt;
 	struct p9_req_t *req;
@@ -1616,7 +1617,8 @@ int p9_client_unlinkat(struct p9_fid *dfid, const char *name, int flags)
 EXPORT_SYMBOL(p9_client_read);
 
 int
-p9_client_write(struct p9_fid *fid, u64 offset, struct iov_iter *from, int *err)
+p9_client_write(struct p9_fid *fid, struct kiocb *iocb, u64 offset,
+				struct iov_iter *from, int *err)
 {
 	struct p9_client *clnt = fid->clnt;
 	struct p9_req_t *req;
-- 
1.9.1

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web