>> At least CEPH has active maintainers... Well that's just mean :-) ... Anywho... >> in many cases, the existing iterate() implementation works just fine as iterate_shared(). >> https://lwn.net/Articles/686943/ I changed the orangefs .iterate to .iterate_shared... ls still works, find still works, xfstests shows no regressions... I found and ran my getdents program from back when we were working to go upstream... it works the same with .iterate as it does with .iterate_shared... -Mike On Thu, Aug 18, 2022 at 12:15 PM John Johansen <john.johansen@xxxxxxxxxxxxx> wrote: > > On 8/16/22 12:11, Matthew Wilcox wrote: > > On Tue, Aug 16, 2022 at 11:58:36AM -0700, Linus Torvalds wrote: > >> That said, our filldir code is still confusing as hell. And I would > >> really like to see that "shared vs non-shared" iterator thing go away, > >> with everybody using the shared one - and filesystems that can't deal > >> with it using their own lock. > >> > >> But that's a completely independent wart in our complicated filldir saga. > >> > >> But if somebody were to look at that iterate-vs-iterate_shared, that > >> would be lovely. A quick grep shows that we don't have *that* many of > >> the non-shared cases left: > >> > >> git grep '\.iterate\>.*=' > >> > >> seems to imply that converting them to a "use my own load" wouldn't be > >> _too_ bad. > >> > >> And some of them might actually be perfectly ok with the shared > >> semantics (ie inode->i_rwsem held just for reading) and they just were > >> never converted originally. > > > > What's depressing is that some of these are newly added. It'd be > > great if we could attach something _like_ __deprecated to things > > that checkpatch could pick up on. > > > > fs/adfs/dir_f.c: .iterate = adfs_f_iterate, > > fs/adfs/dir_fplus.c: .iterate = adfs_fplus_iterate, > > > > ADFS is read-only, so must be safe? > > > > fs/ceph/dir.c: .iterate = ceph_readdir, > > fs/ceph/dir.c: .iterate = ceph_readdir, > > > > At least CEPH has active maintainers, cc'd > > > > fs/coda/dir.c: .iterate = coda_readdir, > > > > Would anyone notice if we broke CODA? Maintainers cc'd anyway. > > > > fs/exfat/dir.c: .iterate = exfat_iterate, > > > > Exfat is a new addition, but has active maintainers. > > > > fs/jfs/namei.c: .iterate = jfs_readdir, > > > > Maintainer cc'd > > > > fs/ntfs/dir.c: .iterate = ntfs_readdir, /* Read directory contents. */ > > > > Maybe we can get rid of ntfs soon. > > > > fs/ocfs2/file.c: .iterate = ocfs2_readdir, > > fs/ocfs2/file.c: .iterate = ocfs2_readdir, > > > > maintainers cc'd > > > > fs/orangefs/dir.c: .iterate = orangefs_dir_iterate, > > > > New; maintainer cc'd > > > > fs/overlayfs/readdir.c: .iterate = ovl_iterate, > > > > Active maintainer, cc'd > > > > fs/proc/base.c: .iterate = proc_##LSM##_attr_dir_iterate, \ > > > > Hmm. We need both SMACK and Apparmor to agree to this ... cc's added. > > This is fine for AppArmor > > > > > > fs/vboxsf/dir.c: .iterate = vboxsf_dir_iterate, > > > > Also newly added. Maintainer cc'd. > > >
