| Message ID | 1481230746-16741-2-git-send-email-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
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.
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. ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today.http://sdm.link/xeonphi
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);
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(-)