From: jiangyiwen <jiangyiwen@huawei.com>
To: Matthew Wilcox <willy@infradead.org>,
Dominique Martinet <asmadeus@codewreck.org>
Cc: 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 3/6] 9p: Replace the fidlist with an IDR
Date: Fri, 13 Jul 2018 10:05:50 +0800 [thread overview]
Message-ID: <5B4808FE.7010500@huawei.com> (raw)
In-Reply-To: <20180711210225.19730-4-willy@infradead.org>
On 2018/7/12 5:02, Matthew Wilcox wrote:
> The p9_idpool being used to allocate the IDs uses an IDR to allocate
> the IDs ... which we then keep in a doubly-linked list, rather than in
> the IDR which allocated them. We can use an IDR directly which saves
> two pointers per p9_fid, and a tiny memory allocation per p9_client.
>
> Signed-off-by: Matthew Wilcox <willy@infradead.org>
> ---
> include/net/9p/client.h | 9 +++------
> net/9p/client.c | 44 +++++++++++++++--------------------------
> 2 files changed, 19 insertions(+), 34 deletions(-)
>
> diff --git a/include/net/9p/client.h b/include/net/9p/client.h
> index 7af9d769b97d..e405729cd1c7 100644
> --- a/include/net/9p/client.h
> +++ b/include/net/9p/client.h
> @@ -27,6 +27,7 @@
> #define NET_9P_CLIENT_H
>
> #include <linux/utsname.h>
> +#include <linux/idr.h>
>
> /* Number of requests per row */
> #define P9_ROW_MAXTAG 255
> @@ -128,8 +129,7 @@ struct p9_req_t {
> * @proto_version: 9P protocol version to use
> * @trans_mod: module API instantiated with this client
> * @trans: tranport instance state and API
> - * @fidpool: fid handle accounting for session
> - * @fidlist: List of active fid handles
> + * @fids: All active FID handles
> * @tagpool - transaction id accounting for session
> * @reqs - 2D array of requests
> * @max_tag - current maximum tag id allocated
> @@ -169,8 +169,7 @@ struct p9_client {
> } tcp;
> } trans_opts;
>
> - struct p9_idpool *fidpool;
> - struct list_head fidlist;
> + struct idr fids;
>
> struct p9_idpool *tagpool;
> struct p9_req_t *reqs[P9_ROW_MAXTAG];
> @@ -188,7 +187,6 @@ struct p9_client {
> * @iounit: the server reported maximum transaction size for this file
> * @uid: the numeric uid of the local user who owns this handle
> * @rdir: readdir accounting structure (allocated on demand)
> - * @flist: per-client-instance fid tracking
> * @dlist: per-dentry fid tracking
> *
> * TODO: This needs lots of explanation.
> @@ -204,7 +202,6 @@ struct p9_fid {
>
> void *rdir;
>
> - struct list_head flist;
> struct hlist_node dlist; /* list of all fids attached to a dentry */
> };
>
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 389a2904b7b3..b89c7298267c 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -908,30 +908,29 @@ static struct p9_fid *p9_fid_create(struct p9_client *clnt)
> {
> int ret;
> struct p9_fid *fid;
> - unsigned long flags;
>
> p9_debug(P9_DEBUG_FID, "clnt %p\n", clnt);
> fid = kmalloc(sizeof(struct p9_fid), GFP_KERNEL);
> if (!fid)
> return NULL;
>
> - ret = p9_idpool_get(clnt->fidpool);
> - if (ret < 0)
> - goto error;
> - fid->fid = ret;
> -
> memset(&fid->qid, 0, sizeof(struct p9_qid));
> fid->mode = -1;
> fid->uid = current_fsuid();
> fid->clnt = clnt;
> fid->rdir = NULL;
> - spin_lock_irqsave(&clnt->lock, flags);
> - list_add(&fid->flist, &clnt->fidlist);
> - spin_unlock_irqrestore(&clnt->lock, flags);
> + fid->fid = 0;
>
> - return fid;
> + idr_preload(GFP_KERNEL);
It is best to use GFP_NOFS instead, or else it may cause some
unpredictable problem, because when out of memory it will
reclaim memory from v9fs.
> + spin_lock_irq(&clnt->lock);
> + ret = idr_alloc_u32(&clnt->fids, fid, &fid->fid, P9_NOFID - 1,
> + GFP_NOWAIT);
> + spin_unlock_irq(&clnt->lock);
use spin_lock instead, clnt->lock is not used in irq context.
> + idr_preload_end();
> +
> + if (!ret)
> + return fid;
>
> -error:
> kfree(fid);
> return NULL;
> }
> @@ -943,9 +942,8 @@ static void p9_fid_destroy(struct p9_fid *fid)
>
> p9_debug(P9_DEBUG_FID, "fid %d\n", fid->fid);
> clnt = fid->clnt;
> - p9_idpool_put(fid->fid, clnt->fidpool);
> spin_lock_irqsave(&clnt->lock, flags);
> - list_del(&fid->flist);
> + idr_remove(&clnt->fids, fid->fid);
> spin_unlock_irqrestore(&clnt->lock, flags);
> kfree(fid->rdir);
> kfree(fid);
> @@ -1028,7 +1026,7 @@ struct p9_client *p9_client_create(const char *dev_name, char *options)
> memcpy(clnt->name, client_id, strlen(client_id) + 1);
>
> spin_lock_init(&clnt->lock);
> - INIT_LIST_HEAD(&clnt->fidlist);
> + idr_init(&clnt->fids);
>
> err = p9_tag_init(clnt);
> if (err < 0)
> @@ -1048,18 +1046,12 @@ struct p9_client *p9_client_create(const char *dev_name, char *options)
> goto destroy_tagpool;
> }
>
> - clnt->fidpool = p9_idpool_create();
> - if (IS_ERR(clnt->fidpool)) {
> - err = PTR_ERR(clnt->fidpool);
> - goto put_trans;
> - }
> -
> p9_debug(P9_DEBUG_MUX, "clnt %p trans %p msize %d protocol %d\n",
> clnt, clnt->trans_mod, clnt->msize, clnt->proto_version);
>
> err = clnt->trans_mod->create(clnt, dev_name, options);
> if (err)
> - goto destroy_fidpool;
> + goto put_trans;
>
> if (clnt->msize > clnt->trans_mod->maxsize)
> clnt->msize = clnt->trans_mod->maxsize;
> @@ -1072,8 +1064,6 @@ struct p9_client *p9_client_create(const char *dev_name, char *options)
>
> close_trans:
> clnt->trans_mod->close(clnt);
> -destroy_fidpool:
> - p9_idpool_destroy(clnt->fidpool);
> put_trans:
> v9fs_put_trans(clnt->trans_mod);
> destroy_tagpool:
> @@ -1086,7 +1076,8 @@ EXPORT_SYMBOL(p9_client_create);
>
> void p9_client_destroy(struct p9_client *clnt)
> {
> - struct p9_fid *fid, *fidptr;
> + struct p9_fid *fid;
> + int id;
>
> p9_debug(P9_DEBUG_MUX, "clnt %p\n", clnt);
>
> @@ -1095,14 +1086,11 @@ void p9_client_destroy(struct p9_client *clnt)
>
> v9fs_put_trans(clnt->trans_mod);
>
> - list_for_each_entry_safe(fid, fidptr, &clnt->fidlist, flist) {
> + idr_for_each_entry(&clnt->fids, fid, id) {
> pr_info("Found fid %d not clunked\n", fid->fid);
> p9_fid_destroy(fid);
> }
>
> - if (clnt->fidpool)
> - p9_idpool_destroy(clnt->fidpool);
> -
I suggest add idr_destroy in the end.
> p9_tag_cleanup(clnt);
>
> kfree(clnt);
>
next prev parent reply other threads:[~2018-07-13 2:18 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 ` jiangyiwen [this message]
2018-07-13 2:48 ` [V9fs-developer] " 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 ` [V9fs-developer] " Greg Kurz
2018-07-12 14:40 ` 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=5B4808FE.7010500@huawei.com \
--to=jiangyiwen@huawei.com \
--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).