From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932758AbcLIH2M (ORCPT ); Fri, 9 Dec 2016 02:28:12 -0500 Received: from nautica.notk.org ([91.121.71.147]:36564 "EHLO nautica.notk.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932610AbcLIH2L (ORCPT ); Fri, 9 Dec 2016 02:28:11 -0500 Date: Fri, 9 Dec 2016 08:18:24 +0100 From: Dominique Martinet To: Stefano Stabellini Cc: v9fs-developer@lists.sourceforge.net, ericvh@gmail.com, rminnich@sandia.gov, linux-kernel@vger.kernel.org, lucho@ionkov.net Subject: Re: [V9fs-developer] [PATCH 2/5] 9p: store req details and callback in struct p9_req_t Message-ID: <20161209071824.GC18158@nautica> References: <1481230746-16741-1-git-send-email-sstabellini@kernel.org> <1481230746-16741-2-git-send-email-sstabellini@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1481230746-16741-2-git-send-email-sstabellini@kernel.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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