smb: client: remove cached_fid->on_list

cfids are always on some list throughout its lifetime, so we can drop
this field.

Decrement cfids->num_entries now based on valid -> invalid transition.

Other changes:
- add helpers cfid_expired() and cfid_is_valid()
- check cfid_is_valid() even on success when leaving open_cached_dir()

Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>

2 files changed, 44 insertions, 31 deletions

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index af2fe416ddcf..a32d71540bba 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -15,20 +15,29 @@
 static struct cached_fid *init_cached_dir(const char *path);
 static void free_cached_dir(struct cached_fid *cfid);
 static void smb2_close_cached_fid(struct kref *ref);
-static void cfids_laundromat_worker(struct work_struct *work);
 
 struct cached_dir_dentry {
 	struct list_head entry;
 	struct dentry *dentry;
 };
 
+static inline bool cfid_expired(const struct cached_fid *cfid)
+{
+	return (cfid->last_access_time &&
+		time_is_before_jiffies(cfid->last_access_time + HZ * dir_cache_timeout));
+}
+
+static inline bool cfid_is_valid(const struct cached_fid *cfid)
+{
+	return (cfid->has_lease && cfid->time && !cfid_expired(cfid));
+}
+
 static inline void invalidate_cfid(struct cached_fid *cfid, bool force_expire)
 {
-	if (cfid->on_list)
+	if (cfid_is_valid(cfid))
 		cfid->cfids->num_entries--;
 
 	/* do not change other fields here! */
-	cfid->on_list = false;
 	cfid->time = 0;
 	if (force_expire)
 		cfid->last_access_time = 1;
@@ -48,9 +57,9 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
 			 * fully cached or it may be in the process of
 			 * being deleted due to a lease break.
 			 */
-			if (!cfid->time || !cfid->has_lease) {
+			if (!cfid_is_valid(cfid))
 				return NULL;
-			}
+
 			kref_get(&cfid->refcount);
 			return cfid;
 		}
@@ -68,7 +77,6 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
 	cfid->cfids = cfids;
 	cfids->num_entries++;
 	list_add(&cfid->entry, &cfids->entries);
-	cfid->on_list = true;
 	kref_get(&cfid->refcount);
 	/*
 	 * Set @cfid->has_lease to true during construction so that the lease
@@ -235,6 +243,7 @@ replay_again:
 	} else {
 		dentry = path_to_dentry(cifs_sb, npath);
 		if (IS_ERR(dentry)) {
+			dentry = NULL;
 			rc = -ENOENT;
 			goto out;
 		}
@@ -260,6 +269,7 @@ replay_again:
 	}
 	cfid->dentry = dentry;
 	cfid->tcon = tcon;
+	dentry = NULL;
 
 	/*
 	 * We do not hold the lock for the open because in case
@@ -387,24 +397,19 @@ oshr_free:
 	free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
 	free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
 out:
+	/* cfid invalidated in the mean time, drop it below */
+	if (!rc && !cfid_is_valid(cfid))
+		rc = -ENOENT;
+
 	if (rc) {
 		spin_lock(&cfids->cfid_list_lock);
-		if (cfid->on_list) {
-			list_del(&cfid->entry);
-			cfid->on_list = false;
-			cfids->num_entries--;
-		}
-		if (cfid->has_lease) {
-			/*
-			 * We are guaranteed to have two references at this
-			 * point. One for the caller and one for a potential
-			 * lease. Release one here, and the second below.
-			 */
-			cfid->has_lease = false;
-			kref_put(&cfid->refcount, smb2_close_cached_fid);
-		}
+		/* we don't know what caused the invalidation, so don't force cleanup */
+		invalidate_cfid(cfid, false);
 		spin_unlock(&cfids->cfid_list_lock);
 
+		if (!cfid->dentry)
+			dput(dentry);
+
 		kref_put(&cfid->refcount, smb2_close_cached_fid);
 	} else {
 		*ret_cfid = cfid;
@@ -431,7 +436,7 @@ int open_cached_dir_by_dentry(struct cifs_tcon *tcon,
 
 	spin_lock(&cfids->cfid_list_lock);
 	list_for_each_entry(cfid, &cfids->entries, entry) {
-		if (dentry && cfid->dentry == dentry) {
+		if (cfid_is_valid(cfid) && cfid->dentry == dentry) {
 			cifs_dbg(FYI, "found a cached file handle by dentry\n");
 			kref_get(&cfid->refcount);
 			*ret_cfid = cfid;
@@ -447,14 +452,19 @@ static void
 smb2_close_cached_fid(struct kref *ref)
 {
 	struct cached_fid *cfid = container_of(ref, struct cached_fid, refcount);
+	struct cached_fids *cfids = cfid->cfids;
+	struct dentry *dentry = NULL;
 
-	spin_lock(&cfid->cfids->cfid_list_lock);
+	spin_lock(&cfids->cfid_list_lock);
 	list_del(&cfid->entry);
 	invalidate_cfid(cfid, true);
-	spin_unlock(&cfid->cfids->cfid_list_lock);
-
-	dput(cfid->dentry);
+	spin_lock(&cfid->fid_lock);
+	dentry = cfid->dentry;
 	cfid->dentry = NULL;
+	spin_unlock(&cfid->fid_lock);
+	spin_unlock(&cfids->cfid_list_lock);
+
+	dput(dentry);
 
 	free_cached_dir(cfid);
 }
@@ -552,14 +562,14 @@ done:
 void invalidate_all_cached_dirs(struct cifs_tcon *tcon)
 {
 	struct cached_fids *cfids = tcon->cfids;
-	struct cached_fid *cfid, *q;
+	struct cached_fid *cfid;
 
 	if (!cfids)
 		return;
 
 	/* Mark all the cfids as closed, and invalidate them for laundromat cleanup. */
 	spin_lock(&cfids->cfid_list_lock);
-	list_for_each_entry_safe(cfid, q, &cfids->entries, entry) {
+	list_for_each_entry(cfid, &cfids->entries, entry) {
 		invalidate_cfid(cfid, true);
 		cfid->is_open = false;
 	}
@@ -628,9 +638,14 @@ static struct cached_fid *init_cached_dir(const char *path)
 static void free_cached_dir(struct cached_fid *cfid)
 {
 	struct cached_dirent *dirent, *q;
+	struct dentry *dentry;
 
-	dput(cfid->dentry);
+	spin_lock(&cfid->fid_lock);
+	dentry = cfid->dentry;
 	cfid->dentry = NULL;
+	spin_unlock(&cfid->fid_lock);
+
+	dput(dentry);
 
 	/*
 	 * Delete all cached dirent names
@@ -657,8 +672,7 @@ static void cfids_laundromat_worker(struct work_struct *work)
 
 	spin_lock(&cfids->cfid_list_lock);
 	list_for_each_entry_safe(cfid, q, &cfids->entries, entry) {
-		if (cfid->last_access_time &&
-		    time_after(jiffies, cfid->last_access_time + HZ * dir_cache_timeout)) {
+		if (cfid_expired(cfid)) {
 			invalidate_cfid(cfid, true);
 			list_move(&cfid->entry, &entry);
 		}
diff --git a/fs/smb/client/cached_dir.h b/fs/smb/client/cached_dir.h
index e549f019b923..feca9a361635 100644
--- a/fs/smb/client/cached_dir.h
+++ b/fs/smb/client/cached_dir.h
@@ -35,7 +35,6 @@ struct cached_fid {
 	const char *path;
 	bool has_lease:1;
 	bool is_open:1;
-	bool on_list:1;
 	bool file_all_info_is_valid:1;
 	unsigned long time; /* jiffies of when lease was taken */
 	unsigned long last_access_time; /* jiffies of when last accessed */