On Thu, Aug 07, 2025 at 05:17:56PM +1000, Aleksa Sarai wrote: > On 2025-08-07, Aleksa Sarai <cyphar@xxxxxxxxxx> wrote: > > On 2025-08-06, Askar Safin <safinaskar@xxxxxxxxxxxx> wrote: > > > > I just realised that we probably also want to support FSCONFIG_SET_PATH > > > > > > I just checked kernel code. Indeed nobody uses FSCONFIG_SET_PATH. > > > Moreover, fsparam_path macro is present since 5.1. And for all this > > > time nobody used it. So, let's just remove FSCONFIG_SET_PATH. Nobody > > > used it, so this will not break anything. > > > > > > If you okay with that, I can submit patch, removing it. > > > > I would prefer you didn't -- "*at()" semantics are very useful to a lot > > of programs (*especially* AT_EMPTY_PATH). I would like the pidns= stuff > > to support it, and probably also overlayfs... > > > > I suspect the primary issue is that when migrating to the new mount API, > > filesystem devs just went with the easiest thing to use > > (FSCONFIG_SET_STRING) even though FSCONFIG_SET_PATH would be better. I > > suspect the lack of documentation around fsconfig(2) played a part too. > > > > My impression is that interest in the minutia about fsconfig(2) is quite > > low on the list of priorities for most filesystem devs, and so the neat > > aspects of fsconfig(2) haven't been fully utilised. (In LPC last year, > > we struggled to come to an agreement on how filesystems should use the > > read(2)-based error interface.) > > > > We can very easily move fsparam_string() or fsparam_file_or_string() > > parameters to fsparam_path() and a future fsparam_file_or_path(). I > > would much prefer that as a user. > > Actually, fsparam_bdev() accepts FSCONFIG_SET_PATH in a very roundabout > way (and the checker doesn't verify anything...?). So there is at least > one user (ext4's "journal_path"), it's just not well-documented (which > I'm trying to fix ;]). > > My plan is to update fs_lookup_param() to be more useful for the (fairly > common) use-case of wanting to support paths and file descriptors, and > going through to clean up some of these unused fsparam_* helpers (or > fsparam_* helpers being abused to implement stuff that the fs_parser > core already supports). > > At the very least, overlayfs, ext4, and this procfs patchset can make > use of it. I've never bothered with actually iplementing FSCONFIG_SET_PATH semantics because I think it's really weird to allow *at semantics when setting filesystem parameters. I always thought it's better to force userspace to provide a file descriptor for the final destination instead of doing some arcane lookup variant for mount configuration. But I'm happy to be convinced of its usefulness...