On Mon 03-05-10 19:56:07, Kirill A. Shutemov wrote:
> We cannot modify file->f_mapping->backing_dev_info, because it will corrupt
> backing device of device node inode, since file->f_mapping is equal to
> inode->i_mapping (see __dentry_open() in fs/open.c).
>
> Let's introduce separate inode for MTD device with appropriate backing
> device.
Now the patch looks much cleaner. Thanks! Having a separate fstype for a
single inode seems like a bit of an overkill but I agree there doesn't seem
to be a suitable filesystem where an inode could live, so it's probably OK.
Two minor comments are below.
> Signed-off-by: Kirill A. Shutemov <kirill@xxxxxxxxxxxxx>
> ---
> drivers/mtd/mtdchar.c | 70 +++++++++++++++++++++++++++++++++++++++++-----
> drivers/mtd/mtdcore.c | 3 ++
> include/linux/mtd/mtd.h | 3 ++
> 3 files changed, 68 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
> index 5b081cb..24ea34f 100644
> --- a/drivers/mtd/mtdchar.c
> +++ b/drivers/mtd/mtdchar.c
> @@ -88,8 +91,21 @@ static int mtd_open(struct inode *inode, struct file *file)
> goto out;
> }
>
> - if (mtd->backing_dev_info)
> - file->f_mapping->backing_dev_info = mtd->backing_dev_info;
> + if (!mtd->inode) {
> + mtd->inode = new_inode(mtd_inode_mnt->mnt_sb);
This can fail so you should check whether mtd->inode is != NULL.
...
> static int __init init_mtdchar(void)
> {
> - int status;
> + int ret;
>
> - status = register_chrdev(MTD_CHAR_MAJOR, "mtd", &mtd_fops);
> - if (status < 0) {
> - printk(KERN_NOTICE "Can't allocate major number %d for Memory Technology Devices.\n",
> - MTD_CHAR_MAJOR);
> + ret = register_chrdev(MTD_CHAR_MAJOR, "mtd", &mtd_fops);
> + if (ret < 0) {
> + pr_notice("Can't allocate major number %d for "
> + "Memory Technology Devices.\n", MTD_CHAR_MAJOR);
> + return ret;
> + }
> +
> + ret = register_filesystem(&mtd_inodefs_type);
> + if (ret) {
> + pr_notice("Can't register mtd_inodefs filesystem: %d\n", ret);
> + goto err_unregister_chdev;
> + }
> +
> + mtd_inode_mnt = kern_mount(&mtd_inodefs_type);
> + if (IS_ERR(mtd_inode_mnt)) {
> + ret = PTR_ERR(mtd_inode_mnt);
> + pr_notice("Error mounting mtd_inodefs filesystem: %d\n", ret);
> + goto err_unregister_filesystem;
> }
>
> - return status;
> + return ret;
> +
> +err_unregister_chdev:
> + unregister_chrdev(MTD_CHAR_MAJOR, "mtd");
> +err_unregister_filesystem:
> + unregister_filesystem(&mtd_inodefs_type);
> + return ret;
I think you should swap unregister_chrdev and unregister_filesystem
blocks...
Honza
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
