From: Paulo Alcantara <pc@cjr.nz>
To: smfrench@gmail.com
Cc: linux-cifs@vger.kernel.org, Paulo Alcantara <pc@cjr.nz>
Subject: [PATCH 2/5] cifs: avoid re-lookups in dfs_cache_find()
Date: Mon, 16 Jan 2023 21:09:49 -0300 [thread overview]
Message-ID: <20230117000952.9965-3-pc@cjr.nz> (raw)
In-Reply-To: <20230117000952.9965-1-pc@cjr.nz>
Simply downgrade the write lock on cache updates from
cache_refresh_path() and avoid unnecessary re-lookup in
dfs_cache_find().
Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
---
fs/cifs/dfs_cache.c | 57 ++++++++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 24 deletions(-)
diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c
index a8ddac1c054c..c82721b3277c 100644
--- a/fs/cifs/dfs_cache.c
+++ b/fs/cifs/dfs_cache.c
@@ -558,7 +558,8 @@ static void remove_oldest_entry_locked(void)
}
/* Add a new DFS cache entry */
-static int add_cache_entry_locked(struct dfs_info3_param *refs, int numrefs)
+static struct cache_entry *add_cache_entry_locked(struct dfs_info3_param *refs,
+ int numrefs)
{
int rc;
struct cache_entry *ce;
@@ -573,11 +574,11 @@ static int add_cache_entry_locked(struct dfs_info3_param *refs, int numrefs)
rc = cache_entry_hash(refs[0].path_name, strlen(refs[0].path_name), &hash);
if (rc)
- return rc;
+ return ERR_PTR(rc);
ce = alloc_cache_entry(refs, numrefs);
if (IS_ERR(ce))
- return PTR_ERR(ce);
+ return ce;
spin_lock(&cache_ttl_lock);
if (!cache_ttl) {
@@ -594,7 +595,7 @@ static int add_cache_entry_locked(struct dfs_info3_param *refs, int numrefs)
atomic_inc(&cache_count);
- return 0;
+ return ce;
}
/* Check if two DFS paths are equal. @s1 and @s2 are expected to be in @cache_cp's charset */
@@ -767,8 +768,12 @@ static int get_dfs_referral(const unsigned int xid, struct cifs_ses *ses, const
*
* For interlinks, cifs_mount() and expand_dfs_referral() are supposed to
* handle them properly.
+ *
+ * On success, return entry with acquired lock for reading, otherwise error ptr.
*/
-static int cache_refresh_path(const unsigned int xid, struct cifs_ses *ses, const char *path)
+static struct cache_entry *cache_refresh_path(const unsigned int xid,
+ struct cifs_ses *ses,
+ const char *path)
{
struct dfs_info3_param *refs = NULL;
struct cache_entry *ce;
@@ -780,10 +785,8 @@ static int cache_refresh_path(const unsigned int xid, struct cifs_ses *ses, cons
down_read(&htable_rw_lock);
ce = lookup_cache_entry(path);
- if (!IS_ERR(ce) && !cache_entry_expired(ce)) {
- up_read(&htable_rw_lock);
- return 0;
- }
+ if (!IS_ERR(ce) && !cache_entry_expired(ce))
+ return ce;
up_read(&htable_rw_lock);
@@ -792,8 +795,10 @@ static int cache_refresh_path(const unsigned int xid, struct cifs_ses *ses, cons
* Request a new DFS referral in order to create or update a cache entry.
*/
rc = get_dfs_referral(xid, ses, path, &refs, &numrefs);
- if (rc)
+ if (rc) {
+ ce = ERR_PTR(rc);
goto out;
+ }
dump_refs(refs, numrefs);
@@ -801,16 +806,24 @@ static int cache_refresh_path(const unsigned int xid, struct cifs_ses *ses, cons
/* Re-check as another task might have it added or refreshed already */
ce = lookup_cache_entry(path);
if (!IS_ERR(ce)) {
- if (cache_entry_expired(ce))
+ if (cache_entry_expired(ce)) {
rc = update_cache_entry_locked(ce, refs, numrefs);
+ if (rc)
+ ce = ERR_PTR(rc);
+ }
} else {
- rc = add_cache_entry_locked(refs, numrefs);
+ ce = add_cache_entry_locked(refs, numrefs);
}
- up_write(&htable_rw_lock);
+ if (IS_ERR(ce)) {
+ up_write(&htable_rw_lock);
+ goto out;
+ }
+
+ downgrade_write(&htable_rw_lock);
out:
free_dfs_info_array(refs, numrefs);
- return rc;
+ return ce;
}
/*
@@ -930,15 +943,8 @@ int dfs_cache_find(const unsigned int xid, struct cifs_ses *ses, const struct nl
if (IS_ERR(npath))
return PTR_ERR(npath);
- rc = cache_refresh_path(xid, ses, npath);
- if (rc)
- goto out_free_path;
-
- down_read(&htable_rw_lock);
-
- ce = lookup_cache_entry(npath);
+ ce = cache_refresh_path(xid, ses, npath);
if (IS_ERR(ce)) {
- up_read(&htable_rw_lock);
rc = PTR_ERR(ce);
goto out_free_path;
}
@@ -1034,10 +1040,13 @@ int dfs_cache_update_tgthint(const unsigned int xid, struct cifs_ses *ses,
cifs_dbg(FYI, "%s: update target hint - path: %s\n", __func__, npath);
- rc = cache_refresh_path(xid, ses, npath);
- if (rc)
+ ce = cache_refresh_path(xid, ses, npath);
+ if (IS_ERR(ce)) {
+ rc = PTR_ERR(ce);
goto out_free_path;
+ }
+ up_read(&htable_rw_lock);
down_write(&htable_rw_lock);
ce = lookup_cache_entry(npath);
--
2.39.0
next prev parent reply other threads:[~2023-01-17 0:11 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-17 0:09 [PATCH 0/5] dfs fixes Paulo Alcantara
2023-01-17 0:09 ` [PATCH 1/5] cifs: fix potential deadlock in cache_refresh_path() Paulo Alcantara
2023-01-17 17:07 ` Aurélien Aptel
2023-01-17 18:03 ` Paulo Alcantara
2023-01-17 0:09 ` Paulo Alcantara [this message]
2023-01-17 0:09 ` [PATCH 3/5] cifs: don't take exclusive lock for updating target hints Paulo Alcantara
2023-01-17 0:09 ` [PATCH 4/5] cifs: remove duplicate code in __refresh_tcon() Paulo Alcantara
2023-01-17 0:09 ` [PATCH 5/5] cifs: handle cache lookup errors different than -ENOENT Paulo Alcantara
2023-01-17 22:00 ` [PATCH v2 0/5] dfs fixes Paulo Alcantara
2023-01-17 22:00 ` [PATCH v2 1/5] cifs: fix potential deadlock in cache_refresh_path() Paulo Alcantara
2023-01-17 22:00 ` [PATCH v2 2/5] cifs: avoid re-lookups in dfs_cache_find() Paulo Alcantara
2023-01-17 22:00 ` [PATCH v2 3/5] cifs: don't take exclusive lock for updating target hints Paulo Alcantara
2023-01-17 22:00 ` [PATCH v2 4/5] cifs: remove duplicate code in __refresh_tcon() Paulo Alcantara
2023-01-17 22:00 ` [PATCH v2 5/5] cifs: handle cache lookup errors different than -ENOENT Paulo Alcantara
2023-01-18 1:33 ` [PATCH v2 0/5] dfs fixes Steve French
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=20230117000952.9965-3-pc@cjr.nz \
--to=pc@cjr.nz \
--cc=linux-cifs@vger.kernel.org \
--cc=smfrench@gmail.com \
/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