Add devfs(5) support for VOP_MKDIR(9) and VOP_RMDIR(9)

[trasz]

Summary

Add devfs(5) support for VOP_MKDIR(9) and VOP_RMDIR(9).
This is required for Linuxulator (to be able to create
/dev/shm mountpoint), and would have been useful for
reroot, instead of the /dev/reroot/reroot hack.

[kib]

How did you tested this ? I am curious if you tried massively-parallel mkdir/rmdir for the same name. Perhaps try to ask Peter Holm to test this.

[mjg]

I think arbitrary mkdir/rmdir is a can of worms, perfectly avoidable for the stated purpose. lindevfs module could be created to extend devfs mount points with whatever is necessary. Preferably this would be a completely separate fs, but that's probably too problematic. i.e. currently the module would make_dev("shm") on it's own. the func can create directories and if it insists on getting a device, a separate variant can be added.

[tijl]

In D20411#440580, @mjg wrote:

I think arbitrary mkdir/rmdir is a can of worms, perfectly avoidable for the stated purpose.

Arbitrary symlinks are already supported. Why would mkdir be any different?

[kib]

In D20411#440658, @tijl wrote:

In D20411#440580, @mjg wrote:

I think arbitrary mkdir/rmdir is a can of worms, perfectly avoidable for the stated purpose.

Arbitrary symlinks are already supported. Why would mkdir be any different?

Indeed, I think this position should be elaborated. I do not see much problems with allowing directories creation, but I think that implementation might be much simpler e.g. by making a pseudo-driver which can be directed by VOP_MKDIR to create the <path to dir>/.hidden node. Anyway, I am curious about the testing results for the provided patch.

[trasz]

In D20411#440660, @kib wrote:

In D20411#440658, @tijl wrote:

In D20411#440580, @mjg wrote:

I think arbitrary mkdir/rmdir is a can of worms, perfectly avoidable for the stated purpose.

Arbitrary symlinks are already supported. Why would mkdir be any different?

Good point. Also, this is a root-only interface.

Indeed, I think this position should be elaborated. I do not see much problems with allowing directories creation, but I think that implementation might be much simpler e.g. by making a pseudo-driver which can be directed by VOP_MKDIR to create the <path to dir>/.hidden node. Anyway, I am curious about the testing results for the provided patch.

I don't see how that would be any simpler, given that you'd need a way to communicate with that driver. Besides - this is just 150 lines of code, largely copy/pasted from devfs_symlink() and devfs_remove().

Also, regarding mjg@'s suggestion: I've tried that, it is simple, but it's got a number of problems: it breaks stuff for people who want /dev/shm to be a symlink; you need to make sure to load linux_common.ko before trying to mount /dev/shm; weird stuff happens when you unload it with /dev/shm still mounted, etc. Making mkdir work is just nicer from the design point of view.

[pho]

I ran all of the devfs tests I have.
I added a new parallel mkdir() and rmdir() test with VM pressure.
No problems seen.

[trasz]

In D20411#440676, @pho wrote:

I ran all of the devfs tests I have.
I added a new parallel mkdir() and rmdir() test with VM pressure.
No problems seen.

Thank you.

[kib]

Imagine that there is no pseudo ttys in the system, and user called mkdir('/dev/pts'). Then a new pty pair is created, which makes a device node in /dev/pts. On device destruction, the node is removed, and its contained directory is removed if empty. But '/dev/pts' was created by user, not my makedev(9).

In fact, the way in which you did it is even more complicated. Your stray directories are attached to specific mount point. This means that the directory visible to some dmp's when they populate but not other, More important, when the last device node in the stray directory is removed, it should be removed only in some dmp's and not another.

sys/fs/devfs/devfs_vnops.c

1079

Put int after pointers.

1087

Space before '('.

1614

Do not use initializations in local declarations block.

1617

These asserts are rather pointless, VOP wrappers do the checks.

1619

You must hold dmp there, because devfs_delete() might drop dmp lock internally.

1628

I believe that you may remove non-empty directory.

1648

You must hold de around devfs_delete call, since it might drop sx_xlock. But I think that you should reconsider your copy/paste and reuse devfs_rmdir_empty() somehow. May be add a flag to the function to indicate that it should not remove parents.

1655

How could de_cdp be non-NULL for a directory ?

[trasz]

Okay, after giving it some thought I no longer think it's a good idea. I fail to see a reason why anyone would want to create directories in /dev by hand; /dev/shm is a single, specific brain damage that can be addressed much easier with https://reviews.freebsd.org/D20333.