simplify lookup_open()/atomic_open() - do the temporary mnt_want_write() early

The write ref to vfsmount taken in lookup_open()/atomic_open() is going to be dropped; we take the one to stay in dentry_open(). Just grab the temporary in caller if it looks like we are going to need it (create/truncate/writable open) and pass (by value) "has it succeeded" flag. Instead of doing mnt_want_write() inside, check that flag and treat "false" as "mnt_want_write() has just failed". mnt_want_write() is cheap and the things get considerably simpler and more robust that way - we get it and drop it in the same function, to start with, rather than passing a "has something in the guts of really scary functions taken it" back to caller. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

simplify lookup_open()/atomic_open() - do the temporary mnt_want_write() early
The write ref to vfsmount taken in lookup_open()/atomic_open() is going to be dropped; we take the one to stay in dentry_open(). Just grab the temporary in caller if it looks like we are going to need it (create/truncate/writable open) and pass (by value) "has it succeeded" flag. Instead of doing mnt_want_write() inside, check that flag and treat "false" as "mnt_want_write() has just failed". mnt_want_write() is cheap and the things get considerably simpler and more robust that way - we get it and drop it in the same function, to start with, rather than passing a "has something in the guts of really scary functions taken it" back to caller. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Al Viro
1 parent f8310c5920
Showing 1 changed file with 29 additions and 22 deletions Side-by-side Diff
fs/namei.c
@@ -2395,7 +2395,7 @@
 static int atomic_open(struct nameidata *nd, struct dentry *dentry,
 			struct path *path, struct file *file,
 			const struct open_flags *op,
-			bool *want_write, bool need_lookup,
+			bool got_write, bool need_lookup,
 			int *opened)
 {
 	struct inode *dir =  nd->path.dentry->d_inode;
@@ -2432,12 +2432,9 @@
 	 * Another problem is returing the "right" error value (e.g. for an
 	 * O_EXCL open we want to return EEXIST not EROFS).
 	 */
-	if ((open_flag & (O_CREAT | O_TRUNC)) ||
-	    (open_flag & O_ACCMODE) != O_RDONLY) {
-		error = mnt_want_write(nd->path.mnt);
-		if (!error) {
-			*want_write = true;
-		} else if (!(open_flag & O_CREAT)) {
+	if (((open_flag & (O_CREAT | O_TRUNC)) ||
+	    (open_flag & O_ACCMODE) != O_RDONLY) && unlikely(!got_write)) {
+		if (!(open_flag & O_CREAT)) {
 			/*
 			 * No O_CREATE -> atomicity not a requirement -> fall
 			 * back to lookup + open
  
@@ -2445,11 +2442,11 @@
 			goto no_open;
 		} else if (open_flag & (O_EXCL | O_TRUNC)) {
 			/* Fall back and fail with the right error */
-			create_error = error;
+			create_error = -EROFS;
 			goto no_open;
 		} else {
 			/* No side effects, safe to clear O_CREAT */
-			create_error = error;
+			create_error = -EROFS;
 			open_flag &= ~O_CREAT;
 		}
 	}
@@ -2556,7 +2553,7 @@
 static int lookup_open(struct nameidata *nd, struct path *path,
 			struct file *file,
 			const struct open_flags *op,
-			bool *want_write, int *opened)
+			bool got_write, int *opened)
 {
 	struct dentry *dir = nd->path.dentry;
 	struct inode *dir_inode = dir->d_inode;
@@ -2574,7 +2571,7 @@
 		goto out_no_open;
  
 	if ((nd->flags & LOOKUP_OPEN) && dir_inode->i_op->atomic_open) {
-		return atomic_open(nd, dentry, path, file, op, want_write,
+		return atomic_open(nd, dentry, path, file, op, got_write,
 				   need_lookup, opened);
 	}
  
  
@@ -2598,10 +2595,10 @@
 		 * a permanent write count is taken through
 		 * the 'struct file' in finish_open().
 		 */
-		error = mnt_want_write(nd->path.mnt);
-		if (error)
+		if (!got_write) {
+			error = -EROFS;
 			goto out_dput;
-		*want_write = true;
+		}
 		*opened |= FILE_CREATED;
 		error = security_path_mknod(&nd->path, dentry, mode, 0);
 		if (error)
@@ -2631,7 +2628,7 @@
 	struct dentry *dir = nd->path.dentry;
 	int open_flag = op->open_flag;
 	bool will_truncate = (open_flag & O_TRUNC) != 0;
-	bool want_write = false;
+	bool got_write = false;
 	int acc_mode = op->acc_mode;
 	struct inode *inode;
 	bool symlink_ok = false;
  
@@ -2700,8 +2697,18 @@
 	}
  
 retry_lookup:
+	if (op->open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) {
+		error = mnt_want_write(nd->path.mnt);
+		if (!error)
+			got_write = true;
+		/*
+		 * do _not_ fail yet - we might not need that or fail with
+		 * a different error; let lookup_open() decide; we'll be
+		 * dropping this one anyway.
+		 */
+	}
 	mutex_lock(&dir->d_inode->i_mutex);
-	error = lookup_open(nd, path, file, op, &want_write, opened);
+	error = lookup_open(nd, path, file, op, got_write, opened);
 	mutex_unlock(&dir->d_inode->i_mutex);
  
 	if (error <= 0) {
  
@@ -2736,9 +2743,9 @@
 	 * possible mount and symlink following (this might be optimized away if
 	 * necessary...)
 	 */
-	if (want_write) {
+	if (got_write) {
 		mnt_drop_write(nd->path.mnt);
-		want_write = false;
+		got_write = false;
 	}
  
 	error = -EEXIST;
@@ -2803,7 +2810,7 @@
 		error = mnt_want_write(nd->path.mnt);
 		if (error)
 			goto out;
-		want_write = true;
+		got_write = true;
 	}
 finish_open_created:
 	error = may_open(&nd->path, acc_mode, open_flag);
@@ -2830,7 +2837,7 @@
 			goto exit_fput;
 	}
 out:
-	if (want_write)
+	if (got_write)
 		mnt_drop_write(nd->path.mnt);
 	path_put(&save_parent);
 	terminate_walk(nd);
  
@@ -2854,9 +2861,9 @@
 	nd->inode = dir->d_inode;
 	save_parent.mnt = NULL;
 	save_parent.dentry = NULL;
-	if (want_write) {
+	if (got_write) {
 		mnt_drop_write(nd->path.mnt);
-		want_write = false;
+		got_write = false;
 	}
 	retried = true;
 	goto retry_lookup;
...	...	@@ -2395,7 +2395,7 @@
2395	2395	static int atomic_open(struct nameidata nd, struct dentry dentry,
2396	2396	struct path path, struct file file,
2397	2397	const struct open_flags *op,
2398		- bool *want_write, bool need_lookup,
	2398	+ bool got_write, bool need_lookup,
2399	2399	int *opened)
2400	2400	{
2401	2401	struct inode *dir = nd->path.dentry->d_inode;
...	...	@@ -2432,12 +2432,9 @@
2432	2432	* Another problem is returing the "right" error value (e.g. for an
2433	2433	* O_EXCL open we want to return EEXIST not EROFS).
2434	2434	*/
2435		- if ((open_flag & (O_CREAT \| O_TRUNC)) \|\|
2436		- (open_flag & O_ACCMODE) != O_RDONLY) {
2437		- error = mnt_want_write(nd->path.mnt);
2438		- if (!error) {
2439		- *want_write = true;
2440		- } else if (!(open_flag & O_CREAT)) {
	2435	+ if (((open_flag & (O_CREAT \| O_TRUNC)) \|\|
	2436	+ (open_flag & O_ACCMODE) != O_RDONLY) && unlikely(!got_write)) {
	2437	+ if (!(open_flag & O_CREAT)) {
2441	2438	/*
2442	2439	* No O_CREATE -> atomicity not a requirement -> fall
2443	2440	* back to lookup + open
2444	2441
...	...	@@ -2445,11 +2442,11 @@
2445	2442	goto no_open;
2446	2443	} else if (open_flag & (O_EXCL \| O_TRUNC)) {
2447	2444	/* Fall back and fail with the right error */
2448		- create_error = error;
	2445	+ create_error = -EROFS;
2449	2446	goto no_open;
2450	2447	} else {
2451	2448	/* No side effects, safe to clear O_CREAT */
2452		- create_error = error;
	2449	+ create_error = -EROFS;
2453	2450	open_flag &= ~O_CREAT;
2454	2451	}
2455	2452	}
...	...	@@ -2556,7 +2553,7 @@
2556	2553	static int lookup_open(struct nameidata nd, struct path path,
2557	2554	struct file *file,
2558	2555	const struct open_flags *op,
2559		- bool want_write, int opened)
	2556	+ bool got_write, int *opened)
2560	2557	{
2561	2558	struct dentry *dir = nd->path.dentry;
2562	2559	struct inode *dir_inode = dir->d_inode;
...	...	@@ -2574,7 +2571,7 @@
2574	2571	goto out_no_open;
2575	2572
2576	2573	if ((nd->flags & LOOKUP_OPEN) && dir_inode->i_op->atomic_open) {
2577		- return atomic_open(nd, dentry, path, file, op, want_write,
	2574	+ return atomic_open(nd, dentry, path, file, op, got_write,
2578	2575	need_lookup, opened);
2579	2576	}
2580	2577
2581	2578
...	...	@@ -2598,10 +2595,10 @@
2598	2595	* a permanent write count is taken through
2599	2596	* the 'struct file' in finish_open().
2600	2597	*/
2601		- error = mnt_want_write(nd->path.mnt);
2602		- if (error)
	2598	+ if (!got_write) {
	2599	+ error = -EROFS;
2603	2600	goto out_dput;
2604		- *want_write = true;
	2601	+ }
2605	2602	*opened \|= FILE_CREATED;
2606	2603	error = security_path_mknod(&nd->path, dentry, mode, 0);
2607	2604	if (error)
...	...	@@ -2631,7 +2628,7 @@
2631	2628	struct dentry *dir = nd->path.dentry;
2632	2629	int open_flag = op->open_flag;
2633	2630	bool will_truncate = (open_flag & O_TRUNC) != 0;
2634		- bool want_write = false;
	2631	+ bool got_write = false;
2635	2632	int acc_mode = op->acc_mode;
2636	2633	struct inode *inode;
2637	2634	bool symlink_ok = false;
2638	2635
...	...	@@ -2700,8 +2697,18 @@
2700	2697	}
2701	2698
2702	2699	retry_lookup:
	2700	+ if (op->open_flag & (O_CREAT \| O_TRUNC \| O_WRONLY \| O_RDWR)) {
	2701	+ error = mnt_want_write(nd->path.mnt);
	2702	+ if (!error)
	2703	+ got_write = true;
	2704	+ /*
	2705	+ * do _not_ fail yet - we might not need that or fail with
	2706	+ * a different error; let lookup_open() decide; we'll be
	2707	+ * dropping this one anyway.
	2708	+ */
	2709	+ }
2703	2710	mutex_lock(&dir->d_inode->i_mutex);
2704		- error = lookup_open(nd, path, file, op, &want_write, opened);
	2711	+ error = lookup_open(nd, path, file, op, got_write, opened);
2705	2712	mutex_unlock(&dir->d_inode->i_mutex);
2706	2713
2707	2714	if (error <= 0) {
2708	2715
...	...	@@ -2736,9 +2743,9 @@
2736	2743	* possible mount and symlink following (this might be optimized away if
2737	2744	* necessary...)
2738	2745	*/
2739		- if (want_write) {
	2746	+ if (got_write) {
2740	2747	mnt_drop_write(nd->path.mnt);
2741		- want_write = false;
	2748	+ got_write = false;
2742	2749	}
2743	2750
2744	2751	error = -EEXIST;
...	...	@@ -2803,7 +2810,7 @@
2803	2810	error = mnt_want_write(nd->path.mnt);
2804	2811	if (error)
2805	2812	goto out;
2806		- want_write = true;
	2813	+ got_write = true;
2807	2814	}
2808	2815	finish_open_created:
2809	2816	error = may_open(&nd->path, acc_mode, open_flag);
...	...	@@ -2830,7 +2837,7 @@
2830	2837	goto exit_fput;
2831	2838	}
2832	2839	out:
2833		- if (want_write)
	2840	+ if (got_write)
2834	2841	mnt_drop_write(nd->path.mnt);
2835	2842	path_put(&save_parent);
2836	2843	terminate_walk(nd);
2837	2844
...	...	@@ -2854,9 +2861,9 @@
2854	2861	nd->inode = dir->d_inode;
2855	2862	save_parent.mnt = NULL;
2856	2863	save_parent.dentry = NULL;
2857		- if (want_write) {
	2864	+ if (got_write) {
2858	2865	mnt_drop_write(nd->path.mnt);
2859		- want_write = false;
	2866	+ got_write = false;
2860	2867	}
2861	2868	retried = true;
2862	2869	goto retry_lookup;