Re: [PATCH, RFC] what's the point of mtd_inodefs?

[Richard Weinberger]

On Tue, Oct 22, 2013 at 5:33 PM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> I've been looking at mtdchar as part of a bigger series touching all
> kidns of places and really wonder what the point of mtd_inodefs is.

As far I can tell it was introduced because of that issue:
http://lists.infradead.org/pipermail/linux-mtd/2010-April/029593.html

> We use it to make the file->f_mapping of the mtdchar device point to
> the mapping of it's inodes, but there's nothing special happening with
> mtd_inodefs inodes.  It seems to me like simply switching the
> backing_dev_info to the mtd one, similarly to what we do for /dev/mem
> and friends would be enough for mtdchar.
>
> If this works for the intended use case I'd love to add this series
> to my to be posted series as it would simplify it greatly.
>
>
> diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
> index 684bfa3..a7c9f37 100644
> --- a/drivers/mtd/mtdchar.c
> +++ b/drivers/mtd/mtdchar.c
> @@ -48,7 +48,6 @@ static DEFINE_MUTEX(mtd_mutex);
>   */
>  struct mtd_file_info {
>         struct mtd_info *mtd;
> -       struct inode *ino;
>         enum mtd_file_modes mode;
>  };
>
> @@ -58,10 +57,6 @@ static loff_t mtdchar_lseek(struct file *file, loff_t offset, int orig)
>         return fixed_size_llseek(file, offset, orig, mfi->mtd->size);
>  }
>
> -static int count;
> -static struct vfsmount *mnt;
> -static struct file_system_type mtd_inodefs_type;
> -
>  static int mtdchar_open(struct inode *inode, struct file *file)
>  {
>         int minor = iminor(inode);
> @@ -69,7 +64,6 @@ static int mtdchar_open(struct inode *inode, struct file *file)
>         int ret = 0;
>         struct mtd_info *mtd;
>         struct mtd_file_info *mfi;
> -       struct inode *mtd_ino;
>
>         pr_debug("MTD_open\n");
>
> @@ -77,60 +71,42 @@ static int mtdchar_open(struct inode *inode, struct file *file)
>         if ((file->f_mode & FMODE_WRITE) && (minor & 1))
>                 return -EACCES;
>
> -       ret = simple_pin_fs(&mtd_inodefs_type, &mnt, &count);
> -       if (ret)
> -               return ret;
> -
>         mutex_lock(&mtd_mutex);
> -       mtd = get_mtd_device(NULL, devnum);
>
> +       mtd = get_mtd_device(NULL, devnum);
>         if (IS_ERR(mtd)) {
>                 ret = PTR_ERR(mtd);
> -               goto out;
> +               goto out_unlock;
>         }
>
>         if (mtd->type == MTD_ABSENT) {
>                 ret = -ENODEV;
> -               goto out1;
> -       }
> -
> -       mtd_ino = iget_locked(mnt->mnt_sb, devnum);
> -       if (!mtd_ino) {
> -               ret = -ENOMEM;
> -               goto out1;
> +               goto out_put_device;
>         }
> -       if (mtd_ino->i_state & I_NEW) {
> -               mtd_ino->i_private = mtd;
> -               mtd_ino->i_mode = S_IFCHR;
> -               mtd_ino->i_data.backing_dev_info = mtd->backing_dev_info;
> -               unlock_new_inode(mtd_ino);
> -       }
> -       file->f_mapping = mtd_ino->i_mapping;
>
>         /* You can't open it RW if it's not a writeable device */
>         if ((file->f_mode & FMODE_WRITE) && !(mtd->flags & MTD_WRITEABLE)) {
>                 ret = -EACCES;
> -               goto out2;
> +               goto out_put_device;
>         }
>
>         mfi = kzalloc(sizeof(*mfi), GFP_KERNEL);
>         if (!mfi) {
>                 ret = -ENOMEM;
> -               goto out2;
> +               goto out_put_device;
>         }
> -       mfi->ino = mtd_ino;
>         mfi->mtd = mtd;
> +
>         file->private_data = mfi;
> +       file->f_mapping->backing_dev_info = mtd->backing_dev_info;
> +
>         mutex_unlock(&mtd_mutex);
>         return 0;
>
> -out2:
> -       iput(mtd_ino);
> -out1:
> +out_put_device:
>         put_mtd_device(mtd);
> -out:
> +out_unlock:
>         mutex_unlock(&mtd_mutex);
> -       simple_release_fs(&mnt, &count);
>         return ret;
>  } /* mtdchar_open */
>
> @@ -147,12 +123,9 @@ static int mtdchar_close(struct inode *inode, struct file *file)
>         if ((file->f_mode & FMODE_WRITE))
>                 mtd_sync(mtd);
>
> -       iput(mfi->ino);
> -
>         put_mtd_device(mtd);
>         file->private_data = NULL;
>         kfree(mfi);
> -       simple_release_fs(&mnt, &count);
>
>         return 0;
>  } /* mtdchar_close */
> @@ -1147,24 +1120,6 @@ static const struct file_operations mtd_fops = {
>  #endif
>  };
>
> -static const struct super_operations mtd_ops = {
> -       .drop_inode = generic_delete_inode,
> -       .statfs = simple_statfs,
> -};
> -
> -static struct dentry *mtd_inodefs_mount(struct file_system_type *fs_type,
> -                               int flags, const char *dev_name, void *data)
> -{
> -       return mount_pseudo(fs_type, "mtd_inode:", &mtd_ops, NULL, MTD_INODE_FS_MAGIC);
> -}
> -
> -static struct file_system_type mtd_inodefs_type = {
> -       .name = "mtd_inodefs",
> -       .mount = mtd_inodefs_mount,
> -       .kill_sb = kill_anon_super,
> -};
> -MODULE_ALIAS_FS("mtd_inodefs");
> -
>  int __init init_mtdchar(void)
>  {
>         int ret;
> @@ -1174,26 +1129,12 @@ int __init init_mtdchar(void)
>         if (ret < 0) {
>                 pr_err("Can't allocate major number %d for MTD\n",
>                        MTD_CHAR_MAJOR);
> -               return ret;
>         }
> -
> -       ret = register_filesystem(&mtd_inodefs_type);
> -       if (ret) {
> -               pr_err("Can't register mtd_inodefs filesystem, error %d\n",
> -                      ret);
> -               goto err_unregister_chdev;
> -       }
> -
> -       return ret;
> -
> -err_unregister_chdev:
> -       __unregister_chrdev(MTD_CHAR_MAJOR, 0, 1 << MINORBITS, "mtd");
>         return ret;
>  }
>
>  void __exit cleanup_mtdchar(void)
>  {
> -       unregister_filesystem(&mtd_inodefs_type);
>         __unregister_chrdev(MTD_CHAR_MAJOR, 0, 1 << MINORBITS, "mtd");
>  }
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/



-- 
Thanks,
//richard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/