Unverified Commit 3e327154 authored by Linus Torvalds's avatar Linus Torvalds Committed by Christian Brauner
Browse files

vfs: get rid of old '->iterate' directory operation 



All users now just use '->iterate_shared()', which only takes the
directory inode lock for reading.

Filesystems that never got convered to shared mode now instead use a
wrapper that drops the lock, re-takes it in write mode, calls the old
function, and then downgrades the lock back to read mode.

This way the VFS layer and other callers no longer need to care about
filesystems that never got converted to the modern era.

The filesystems that use the new wrapper are ceph, coda, exfat, jfs,
ntfs, ocfs2, overlayfs, and vboxsf.

Honestly, several of them look like they really could just iterate their
directories in shared mode and skip the wrapper entirely, but the point
of this change is to not change semantics or fix filesystems that
haven't been fixed in the last 7+ years, but to finally get rid of the
dual iterators.

Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent 0a2c2baa

Documentation/filesystems/locking.rst

+2 −3
Original line number Diff line number Diff line
@@ -551,9 +551,8 @@ mutex or just to use i_size_read() instead. 
Note: this does not protect the file->f_pos against concurrent modifications

since this is something the userspace has to take care about.



->iterate() is called with i_rwsem exclusive.



->iterate_shared() is called with i_rwsem at least shared.

->iterate_shared() is called with i_rwsem held for reading, and with the

file f_pos_lock held exclusively



->fasync() is responsible for maintaining the FASYNC bit in filp->f_flags.

Most instances call fasync_helper(), which does that maintenance, so it's

Documentation/filesystems/porting.rst

+10 −15
Original line number Diff line number Diff line
@@ -537,7 +537,7 @@ vfs_readdir() is gone; switch to iterate_dir() instead 


**mandatory**



->readdir() is gone now; switch to ->iterate()

->readdir() is gone now; switch to ->iterate_shared()



**mandatory**
@@ -693,24 +693,19 @@ parallel now. 


---



**recommended**

**mandatory**



->iterate_shared() is added; it's a parallel variant of ->iterate().

->iterate_shared() is added.

Exclusion on struct file level is still provided (as well as that

between it and lseek on the same struct file), but if your directory

has been opened several times, you can get these called in parallel.

Exclusion between that method and all directory-modifying ones is

still provided, of course.



Often enough ->iterate() can serve as ->iterate_shared() without any

changes - it is a read-only operation, after all.  If you have any

per-inode or per-dentry in-core data structures modified by ->iterate(),

you might need something to serialize the access to them.  If you

do dcache pre-seeding, you'll need to switch to d_alloc_parallel() for

that; look for in-tree examples.



Old method is only used if the new one is absent; eventually it will

be removed.  Switch while you still can; the old one won't stay.

If you have any per-inode or per-dentry in-core data structures modified

by ->iterate_shared(), you might need something to serialize the access

to them.  If you do dcache pre-seeding, you'll need to switch to

d_alloc_parallel() for that; look for in-tree examples.



---
@@ -930,9 +925,9 @@ should be done by looking at FMODE_LSEEK in file->f_mode. 
filldir_t (readdir callbacks) calling conventions have changed.  Instead of

returning 0 or -E... it returns bool now.  false means "no more" (as -E... used

to) and true - "keep going" (as 0 in old calling conventions).  Rationale:

callers never looked at specific -E... values anyway.  ->iterate() and

->iterate_shared() instance require no changes at all, all filldir_t ones in

the tree converted.

callers never looked at specific -E... values anyway. -> iterate_shared()

instances require no changes at all, all filldir_t ones in the tree

converted.



---

fs/ceph/dir.c

+3 −2
Original line number Diff line number Diff line
@@ -2019,9 +2019,10 @@ unsigned ceph_dentry_hash(struct inode *dir, struct dentry *dn) 
	}

}



WRAP_DIR_ITER(ceph_readdir) // FIXME!

const struct file_operations ceph_dir_fops = {

	.read = ceph_read_dir,

	.iterate = ceph_readdir,

	.iterate_shared = shared_ceph_readdir,

	.llseek = ceph_dir_llseek,

	.open = ceph_open,

	.release = ceph_release,
@@ -2033,7 +2034,7 @@ const struct file_operations ceph_dir_fops = { 
};



const struct file_operations ceph_snapdir_fops = {

	.iterate = ceph_readdir,

	.iterate_shared = shared_ceph_readdir,

	.llseek = ceph_dir_llseek,

	.open = ceph_open,

	.release = ceph_release,

fs/coda/dir.c

+7 −13
Original line number Diff line number Diff line
@@ -429,21 +429,14 @@ static int coda_readdir(struct file *coda_file, struct dir_context *ctx) 
	cfi = coda_ftoc(coda_file);

	host_file = cfi->cfi_container;



	if (host_file->f_op->iterate || host_file->f_op->iterate_shared) {

	if (host_file->f_op->iterate_shared) {

		struct inode *host_inode = file_inode(host_file);

		ret = -ENOENT;

		if (!IS_DEADDIR(host_inode)) {

			if (host_file->f_op->iterate_shared) {

			inode_lock_shared(host_inode);

			ret = host_file->f_op->iterate_shared(host_file, ctx);

			file_accessed(host_file);

			inode_unlock_shared(host_inode);

			} else {

				inode_lock(host_inode);

				ret = host_file->f_op->iterate(host_file, ctx);

				file_accessed(host_file);

				inode_unlock(host_inode);

			}

		}

		return ret;

	}
@@ -585,10 +578,11 @@ const struct inode_operations coda_dir_inode_operations = { 
	.setattr	= coda_setattr,

};



WRAP_DIR_ITER(coda_readdir) // FIXME!

const struct file_operations coda_dir_operations = {

	.llseek		= generic_file_llseek,

	.read		= generic_read_dir,

	.iterate	= coda_readdir,

	.iterate_shared	= shared_coda_readdir,

	.open		= coda_open,

	.release	= coda_release,

	.fsync		= coda_fsync,

fs/exfat/dir.c

+2 −1
Original line number Diff line number Diff line
@@ -306,10 +306,11 @@ static int exfat_iterate(struct file *file, struct dir_context *ctx) 
	return err;

}



WRAP_DIR_ITER(exfat_iterate) // FIXME!

const struct file_operations exfat_dir_operations = {

	.llseek		= generic_file_llseek,

	.read		= generic_read_dir,

	.iterate	= exfat_iterate,

	.iterate_shared	= shared_exfat_iterate,

	.unlocked_ioctl = exfat_ioctl,

#ifdef CONFIG_COMPAT

	.compat_ioctl = exfat_compat_ioctl,