| Message ID | 20200721162818.197315-7-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/24] fs: refactor do_mount | expand |
On Tue, Jul 21, 2020 at 06:28:00PM +0200, Christoph Hellwig wrote: > Instead of passing a kernel pointer to vfs_stat by relying on the > implicit set_fs(KERNEL_DS) in md_setup_drive, just open code the > trivial getattr, and use the opportunity to move a little bit more > code from the caller into the new helper. Ugh... > +static void __init md_lookup_dev(const char *devname, dev_t *dev) > +{ > + struct kstat stat; > + struct path path; > + char filename[64]; > + > + if (strncmp(devname, "/dev/", 5) == 0) > + devname += 5; > + snprintf(filename, 63, "/dev/%s", devname); > + > + if (!kern_path(filename, LOOKUP_FOLLOW, &path) && > + !vfs_getattr(&path, &stat, STATX_BASIC_STATS, AT_NO_AUTOMOUNT) && > + S_ISBLK(stat.mode)) > + *dev = new_decode_dev(stat.rdev); > + path_put(&path); > +} How about fs/for_init.c and putting the damn helpers there? With calling conventions as close to syscalls as possible, and a fat comment regarding their intended use being _ONLY_ the setup in should-have-been-done-in-userland parts of init? I really want to keep the surface as small as possible - we had fun shite several releases ago when somebody tried that kind of crap (with open(), IIRC). Let's not go there again...
On Tue, Jul 21, 2020 at 05:55:39PM +0100, Al Viro wrote: > How about fs/for_init.c and putting the damn helpers there? With > calling conventions as close to syscalls as possible, and a fat > comment regarding their intended use being _ONLY_ the setup > in should-have-been-done-in-userland parts of init? Where do you want the prototypes to go? Also do you want devtmpfs use the same helpers, which then't can't be marked __init (mount, chdir, chroot), or separate copies?
On Tue, Jul 21, 2020 at 08:27:01PM +0200, Christoph Hellwig wrote: > On Tue, Jul 21, 2020 at 05:55:39PM +0100, Al Viro wrote: > > How about fs/for_init.c and putting the damn helpers there? With > > calling conventions as close to syscalls as possible, and a fat > > comment regarding their intended use being _ONLY_ the setup > > in should-have-been-done-in-userland parts of init? > > Where do you want the prototypes to go? Also do you want devtmpfs > use the same helpers, which then't can't be marked __init (mount, > chdir, chroot), or separate copies? Hmm... mount still can be __init (devtmpfs_mount() is), and I suspect devtmpfs_setup() could also be made such - just turn devtmpfsd() into static int __init devtmpfsd(void *p) { int err = devtmpfs_setup(p); if (!err) devtmpfsd_real(); /* never returns */ return err; } and you are done. As for the prototypes... include/linux/init_syscalls.h, perhaps?
On Wed, Jul 22, 2020 at 08:44:32AM +0100, Al Viro wrote: > On Tue, Jul 21, 2020 at 08:27:01PM +0200, Christoph Hellwig wrote: > > On Tue, Jul 21, 2020 at 05:55:39PM +0100, Al Viro wrote: > > > How about fs/for_init.c and putting the damn helpers there? With > > > calling conventions as close to syscalls as possible, and a fat > > > comment regarding their intended use being _ONLY_ the setup > > > in should-have-been-done-in-userland parts of init? > > > > Where do you want the prototypes to go? Also do you want devtmpfs > > use the same helpers, which then't can't be marked __init (mount, > > chdir, chroot), or separate copies? > > Hmm... mount still can be __init (devtmpfs_mount() is), and I suspect > devtmpfs_setup() could also be made such - just turn devtmpfsd() > into > static int __init devtmpfsd(void *p) > { > int err = devtmpfs_setup(p); > > if (!err) > devtmpfsd_real(); /* never returns */ > return err; > } > and you are done. Yes, that seems to work. We can obviously call non-__init functions from __init ones, and kthread_run doesn't seem to care if it gets passed a __init function. Here is what I have now: http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/init_path
diff --git a/drivers/md/md-autodetect.c b/drivers/md/md-autodetect.c index 14b6e86814c061..1e8f1df257a112 100644 --- a/drivers/md/md-autodetect.c +++ b/drivers/md/md-autodetect.c @@ -8,6 +8,7 @@ #include <linux/raid/detect.h> #include <linux/raid/md_u.h> #include <linux/raid/md_p.h> +#include <linux/namei.h> #include "md.h" /* @@ -119,6 +120,23 @@ static int __init md_setup(char *str) return 1; } +static void __init md_lookup_dev(const char *devname, dev_t *dev) +{ + struct kstat stat; + struct path path; + char filename[64]; + + if (strncmp(devname, "/dev/", 5) == 0) + devname += 5; + snprintf(filename, 63, "/dev/%s", devname); + + if (!kern_path(filename, LOOKUP_FOLLOW, &path) && + !vfs_getattr(&path, &stat, STATX_BASIC_STATS, AT_NO_AUTOMOUNT) && + S_ISBLK(stat.mode)) + *dev = new_decode_dev(stat.rdev); + path_put(&path); +} + static void __init md_setup_drive(struct md_setup_args *args) { char *devname = args->device_names; @@ -138,21 +156,14 @@ static void __init md_setup_drive(struct md_setup_args *args) } for (i = 0; i < MD_SB_DISKS && devname != NULL; i++) { - struct kstat stat; - char *p; - char comp_name[64]; dev_t dev; + char *p; p = strchr(devname, ','); if (p) *p++ = 0; - dev = name_to_dev_t(devname); - if (strncmp(devname, "/dev/", 5) == 0) - devname += 5; - snprintf(comp_name, 63, "/dev/%s", devname); - if (vfs_stat(comp_name, &stat) == 0 && S_ISBLK(stat.mode)) - dev = new_decode_dev(stat.rdev); + md_lookup_dev(devname, &dev); if (!dev) { pr_warn("md: Unknown device name: %s\n", devname); break;
Instead of passing a kernel pointer to vfs_stat by relying on the implicit set_fs(KERNEL_DS) in md_setup_drive, just open code the trivial getattr, and use the opportunity to move a little bit more code from the caller into the new helper. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/md/md-autodetect.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-)