From: Greg Kurz <groug@kaod.org>
To: Matthew Wilcox <willy@infradead.org>
Cc: Dominique Martinet <asmadeus@codewreck.org>,
Latchesar Ionkov <lucho@ionkov.net>,
Eric Van Hensbergen <ericvh@gmail.com>,
linux-kernel@vger.kernel.org, Ron Minnich <rminnich@sandia.gov>,
linux-fsdevel@vger.kernel.org,
v9fs-developer@lists.sourceforge.net
Subject: Re: [V9fs-developer] [PATCH v2 4/6] 9p: Embed wait_queue_head into p9_req_t
Date: Thu, 12 Jul 2018 16:36:49 +0200 [thread overview]
Message-ID: <20180712163649.36057b6e@bahia.lan> (raw)
In-Reply-To: <20180711210225.19730-5-willy@infradead.org>
On Wed, 11 Jul 2018 14:02:23 -0700
Matthew Wilcox <willy@infradead.org> wrote:
> On a 64-bit system, the wait_queue_head_t is 24 bytes while the pointer
> to it is 8 bytes. Growing the p9_req_t by 16 bytes is better than
> performing a 24-byte memory allocation.
>
This is true when all tags have been used at least once. But the current code
lazily allocates the wait_queue_head_t, ie, only when a tag is used for the
first time. Your patch causes a full row of wait_quest_head_t to be pre-allocated.
ie, P9_ROW_MAXTAG * 24 = 255 * 24 = 6120
instead of (P9_ROW_MAXTAG * 8) + 24 = 255 * 8 + 24 = 2064
This is nearly a page of allocated memory that might be never used.
Not sure if this is a problem though...
> Signed-off-by: Matthew Wilcox <willy@infradead.org>
> ---
> include/net/9p/client.h | 2 +-
> net/9p/client.c | 19 +++++--------------
> net/9p/trans_virtio.c | 2 +-
> 3 files changed, 7 insertions(+), 16 deletions(-)
>
... and the diffstat is nice :) so
Reviewed-by: Greg Kurz <groug@kaod.org>
> diff --git a/include/net/9p/client.h b/include/net/9p/client.h
> index e405729cd1c7..0fa0fbab33b0 100644
> --- a/include/net/9p/client.h
> +++ b/include/net/9p/client.h
> @@ -113,7 +113,7 @@ enum p9_req_status_t {
> struct p9_req_t {
> int status;
> int t_err;
> - wait_queue_head_t *wq;
> + wait_queue_head_t wq;
> struct p9_fcall *tc;
> struct p9_fcall *rc;
> void *aux;
> diff --git a/net/9p/client.c b/net/9p/client.c
> index b89c7298267c..bc8aba6b5ce0 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -282,8 +282,9 @@ p9_tag_alloc(struct p9_client *c, u16 tag, unsigned int max_size)
> return ERR_PTR(-ENOMEM);
> }
> for (col = 0; col < P9_ROW_MAXTAG; col++) {
> - c->reqs[row][col].status = REQ_STATUS_IDLE;
> - c->reqs[row][col].tc = NULL;
> + req = &c->reqs[row][col];
> + req->status = REQ_STATUS_IDLE;
> + init_waitqueue_head(&req->wq);
> }
> c->max_tag += P9_ROW_MAXTAG;
> }
> @@ -293,13 +294,6 @@ p9_tag_alloc(struct p9_client *c, u16 tag, unsigned int max_size)
> col = tag % P9_ROW_MAXTAG;
>
> req = &c->reqs[row][col];
> - if (!req->wq) {
> - req->wq = kmalloc(sizeof(wait_queue_head_t), GFP_NOFS);
> - if (!req->wq)
> - goto grow_failed;
> - init_waitqueue_head(req->wq);
> - }
> -
> if (!req->tc)
> req->tc = p9_fcall_alloc(alloc_msize);
> if (!req->rc)
> @@ -319,9 +313,7 @@ p9_tag_alloc(struct p9_client *c, u16 tag, unsigned int max_size)
> pr_err("Couldn't grow tag array\n");
> kfree(req->tc);
> kfree(req->rc);
> - kfree(req->wq);
> req->tc = req->rc = NULL;
> - req->wq = NULL;
> return ERR_PTR(-ENOMEM);
> }
>
> @@ -409,7 +401,6 @@ static void p9_tag_cleanup(struct p9_client *c)
> /* free requests associated with tags */
> for (row = 0; row < (c->max_tag/P9_ROW_MAXTAG); row++) {
> for (col = 0; col < P9_ROW_MAXTAG; col++) {
> - kfree(c->reqs[row][col].wq);
> kfree(c->reqs[row][col].tc);
> kfree(c->reqs[row][col].rc);
> }
> @@ -452,7 +443,7 @@ void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status)
> smp_wmb();
> req->status = status;
>
> - wake_up(req->wq);
> + wake_up(&req->wq);
> p9_debug(P9_DEBUG_MUX, "wakeup: %d\n", req->tc->tag);
> }
> EXPORT_SYMBOL(p9_client_cb);
> @@ -773,7 +764,7 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
> }
> again:
> /* Wait for the response */
> - err = wait_event_killable(*req->wq, req->status >= REQ_STATUS_RCVD);
> + err = wait_event_killable(req->wq, req->status >= REQ_STATUS_RCVD);
>
> /*
> * Make sure our req is coherent with regard to updates in other
> diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
> index 05006cbb3361..3e096c98313c 100644
> --- a/net/9p/trans_virtio.c
> +++ b/net/9p/trans_virtio.c
> @@ -490,7 +490,7 @@ p9_virtio_zc_request(struct p9_client *client, struct p9_req_t *req,
> virtqueue_kick(chan->vq);
> spin_unlock_irqrestore(&chan->lock, flags);
> p9_debug(P9_DEBUG_TRANS, "virtio request kicked\n");
> - err = wait_event_killable(*req->wq, req->status >= REQ_STATUS_RCVD);
> + err = wait_event_killable(req->wq, req->status >= REQ_STATUS_RCVD);
> /*
> * Non kernel buffers are pinned, unpin them
> */
next prev parent reply other threads:[~2018-07-12 14:36 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-11 21:02 [PATCH v2 0/6] 9p: Use IDRs more effectively Matthew Wilcox
2018-07-11 21:02 ` [PATCH v2 1/6] 9p: Fix comment on smp_wmb Matthew Wilcox
2018-07-12 11:55 ` [V9fs-developer] " Greg Kurz
2018-07-11 21:02 ` [PATCH v2 2/6] 9p: Change p9_fid_create calling convention Matthew Wilcox
2018-07-12 2:15 ` [V9fs-developer] " piaojun
2018-07-12 11:56 ` Greg Kurz
2018-07-13 1:18 ` jiangyiwen
2018-07-11 21:02 ` [PATCH v2 3/6] 9p: Replace the fidlist with an IDR Matthew Wilcox
2018-07-12 11:17 ` Dominique Martinet
2018-07-12 11:23 ` Matthew Wilcox
2018-07-12 11:30 ` Dominique Martinet
2018-07-13 2:05 ` [V9fs-developer] " jiangyiwen
2018-07-13 2:48 ` Matthew Wilcox
2018-07-11 21:02 ` [PATCH v2 4/6] 9p: Embed wait_queue_head into p9_req_t Matthew Wilcox
2018-07-12 14:36 ` Greg Kurz [this message]
2018-07-12 14:40 ` [V9fs-developer] " Dominique Martinet
2018-07-12 14:59 ` Greg Kurz
2018-07-11 21:02 ` [PATCH v2 5/6] 9p: Use a slab for allocating requests Matthew Wilcox
2018-07-18 10:05 ` Dominique Martinet
2018-07-18 11:49 ` Matthew Wilcox
2018-07-18 12:46 ` Dominique Martinet
2018-07-23 11:52 ` Greg Kurz
2018-07-23 12:25 ` Dominique Martinet
2018-07-23 14:24 ` Greg Kurz
2018-07-30 9:31 ` Dominique Martinet
2018-07-30 9:34 ` [PATCH 1/2] net/9p: embed fcall in req to round down buffer allocs Dominique Martinet
2018-07-30 9:34 ` [PATCH 2/2] net/9p: add a per-client fcall kmem_cache Dominique Martinet
2018-07-31 1:18 ` [V9fs-developer] " piaojun
2018-07-31 1:35 ` Dominique Martinet
2018-07-31 1:45 ` piaojun
2018-07-31 2:46 ` Matthew Wilcox
2018-07-31 4:17 ` Dominique Martinet
2018-08-01 14:28 ` [V9fs-developer] " Greg Kurz
2018-08-01 15:22 ` Dominique Martinet
2018-07-31 0:55 ` [V9fs-developer] [PATCH 1/2] net/9p: embed fcall in req to round down buffer allocs piaojun
2018-07-31 1:12 ` Dominique Martinet
2018-07-31 1:28 ` piaojun
2018-08-01 14:14 ` Greg Kurz
2018-08-01 14:38 ` Dominique Martinet
2018-08-01 15:03 ` Greg Kurz
2018-08-02 2:37 ` [PATCH v2 " Dominique Martinet
2018-08-02 2:37 ` [PATCH v2 2/2] net/9p: add a per-client fcall kmem_cache Dominique Martinet
2018-08-02 4:58 ` [V9fs-developer] " Dominique Martinet
2018-08-02 9:23 ` [PATCH v2 1/2] net/9p: embed fcall in req to round down buffer allocs Greg Kurz
2018-08-02 22:03 ` Dominique Martinet
2018-08-09 14:33 ` [PATCH v3 " Dominique Martinet
2018-08-09 14:33 ` [PATCH v3 2/2] net/9p: add a per-client fcall kmem_cache Dominique Martinet
2018-08-10 1:23 ` piaojun
2018-08-10 1:41 ` Dominique Martinet
2018-08-10 1:49 ` piaojun
2018-08-10 0:47 ` [PATCH v3 1/2] net/9p: embed fcall in req to round down buffer allocs piaojun
2018-07-11 21:02 ` [PATCH v2 6/6] 9p: Remove p9_idpool Matthew Wilcox
2018-07-11 23:37 ` [PATCH v2 0/6] 9p: Use IDRs more effectively Dominique Martinet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180712163649.36057b6e@bahia.lan \
--to=groug@kaod.org \
--cc=asmadeus@codewreck.org \
--cc=ericvh@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lucho@ionkov.net \
--cc=rminnich@sandia.gov \
--cc=v9fs-developer@lists.sourceforge.net \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).