[PATCH] xfs_dm_rdwr() needs to pass a vfsmount to dentry_open()

[PATCH] xfs_dm_rdwr() needs to pass a vfsmount to dentry_open()

[Lachlan McIlroy]

There's a new check added to dentry_open():

    if (!mnt) {
        printk(KERN_WARNING "%s called with NULL vfsmount\n", __func__);
        dump_stack();
        return ERR_PTR(-EINVAL);
    }

We need to pass a mountpoint in the call to dentry_open() in xfs_dm_rdwr()
to avoid this code.

--- fs/xfs/dmapi/xfs_dm.c_1.72	2008-05-15 13:23:41.000000000 +1000
+++ fs/xfs/dmapi/xfs_dm.c	2008-05-15 17:51:41.000000000 +1000
@@ -52,6 +52,8 @@
 #include <dmapi_kern.h>
 #include "xfs_dm.h"
 
+#include <linux/mount.h>
+
 #define MAXNAMLEN MAXNAMELEN
 
 #define MIN_DIO_SIZE(mp)		((mp)->m_sb.sb_sectsize)
@@ -1121,7 +1123,7 @@ xfs_dm_rdwr(
 		return ENOMEM;
 	}
 
-	file = dentry_open(dentry, NULL, oflags);
+	file = dentry_open(dentry, mntget(ip->i_mount->m_vfsmount), oflags);
 	if (IS_ERR(file)) {
 		return -PTR_ERR(file);
 	}
--- fs/xfs/linux-2.6/xfs_super.c_1.416	2008-05-15 13:23:43.000000000 +1000
+++ fs/xfs/linux-2.6/xfs_super.c	2008-05-15 13:29:49.000000000 +1000
@@ -1397,8 +1397,14 @@ xfs_fs_get_sb(
 	void			*data,
 	struct vfsmount		*mnt)
 {
-	return get_sb_bdev(fs_type, flags, dev_name, data, xfs_fs_fill_super,
+	int	error;
+
+	error = get_sb_bdev(fs_type, flags, dev_name, data, xfs_fs_fill_super,
 			   mnt);
+	if (!error)
+		((struct xfs_mount *)mnt->mnt_sb->s_fs_info)->m_vfsmount = mnt;
+
+	return error;
 }
 
 static struct super_operations xfs_super_operations = {
--- fs/xfs/xfs_mount.h_1.265	2008-05-15 13:23:49.000000000 +1000
+++ fs/xfs/xfs_mount.h	2008-05-15 13:21:50.000000000 +1000
@@ -337,6 +337,7 @@ typedef struct xfs_mount {
 	spinlock_t		m_sync_lock;	/* work item list lock */
 	int			m_sync_seq;	/* sync thread generation no. */
 	wait_queue_head_t	m_wait_single_sync_task;
+	struct vfsmount		*m_vfsmount;
 } xfs_mount_t;
 
 /*

Re: [PATCH] xfs_dm_rdwr() needs to pass a vfsmount to dentry_open()

[David Chinner]

On Fri, May 16, 2008 at 11:30:07AM +1000, Lachlan McIlroy wrote:
> There's a new check added to dentry_open():
> 
>    if (!mnt) {
>        printk(KERN_WARNING "%s called with NULL vfsmount\n", __func__);
>        dump_stack();
>        return ERR_PTR(-EINVAL);
>    }
> 
> We need to pass a mountpoint in the call to dentry_open() in xfs_dm_rdwr()
> to avoid this code.
> 
> --- fs/xfs/dmapi/xfs_dm.c_1.72	2008-05-15 13:23:41.000000000 +1000
> +++ fs/xfs/dmapi/xfs_dm.c	2008-05-15 17:51:41.000000000 +1000
> @@ -52,6 +52,8 @@
> #include <dmapi_kern.h>
> #include "xfs_dm.h"
> 
> +#include <linux/mount.h>

Should be defined in fs/xfs/linux-2.6/xfs_linux.h, right?

> +
> #define MAXNAMLEN MAXNAMELEN
> 
> #define MIN_DIO_SIZE(mp)		((mp)->m_sb.sb_sectsize)
> @@ -1121,7 +1123,7 @@ xfs_dm_rdwr(
> 		return ENOMEM;
> 	}
> 
> -	file = dentry_open(dentry, NULL, oflags);
> +	file = dentry_open(dentry, mntget(ip->i_mount->m_vfsmount), oflags);
> 	if (IS_ERR(file)) {
> 		return -PTR_ERR(file);
> 	}
> --- fs/xfs/linux-2.6/xfs_super.c_1.416	2008-05-15 13:23:43.000000000 +1000
> +++ fs/xfs/linux-2.6/xfs_super.c	2008-05-15 13:29:49.000000000 +1000
> @@ -1397,8 +1397,14 @@ xfs_fs_get_sb(
> 	void			*data,
> 	struct vfsmount		*mnt)
> {
> -	return get_sb_bdev(fs_type, flags, dev_name, data, xfs_fs_fill_super,
> +	int	error;
> +
> +	error = get_sb_bdev(fs_type, flags, dev_name, data, 
> xfs_fs_fill_super,
> 			   mnt);
> +	if (!error)
> +		((struct xfs_mount *)mnt->mnt_sb->s_fs_info)->m_vfsmount = 
> mnt;

As I previously suggested, please use XFS_M(mnt->mnt_sb) for this.

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

Re: [PATCH] xfs_dm_rdwr() needs to pass a vfsmount to dentry_open()

[Lachlan McIlroy]

David Chinner wrote:
> On Fri, May 16, 2008 at 11:30:07AM +1000, Lachlan McIlroy wrote:
>> There's a new check added to dentry_open():
>>
>>    if (!mnt) {
>>        printk(KERN_WARNING "%s called with NULL vfsmount\n", __func__);
>>        dump_stack();
>>        return ERR_PTR(-EINVAL);
>>    }
>>
>> We need to pass a mountpoint in the call to dentry_open() in xfs_dm_rdwr()
>> to avoid this code.
>>
>> --- fs/xfs/dmapi/xfs_dm.c_1.72	2008-05-15 13:23:41.000000000 +1000
>> +++ fs/xfs/dmapi/xfs_dm.c	2008-05-15 17:51:41.000000000 +1000
>> @@ -52,6 +52,8 @@
>> #include <dmapi_kern.h>
>> #include "xfs_dm.h"
>>
>> +#include <linux/mount.h>
> 
> Should be defined in fs/xfs/linux-2.6/xfs_linux.h, right?
It's not currently in xfs_linux.h.  It's needed for mntget().  I saw that
xfs_ioctl.c uses mntget() and pulls in linux/mount.h directly so I followed
suit.  I can add it to xfs_linux.h if you prefer.

> 
>> +
>> #define MAXNAMLEN MAXNAMELEN
>>
>> #define MIN_DIO_SIZE(mp)		((mp)->m_sb.sb_sectsize)
>> @@ -1121,7 +1123,7 @@ xfs_dm_rdwr(
>> 		return ENOMEM;
>> 	}
>>
>> -	file = dentry_open(dentry, NULL, oflags);
>> +	file = dentry_open(dentry, mntget(ip->i_mount->m_vfsmount), oflags);
>> 	if (IS_ERR(file)) {
>> 		return -PTR_ERR(file);
>> 	}
>> --- fs/xfs/linux-2.6/xfs_super.c_1.416	2008-05-15 13:23:43.000000000 +1000
>> +++ fs/xfs/linux-2.6/xfs_super.c	2008-05-15 13:29:49.000000000 +1000
>> @@ -1397,8 +1397,14 @@ xfs_fs_get_sb(
>> 	void			*data,
>> 	struct vfsmount		*mnt)
>> {
>> -	return get_sb_bdev(fs_type, flags, dev_name, data, xfs_fs_fill_super,
>> +	int	error;
>> +
>> +	error = get_sb_bdev(fs_type, flags, dev_name, data, 
>> xfs_fs_fill_super,
>> 			   mnt);
>> +	if (!error)
>> +		((struct xfs_mount *)mnt->mnt_sb->s_fs_info)->m_vfsmount = 
>> mnt;
> 
> As I previously suggested, please use XFS_M(mnt->mnt_sb) for this.

Sorry, missed that one.

Re: [PATCH] xfs_dm_rdwr() needs to pass a vfsmount to dentry_open()

[David Chinner]

On Fri, May 16, 2008 at 12:14:55PM +1000, Lachlan McIlroy wrote:
> David Chinner wrote:
> >On Fri, May 16, 2008 at 11:30:07AM +1000, Lachlan McIlroy wrote:
> >>There's a new check added to dentry_open():
> >>
> >>   if (!mnt) {
> >>       printk(KERN_WARNING "%s called with NULL vfsmount\n", __func__);
> >>       dump_stack();
> >>       return ERR_PTR(-EINVAL);
> >>   }
> >>
> >>We need to pass a mountpoint in the call to dentry_open() in xfs_dm_rdwr()
> >>to avoid this code.
> >>
> >>--- fs/xfs/dmapi/xfs_dm.c_1.72	2008-05-15 13:23:41.000000000 +1000
> >>+++ fs/xfs/dmapi/xfs_dm.c	2008-05-15 17:51:41.000000000 +1000
> >>@@ -52,6 +52,8 @@
> >>#include <dmapi_kern.h>
> >>#include "xfs_dm.h"
> >>
> >>+#include <linux/mount.h>
> >
> >Should be defined in fs/xfs/linux-2.6/xfs_linux.h, right?
> It's not currently in xfs_linux.h.  It's needed for mntget().  I saw that
> xfs_ioctl.c uses mntget() and pulls in linux/mount.h directly so I followed
> suit.  I can add it to xfs_linux.h if you prefer.

Typically we include linux specific headers in code outside fs/xfs/linux-2.6
via an implicit include (xfs.h pulls in linux-2.6/xfs_linux.h) to try and
keep platform specific headers out of the core code. Whether that rule applies
to dmapi is a good question - the dmapi code needs so much cleaning up
anyway I'm not sure it matters.....

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

Re: [PATCH] xfs_dm_rdwr() needs to pass a vfsmount to dentry_open()

[Christoph Hellwig]

On Fri, May 16, 2008 at 11:30:07AM +1000, Lachlan McIlroy wrote:
> --- fs/xfs/xfs_mount.h_1.265	2008-05-15 13:23:49.000000000 +1000
> +++ fs/xfs/xfs_mount.h	2008-05-15 13:21:50.000000000 +1000
> @@ -337,6 +337,7 @@ typedef struct xfs_mount {
> 	spinlock_t		m_sync_lock;	/* work item list lock */
> 	int			m_sync_seq;	/* sync thread generation no. */
> 	wait_queue_head_t	m_wait_single_sync_task;
> +	struct vfsmount		*m_vfsmount;
> } xfs_mount_t;

there can be multiple vfsmounts for a single xfs_mount.  In addition to
that please don't mess up core xfs for dmapi braindamage.