osd_uld: fix printk format warning

PATCH Al Viro 2 participants 6 messages Apr 29, 2009 – May 4, 2009 1 message not in the archive SCSI

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Al Viro

author Applied

2009-04-29 18:51 UTC #1 lore raw

Quoted 7 lines · Randy Dunlap

From: Randy Dunlap <[email protected]>
Fix printk format warnings in osd_uld:
drivers/scsi/osd/osd_uld.c:191: warning:format '%s' expects type 'char *', but argument 2 has type 'struct path'
Also fix a small typo.

Applied, will fold on reorder. Thanks for catching that one and apologies for missing the original posting (I am subscribed to linux-scsi, but...)

Incidentally, what the hell is going with ->i_cdev in there? Boaz?

Boaz Harrosh

2009-04-30 10:30 UTC #2 lore raw

Quoted 5 lines · Al Viro

Quoted 4 lines · Randy Dunlap
From: Randy Dunlap <[email protected]>
Fix printk format warnings in osd_uld:

Quoted 8 lines

Quoted 3 lines
drivers/scsi/osd/osd_uld.c:191: warning:format '%s' expects type 'char *', but argument 2 has type 'struct path'
Also fix a small typo.
Applied, will fold on reorder. Thanks for catching that one and apologies for missing the original posting (I am subscribed to linux-scsi, but...)
Incidentally, what the hell is going with ->i_cdev in there? Boaz?

Thanks for asking. The thing is that this function is called from within the kernel by exofs. Now I have found that if user-mode as never opened an handle on my char-device, then inode->i_cdev is NULL even though it is registered and found. My mount utility for exofs does an open/close on the char-device before calling the Kernel mounter, but this is just a script and can be missed by users. (I guess I need to submit an mount.exofs. where should it be submitted?)

Alternatively we perhaps need a udev rule that, one - loads osd.ko when OSD_TYPE devices are discovered by scsi (like sd), and two - do the above open/close. I was meaning to ask someone about these things.

Much obliged
Boaz

Al Viro

author

2009-05-03 18:35 UTC #3 lore raw

Quoted 10 lines · Boaz Harrosh

Quoted 1 line
Incidentally, what the hell is going with ->i_cdev in there? Boaz?
Thanks for asking. The thing is that this function is called from within the kernel by exofs. Now I have found that if user-mode as never opened an handle on my char-device, then inode->i_cdev is NULL even though it is registered and found. My mount utility for exofs does an open/close on the char-device before calling the Kernel mounter, but this is just a script and can be missed by users. (I guess I need to submit an mount.exofs. where should it be submitted?)

Quoted 3 lines

Alternatively we perhaps need a udev rule that, one - loads osd.ko when OSD_TYPE devices are discovered by scsi (like sd), and two - do the above open/close. I was meaning to ask someone about these things.

IDGI. Why not simply do filp_open() and keep struct file * until the end to pin the thing down? And turn your function into

	check that file->f_op is &osd_fops, if it isn't - ERR_PTR(-EINVAL),

if it is - &((...)file->private_data)->od

IOW, how about something like this (on top of previous patches)?

drivers/scsi/osd/osd_uld.c +7 ~~-54~~

index 22b59e1..38b4a26 100644
@@ -173,68 +173,21 @@ static const struct file_operations osd_fops = {
 	.unlocked_ioctl = osd_uld_ioctl,
 };
 
-struct osd_dev *osduld_path_lookup(const char *name)
+struct osd_dev *osduld_device(struct file *file)
 {
-	struct path path;
-	struct inode *inode;
-	struct cdev *cdev;
-	struct osd_uld_device *uninitialized_var(oud);
-	int error;
-
-	if (!name || !*name) {
-		OSD_ERR("Mount with !path || !*path\n");
-		return ERR_PTR(-EINVAL);
-	}
-
-	error = kern_path(name, LOOKUP_FOLLOW, &path);
-	if (error) {
-		OSD_ERR("path_lookup of %s failed=>%d\n", name, error);
-		return ERR_PTR(error);
-	}
-
-	inode = path.dentry->d_inode;
-	error = -EINVAL; /* Not the right device e.g osd_uld_device */
-	if (!S_ISCHR(inode->i_mode)) {
-		OSD_DEBUG("!S_ISCHR()\n");
-		goto out;
-	}
-
-	cdev = inode->i_cdev;
-	if (!cdev) {
-		OSD_ERR("Before mounting an OSD Based filesystem\n");
-		OSD_ERR("  user-mode must open+close the %s device\n", name);
-		OSD_ERR("  Example: bash: echo < %s\n", name);
-		goto out;
-	}
+	struct osd_uld_device *oud;
 
 	/* The Magic wand. Is it our char-dev */
 	/* TODO: Support sg devices */
-	if (cdev->owner != THIS_MODULE) {
+	if (file->f_op != &osd_fops) {
 		OSD_ERR("Error mounting %s - is not an OSD device\n", name);
-		goto out;
+		return ERR_PTR(-EINVAL);
 	}
 
-	oud = container_of(cdev, struct osd_uld_device, cdev);
-
-	__uld_get(oud);
-	error = 0;
-
-out:
-	path_put(&path);
-	return error ? ERR_PTR(error) : &oud->od;
-}
-EXPORT_SYMBOL(osduld_path_lookup);
-
-void osduld_put_device(struct osd_dev *od)
-{
-	if (od) {
-		struct osd_uld_device *oud = container_of(od,
-						struct osd_uld_device, od);
-
-		__uld_put(oud);
-	}
+	oud = file->private_data;
+	return &oud->od;
 }
-EXPORT_SYMBOL(osduld_put_device);
+EXPORT_SYMBOL(osduld_device);
 
 /*
  * Scsi Device operations

fs/exofs/exofs.h +1 -0

index 0fd4c78..45e41f2 100644
@@ -57,6 +57,7 @@
  * our extension to the in-memory superblock
  */
 struct exofs_sb_info {
+	struct file	*s_file;		/* underlying file */
 	struct osd_dev	*s_dev;			/* returned by get_osd_dev    */
 	osd_id		s_pid;			/* partition ID of file system*/
 	int		s_timeout;		/* timeout for OSD operations */

fs/exofs/super.c +19 ~~-15~~

index 3cdb761..c3fcb6d 100644
@@ -35,9 +35,10 @@
 
 #include <linux/string.h>
 #include <linux/parser.h>
-#include <linux/vfs.h>
+#include <linux/statfs.h>
 #include <linux/random.h>
 #include <linux/exportfs.h>
+#include <linux/file.h>
 
 #include "exofs.h"

@@ -271,7 +272,7 @@ static void exofs_put_super(struct super_block *sb)
 				  msecs_to_jiffies(100));
 	}
 
-	osduld_put_device(sbi->s_dev);
+	fput(sbi->s_file);
 	kfree(sb->s_fs_info);
 	sb->s_fs_info = NULL;
 }

@@ -295,13 +296,15 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_fs_info = sbi;
 
 	/* use mount options to fill superblock */
-	sbi->s_dev = osduld_path_lookup(opts->dev_name);
-	if (IS_ERR(sbi->s_dev)) {
-		ret = PTR_ERR(sbi->s_dev);
-		sbi->s_dev = NULL;
+	sbi->s_file = filp_open(opts->dev_name, O_RDWR, 0);
+	ret = PTR_ERR(sbi->s_file);
+	if (IS_ERR(sbi->s_file))
 		goto free_sbi;
-	}
 
+	sbi->s_dev = osduld_device(sbi->s_file);
+	ret = PTR_ERR(sbi->s_dev);
+	if (IS_ERR(sbi->s_dev))
+		goto close_sbi;
 	sbi->s_pid = opts->pid;
 	sbi->s_timeout = opts->timeout;

@@ -326,7 +329,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 			EXOFS_ERR(
 			       "exofs_fill_super: osd_start_request failed.\n");
 		ret = -ENOMEM;
-		goto free_sbi;
+		goto close_sbi;
 	}
 	ret = osd_req_read_kern(or, &obj, 0, &fscb, sizeof(fscb));
 	if (unlikely(ret)) {

@@ -334,7 +337,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 			EXOFS_ERR(
 			       "exofs_fill_super: osd_req_read_kern failed.\n");
 		ret = -ENOMEM;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	ret = exofs_sync_op(or, sbi->s_timeout, sbi->s_cred);

@@ -342,7 +345,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 		if (!silent)
 			EXOFS_ERR("exofs_fill_super: exofs_sync_op failed.\n");
 		ret = -EIO;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	sb->s_magic = le16_to_cpu(fscb.s_magic);

@@ -354,7 +357,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 		if (!silent)
 			EXOFS_ERR("ERROR: Bad magic value\n");
 		ret = -EINVAL;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	/* start generation numbers from a random point */

@@ -368,14 +371,14 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 	if (IS_ERR(root)) {
 		EXOFS_ERR("ERROR: exofs_iget failed\n");
 		ret = PTR_ERR(root);
-		goto free_sbi;
+		goto close_sbi;
 	}
 	sb->s_root = d_alloc_root(root);
 	if (!sb->s_root) {
 		iput(root);
 		EXOFS_ERR("ERROR: get root inode failed\n");
 		ret = -ENOMEM;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	if (!S_ISDIR(root->i_mode)) {

@@ -384,7 +387,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 		EXOFS_ERR("ERROR: corrupt root inode (mode = %hd)\n",
 		       root->i_mode);
 		ret = -EINVAL;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	ret = 0;

@@ -393,8 +396,9 @@ out:
 		osd_end_request(or);
 	return ret;
 
+close_sbi:
+	fput(sbi->s_file);
 free_sbi:
-	osduld_put_device(sbi->s_dev); /* NULL safe */
 	kfree(sbi);
 	goto out;
 }

include/scsi/osd_initiator.h +2 -2

index b24d961..f9b6847 100644
@@ -55,8 +55,8 @@ struct osd_dev {
 };
 
 /* Retrieve/return osd_dev(s) for use by Kernel clients */
-struct osd_dev *osduld_path_lookup(const char *dev_name); /*Use IS_ERR/ERR_PTR*/
-void osduld_put_device(struct osd_dev *od);
+struct file;
+struct osd_dev *osduld_device(struct file *); /*Use IS_ERR/ERR_PTR*/
 
 /* Add/remove test ioctls from external modules */
 typedef int (do_test_fn)(struct osd_dev *od, unsigned cmd, unsigned long arg);

Al Viro

author

2009-05-03 19:02 UTC #4 lore raw

Quoted 6 lines · Al Viro

IDGI. Why not simply do filp_open() and keep struct file * until the end to pin the thing down? And turn your function into
	check that file->f_op is &osd_fops, if it isn't - ERR_PTR(-EINVAL),
if it is - &((...)file->private_data)->od
IOW, how about something like this (on top of previous patches)?

drivers/scsi/osd/osd_uld.c +7 ~~-56~~

index 22b59e1..9fff4ca 100644
@@ -173,68 +173,19 @@ static const struct file_operations osd_fops = {
 	.unlocked_ioctl = osd_uld_ioctl,
 };
 
-struct osd_dev *osduld_path_lookup(const char *name)
+struct osd_dev *osduld_device(struct file *file)
 {
-	struct path path;
-	struct inode *inode;
-	struct cdev *cdev;
-	struct osd_uld_device *uninitialized_var(oud);
-	int error;
-
-	if (!name || !*name) {
-		OSD_ERR("Mount with !path || !*path\n");
-		return ERR_PTR(-EINVAL);
-	}
-
-	error = kern_path(name, LOOKUP_FOLLOW, &path);
-	if (error) {
-		OSD_ERR("path_lookup of %s failed=>%d\n", name, error);
-		return ERR_PTR(error);
-	}
-
-	inode = path.dentry->d_inode;
-	error = -EINVAL; /* Not the right device e.g osd_uld_device */
-	if (!S_ISCHR(inode->i_mode)) {
-		OSD_DEBUG("!S_ISCHR()\n");
-		goto out;
-	}
-
-	cdev = inode->i_cdev;
-	if (!cdev) {
-		OSD_ERR("Before mounting an OSD Based filesystem\n");
-		OSD_ERR("  user-mode must open+close the %s device\n", name);
-		OSD_ERR("  Example: bash: echo < %s\n", name);
-		goto out;
-	}
+	struct osd_uld_device *oud;
 
 	/* The Magic wand. Is it our char-dev */
 	/* TODO: Support sg devices */
-	if (cdev->owner != THIS_MODULE) {
-		OSD_ERR("Error mounting %s - is not an OSD device\n", name);
-		goto out;
-	}
-
-	oud = container_of(cdev, struct osd_uld_device, cdev);
-
-	__uld_get(oud);
-	error = 0;
-
-out:
-	path_put(&path);
-	return error ? ERR_PTR(error) : &oud->od;
-}
-EXPORT_SYMBOL(osduld_path_lookup);
-
-void osduld_put_device(struct osd_dev *od)
-{
-	if (od) {
-		struct osd_uld_device *oud = container_of(od,
-						struct osd_uld_device, od);
+	if (file->f_op != &osd_fops)
+		return ERR_PTR(-EINVAL);
 
-		__uld_put(oud);
-	}
+	oud = file->private_data;
+	return &oud->od;
 }
-EXPORT_SYMBOL(osduld_put_device);
+EXPORT_SYMBOL(osduld_device);
 
 /*
  * Scsi Device operations

fs/exofs/exofs.h +1 -0

index 0fd4c78..45e41f2 100644
@@ -57,6 +57,7 @@
  * our extension to the in-memory superblock
  */
 struct exofs_sb_info {
+	struct file	*s_file;		/* underlying file */
 	struct osd_dev	*s_dev;			/* returned by get_osd_dev    */
 	osd_id		s_pid;			/* partition ID of file system*/
 	int		s_timeout;		/* timeout for OSD operations */

fs/exofs/super.c +19 ~~-15~~

index 3cdb761..c3fcb6d 100644
@@ -35,9 +35,10 @@
 
 #include <linux/string.h>
 #include <linux/parser.h>
-#include <linux/vfs.h>
+#include <linux/statfs.h>
 #include <linux/random.h>
 #include <linux/exportfs.h>
+#include <linux/file.h>
 
 #include "exofs.h"

@@ -271,7 +272,7 @@ static void exofs_put_super(struct super_block *sb)
 				  msecs_to_jiffies(100));
 	}
 
-	osduld_put_device(sbi->s_dev);
+	fput(sbi->s_file);
 	kfree(sb->s_fs_info);
 	sb->s_fs_info = NULL;
 }

@@ -295,13 +296,15 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_fs_info = sbi;
 
 	/* use mount options to fill superblock */
-	sbi->s_dev = osduld_path_lookup(opts->dev_name);
-	if (IS_ERR(sbi->s_dev)) {
-		ret = PTR_ERR(sbi->s_dev);
-		sbi->s_dev = NULL;
+	sbi->s_file = filp_open(opts->dev_name, O_RDWR, 0);
+	ret = PTR_ERR(sbi->s_file);
+	if (IS_ERR(sbi->s_file))
 		goto free_sbi;
-	}
 
+	sbi->s_dev = osduld_device(sbi->s_file);
+	ret = PTR_ERR(sbi->s_dev);
+	if (IS_ERR(sbi->s_dev))
+		goto close_sbi;
 	sbi->s_pid = opts->pid;
 	sbi->s_timeout = opts->timeout;

@@ -326,7 +329,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 			EXOFS_ERR(
 			       "exofs_fill_super: osd_start_request failed.\n");
 		ret = -ENOMEM;
-		goto free_sbi;
+		goto close_sbi;
 	}
 	ret = osd_req_read_kern(or, &obj, 0, &fscb, sizeof(fscb));
 	if (unlikely(ret)) {

@@ -334,7 +337,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 			EXOFS_ERR(
 			       "exofs_fill_super: osd_req_read_kern failed.\n");
 		ret = -ENOMEM;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	ret = exofs_sync_op(or, sbi->s_timeout, sbi->s_cred);

@@ -342,7 +345,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 		if (!silent)
 			EXOFS_ERR("exofs_fill_super: exofs_sync_op failed.\n");
 		ret = -EIO;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	sb->s_magic = le16_to_cpu(fscb.s_magic);

@@ -354,7 +357,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 		if (!silent)
 			EXOFS_ERR("ERROR: Bad magic value\n");
 		ret = -EINVAL;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	/* start generation numbers from a random point */

@@ -368,14 +371,14 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 	if (IS_ERR(root)) {
 		EXOFS_ERR("ERROR: exofs_iget failed\n");
 		ret = PTR_ERR(root);
-		goto free_sbi;
+		goto close_sbi;
 	}
 	sb->s_root = d_alloc_root(root);
 	if (!sb->s_root) {
 		iput(root);
 		EXOFS_ERR("ERROR: get root inode failed\n");
 		ret = -ENOMEM;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	if (!S_ISDIR(root->i_mode)) {

@@ -384,7 +387,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 		EXOFS_ERR("ERROR: corrupt root inode (mode = %hd)\n",
 		       root->i_mode);
 		ret = -EINVAL;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	ret = 0;

@@ -393,8 +396,9 @@ out:
 		osd_end_request(or);
 	return ret;
 
+close_sbi:
+	fput(sbi->s_file);
 free_sbi:
-	osduld_put_device(sbi->s_dev); /* NULL safe */
 	kfree(sbi);
 	goto out;
 }

include/scsi/osd_initiator.h +2 -2

index b24d961..f9b6847 100644
@@ -55,8 +55,8 @@ struct osd_dev {
 };
 
 /* Retrieve/return osd_dev(s) for use by Kernel clients */
-struct osd_dev *osduld_path_lookup(const char *dev_name); /*Use IS_ERR/ERR_PTR*/
-void osduld_put_device(struct osd_dev *od);
+struct file;
+struct osd_dev *osduld_device(struct file *); /*Use IS_ERR/ERR_PTR*/
 
 /* Add/remove test ioctls from external modules */
 typedef int (do_test_fn)(struct osd_dev *od, unsigned cmd, unsigned long arg);

Boaz Harrosh

2009-05-04 16:09 UTC #5 lore raw

Quoted 8 lines · Al Viro

Quoted 6 lines · Al Viro
IDGI. Why not simply do filp_open() and keep struct file * until the end to pin the thing down? And turn your function into
	check that file->f_op is &osd_fops, if it isn't - ERR_PTR(-EINVAL),
if it is - &((...)file->private_data)->od
IOW, how about something like this (on top of previous patches)?

Thanks Al, very much. It is exactly what I was hoping for, but could not figure out for myself.

I will give it an hard testing ASAP, and will queue it for next kernel (2.6.31).

Can I take the osd cleanup hunk form your patchset + Randy's fix and serialize them together with this through the scsi tree? (Together with the rest of the osd patches) Is your patchset dependent on the osd bit for removal of some code? If so then I'll send this patch through your tree so not to have conflicts in linux-next.

Thanks again
Boaz

Quoted 226 lines

drivers/scsi/osd/osd_uld.c +7 ~~-56~~

index 22b59e1..9fff4ca 100644
@@ -173,68 +173,19 @@ static const struct file_operations osd_fops = {
 	.unlocked_ioctl = osd_uld_ioctl,
 };
 
-struct osd_dev *osduld_path_lookup(const char *name)
+struct osd_dev *osduld_device(struct file *file)
 {
-	struct path path;
-	struct inode *inode;
-	struct cdev *cdev;
-	struct osd_uld_device *uninitialized_var(oud);
-	int error;
-
-	if (!name || !*name) {
-		OSD_ERR("Mount with !path || !*path\n");
-		return ERR_PTR(-EINVAL);
-	}
-
-	error = kern_path(name, LOOKUP_FOLLOW, &path);
-	if (error) {
-		OSD_ERR("path_lookup of %s failed=>%d\n", name, error);
-		return ERR_PTR(error);
-	}
-
-	inode = path.dentry->d_inode;
-	error = -EINVAL; /* Not the right device e.g osd_uld_device */
-	if (!S_ISCHR(inode->i_mode)) {
-		OSD_DEBUG("!S_ISCHR()\n");
-		goto out;
-	}
-
-	cdev = inode->i_cdev;
-	if (!cdev) {
-		OSD_ERR("Before mounting an OSD Based filesystem\n");
-		OSD_ERR("  user-mode must open+close the %s device\n", name);
-		OSD_ERR("  Example: bash: echo < %s\n", name);
-		goto out;
-	}
+	struct osd_uld_device *oud;
 
 	/* The Magic wand. Is it our char-dev */
 	/* TODO: Support sg devices */
-	if (cdev->owner != THIS_MODULE) {
-		OSD_ERR("Error mounting %s - is not an OSD device\n", name);
-		goto out;
-	}
-
-	oud = container_of(cdev, struct osd_uld_device, cdev);
-
-	__uld_get(oud);
-	error = 0;
-
-out:
-	path_put(&path);
-	return error ? ERR_PTR(error) : &oud->od;
-}
-EXPORT_SYMBOL(osduld_path_lookup);
-
-void osduld_put_device(struct osd_dev *od)
-{
-	if (od) {
-		struct osd_uld_device *oud = container_of(od,
-						struct osd_uld_device, od);
+	if (file->f_op != &osd_fops)
+		return ERR_PTR(-EINVAL);
 
-		__uld_put(oud);
-	}
+	oud = file->private_data;
+	return &oud->od;
 }
-EXPORT_SYMBOL(osduld_put_device);
+EXPORT_SYMBOL(osduld_device);
 
 /*
  * Scsi Device operations

fs/exofs/exofs.h +1 -0

index 0fd4c78..45e41f2 100644
@@ -57,6 +57,7 @@
  * our extension to the in-memory superblock
  */
 struct exofs_sb_info {
+	struct file	*s_file;		/* underlying file */
 	struct osd_dev	*s_dev;			/* returned by get_osd_dev    */
 	osd_id		s_pid;			/* partition ID of file system*/
 	int		s_timeout;		/* timeout for OSD operations */

fs/exofs/super.c +19 ~~-15~~

index 3cdb761..c3fcb6d 100644
@@ -35,9 +35,10 @@
 
 #include <linux/string.h>
 #include <linux/parser.h>
-#include <linux/vfs.h>
+#include <linux/statfs.h>
 #include <linux/random.h>
 #include <linux/exportfs.h>
+#include <linux/file.h>
 
 #include "exofs.h"

@@ -271,7 +272,7 @@ static void exofs_put_super(struct super_block *sb)
 				  msecs_to_jiffies(100));
 	}
 
-	osduld_put_device(sbi->s_dev);
+	fput(sbi->s_file);
 	kfree(sb->s_fs_info);
 	sb->s_fs_info = NULL;
 }

@@ -295,13 +296,15 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_fs_info = sbi;
 
 	/* use mount options to fill superblock */
-	sbi->s_dev = osduld_path_lookup(opts->dev_name);
-	if (IS_ERR(sbi->s_dev)) {
-		ret = PTR_ERR(sbi->s_dev);
-		sbi->s_dev = NULL;
+	sbi->s_file = filp_open(opts->dev_name, O_RDWR, 0);
+	ret = PTR_ERR(sbi->s_file);
+	if (IS_ERR(sbi->s_file))
 		goto free_sbi;
-	}
 
+	sbi->s_dev = osduld_device(sbi->s_file);
+	ret = PTR_ERR(sbi->s_dev);
+	if (IS_ERR(sbi->s_dev))
+		goto close_sbi;
 	sbi->s_pid = opts->pid;
 	sbi->s_timeout = opts->timeout;

@@ -326,7 +329,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 			EXOFS_ERR(
 			       "exofs_fill_super: osd_start_request failed.\n");
 		ret = -ENOMEM;
-		goto free_sbi;
+		goto close_sbi;
 	}
 	ret = osd_req_read_kern(or, &obj, 0, &fscb, sizeof(fscb));
 	if (unlikely(ret)) {

@@ -334,7 +337,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 			EXOFS_ERR(
 			       "exofs_fill_super: osd_req_read_kern failed.\n");
 		ret = -ENOMEM;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	ret = exofs_sync_op(or, sbi->s_timeout, sbi->s_cred);

@@ -342,7 +345,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 		if (!silent)
 			EXOFS_ERR("exofs_fill_super: exofs_sync_op failed.\n");
 		ret = -EIO;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	sb->s_magic = le16_to_cpu(fscb.s_magic);

@@ -354,7 +357,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 		if (!silent)
 			EXOFS_ERR("ERROR: Bad magic value\n");
 		ret = -EINVAL;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	/* start generation numbers from a random point */

@@ -368,14 +371,14 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 	if (IS_ERR(root)) {
 		EXOFS_ERR("ERROR: exofs_iget failed\n");
 		ret = PTR_ERR(root);
-		goto free_sbi;
+		goto close_sbi;
 	}
 	sb->s_root = d_alloc_root(root);
 	if (!sb->s_root) {
 		iput(root);
 		EXOFS_ERR("ERROR: get root inode failed\n");
 		ret = -ENOMEM;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	if (!S_ISDIR(root->i_mode)) {

@@ -384,7 +387,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent)
 		EXOFS_ERR("ERROR: corrupt root inode (mode = %hd)\n",
 		       root->i_mode);
 		ret = -EINVAL;
-		goto free_sbi;
+		goto close_sbi;
 	}
 
 	ret = 0;

@@ -393,8 +396,9 @@ out:
 		osd_end_request(or);
 	return ret;
 
+close_sbi:
+	fput(sbi->s_file);
 free_sbi:
-	osduld_put_device(sbi->s_dev); /* NULL safe */
 	kfree(sbi);
 	goto out;
 }

include/scsi/osd_initiator.h +2 -2

index b24d961..f9b6847 100644
@@ -55,8 +55,8 @@ struct osd_dev {
 };
 
 /* Retrieve/return osd_dev(s) for use by Kernel clients */
-struct osd_dev *osduld_path_lookup(const char *dev_name); /*Use IS_ERR/ERR_PTR*/
-void osduld_put_device(struct osd_dev *od);
+struct file;
+struct osd_dev *osduld_device(struct file *); /*Use IS_ERR/ERR_PTR*/
 
 /* Add/remove test ioctls from external modules */
 typedef int (do_test_fn)(struct osd_dev *od, unsigned cmd, unsigned long arg);

Al Viro

author

2009-05-04 18:30 UTC #6 lore raw

Quoted 9 lines · Boaz Harrosh

Thanks Al, very much. It is exactly what I was hoping for, but could not figure out for myself.
I will give it an hard testing ASAP, and will queue it for next kernel (2.6.31).
Can I take the osd cleanup hunk form your patchset + Randy's fix and serialize them together with this through the scsi tree? (Together with the rest of the osd patches) Is your patchset dependent on the osd bit for removal of some code? If so then I'll send this patch through your tree so not to have conflicts in linux-next.

Actually, git should manage such merge just fine, so -next is probably not a concern. In any case, if there are any problems with the merge I can take that chunk out - nothing depends on it yet in the public queue. Or I could take this incremental in vfs tree - also not a problem...