Re: [PATCH -next] osd_uld: fix printk format warning

Re: [PATCH -next] osd_uld: fix printk format warning

[Al Viro]

On Wed, Apr 29, 2009 at 09:26:31AM -0700, Randy Dunlap wrote:
> From: Randy Dunlap <randy.dunlap@oracle.com>
> 
> 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?

Re: [PATCH -next] osd_uld: fix printk format warning

[Boaz Harrosh]

On 04/29/2009 09:51 PM, Al Viro wrote:
> On Wed, Apr 29, 2009 at 09:26:31AM -0700, Randy Dunlap wrote:
>> From: Randy Dunlap <randy.dunlap@oracle.com>
>>
>> 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?

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

Re: [PATCH -next] osd_uld: fix printk format warning

[Al Viro]

On Thu, Apr 30, 2009 at 01:30:07PM +0300, Boaz Harrosh wrote:

> > 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.

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)?

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 22b59e1..38b4a26 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -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
diff --git a/fs/exofs/exofs.h b/fs/exofs/exofs.h
index 0fd4c78..45e41f2 100644
--- a/fs/exofs/exofs.h
+++ b/fs/exofs/exofs.h
@@ -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 */
diff --git a/fs/exofs/super.c b/fs/exofs/super.c
index 3cdb761..c3fcb6d 100644
--- a/fs/exofs/super.c
+++ b/fs/exofs/super.c
@@ -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;
 }
diff --git a/include/scsi/osd_initiator.h b/include/scsi/osd_initiator.h
index b24d961..f9b6847 100644
--- a/include/scsi/osd_initiator.h
+++ b/include/scsi/osd_initiator.h
@@ -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);

Re: [PATCH -next] osd_uld: fix printk format warning

[Al Viro]

On Sun, May 03, 2009 at 07:35:41PM +0100, Al Viro wrote:
> 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)?

diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
index 22b59e1..9fff4ca 100644
--- a/drivers/scsi/osd/osd_uld.c
+++ b/drivers/scsi/osd/osd_uld.c
@@ -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
diff --git a/fs/exofs/exofs.h b/fs/exofs/exofs.h
index 0fd4c78..45e41f2 100644
--- a/fs/exofs/exofs.h
+++ b/fs/exofs/exofs.h
@@ -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 */
diff --git a/fs/exofs/super.c b/fs/exofs/super.c
index 3cdb761..c3fcb6d 100644
--- a/fs/exofs/super.c
+++ b/fs/exofs/super.c
@@ -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;
 }
diff --git a/include/scsi/osd_initiator.h b/include/scsi/osd_initiator.h
index b24d961..f9b6847 100644
--- a/include/scsi/osd_initiator.h
+++ b/include/scsi/osd_initiator.h
@@ -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);

Re: [PATCH -next] osd_uld: fix printk format warning

[Boaz Harrosh]

On 05/03/2009 10:02 PM, Al Viro wrote:
> On Sun, May 03, 2009 at 07:35:41PM +0100, Al Viro wrote:
>> 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

> diff --git a/drivers/scsi/osd/osd_uld.c b/drivers/scsi/osd/osd_uld.c
> index 22b59e1..9fff4ca 100644
> --- a/drivers/scsi/osd/osd_uld.c
> +++ b/drivers/scsi/osd/osd_uld.c
> @@ -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
> diff --git a/fs/exofs/exofs.h b/fs/exofs/exofs.h
> index 0fd4c78..45e41f2 100644
> --- a/fs/exofs/exofs.h
> +++ b/fs/exofs/exofs.h
> @@ -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 */
> diff --git a/fs/exofs/super.c b/fs/exofs/super.c
> index 3cdb761..c3fcb6d 100644
> --- a/fs/exofs/super.c
> +++ b/fs/exofs/super.c
> @@ -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;
>  }
> diff --git a/include/scsi/osd_initiator.h b/include/scsi/osd_initiator.h
> index b24d961..f9b6847 100644
> --- a/include/scsi/osd_initiator.h
> +++ b/include/scsi/osd_initiator.h
> @@ -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);

Re: [PATCH -next] osd_uld: fix printk format warning

[Al Viro]

On Mon, May 04, 2009 at 07:09:33PM +0300, Boaz Harrosh wrote:
 
> 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...

[PATCH -next] osd_uld: fix printk format warning

[Randy Dunlap]

From: Randy Dunlap <randy.dunlap@oracle.com>

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.

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 drivers/scsi/osd/osd_uld.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-next-20090423.orig/drivers/scsi/osd/osd_uld.c
+++ linux-next-20090423/drivers/scsi/osd/osd_uld.c
@@ -188,7 +188,7 @@ struct osd_dev *osduld_path_lookup(const
 
 	error = kern_path(name, LOOKUP_FOLLOW, &path);
 	if (error) {
-		OSD_ERR("path_lookup of %s faild=>%d\n", path, error);
+		OSD_ERR("path_lookup of %s failed=>%d\n", name, error);
 		return ERR_PTR(error);
 	}
 

-- 
~Randy

Re: [PATCH -next] osd_uld: fix printk format warning

[Boaz Harrosh]

On 04/24/2009 12:57 AM, Randy Dunlap wrote:
> From: Randy Dunlap <randy.dunlap@oracle.com>
> 
> 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.
> 
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> ---
>  drivers/scsi/osd/osd_uld.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- linux-next-20090423.orig/drivers/scsi/osd/osd_uld.c
> +++ linux-next-20090423/drivers/scsi/osd/osd_uld.c
> @@ -188,7 +188,7 @@ struct osd_dev *osduld_path_lookup(const
>  
>  	error = kern_path(name, LOOKUP_FOLLOW, &path);
>  	if (error) {
> -		OSD_ERR("path_lookup of %s faild=>%d\n", path, error);
> +		OSD_ERR("path_lookup of %s failed=>%d\n", name, error);
>  		return ERR_PTR(error);
>  	}
>  
> 

Hi Randy thanks for the catch.

I cannot do anything with this patch as it is a fallout of an
Al Viro's patch which I do not have access to, from any of my trees.

This is:
  Reduce path_lookup() abuses

     use kern_path() where possible

  Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

(Linux-next's fe48fce1a72439d1b70dc8a9bce80c46ac4a6c96)


Al hi

Thanks for the fix and simplification. At the time of writing this I
did not find the kern_path(), which is much better in this code, Sorry.

Please squash or apply above Randy's patch. And tell me where I can find
it so I can test it and send an ACK-BY if that's needed. (Or if you want
that I pick this code, I'm here to help)

Thanks a lot for fixing this code
Boaz