diff mbox

NFSD: Add a cache for fs_locations information

Message ID 20110829185115.3413.92459.stgit@matisse.1015granger.net (mailing list archive)
State New, archived
Headers show

Commit Message

Chuck Lever Aug. 29, 2011, 6:51 p.m. UTC 
From: Trond Myklebust <Trond.Myklebust@netapp.com>

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
[ cel: since this is server-side, use nfsd4_ prefix instead of nfs4_ prefix. ]
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 fs/nfsd/nfsd.h |    7 +++++++
 fs/nfsd/vfs.c  |   15 +++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jeff Layton Aug. 30, 2011, 7:18 p.m. UTC | #1 
On Mon, 29 Aug 2011 14:51:15 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> [ cel: since this is server-side, use nfsd4_ prefix instead of nfs4_ prefix. ]
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  fs/nfsd/nfsd.h |    7 +++++++
>  fs/nfsd/vfs.c  |   15 +++++++++++++++
>  2 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 7ecfa24..d314812 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -335,6 +335,13 @@ static inline u32 nfsd_suppattrs2(u32 minorversion)
>  #define NFSD_SUPPATTR_EXCLCREAT_WORD2 \
>  	NFSD_WRITEABLE_ATTRS_WORD2
>  
> +extern int nfsd4_is_junction(struct dentry *dentry);
> +#else
> +static inline int nfsd4_is_junction(struct dentry *dentry)
> +{
> +	return 0;
> +}
> +
>  #endif /* CONFIG_NFSD_V4 */
>  
>  #endif /* LINUX_NFSD_NFSD_H */
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index fd0acca..5dac667 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -168,6 +168,8 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
>  {
>  	if (d_mountpoint(dentry))
>  		return 1;
> +	if (nfsd4_is_junction(dentry))
> +		return 1;
>  	if (!(exp->ex_flags & NFSEXP_V4ROOT))
>  		return 0;
>  	return dentry->d_inode != NULL;
> @@ -592,6 +594,19 @@ nfsd4_get_nfs4_acl(struct svc_rqst *rqstp, struct dentry *dentry, struct nfs4_ac
>  	return error;
>  }
>  
> +#define NFSD_XATTR_JUNCTION_PREFIX XATTR_TRUSTED_PREFIX "junction."
> +#define NFSD_XATTR_JUNCTION_TYPE NFSD_XATTR_JUNCTION_PREFIX "type"
> +int nfsd4_is_junction(struct dentry *dentry)
> +{
> +	ssize_t ret;
> +
> +	if (dentry->d_inode != NULL) {
> +		ret = vfs_getxattr(dentry, NFSD_XATTR_JUNCTION_TYPE, NULL, 0);
> +		if (ret > 0)
> +			return 1;
> +	}
> +	return 0;
> +}

Won't the above check be rather expensive? You'll need to do a
getxattr call on almost every path component of every lookup, right?

I may be misremembering your talk from connectathon, but I thought you
were planning to use a well-known mode for junctions that would cut
down on the number of unnecessary getxattrs...

>  #endif /* defined(CONFIG_NFSD_V4) */
>  
>  #ifdef CONFIG_NFSD_V3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chuck Lever Aug. 30, 2011, 8 p.m. UTC | #2 
On Aug 30, 2011, at 3:18 PM, Jeff Layton wrote:

> On Mon, 29 Aug 2011 14:51:15 -0400
> Chuck Lever <chuck.lever@oracle.com> wrote:
> 
>> From: Trond Myklebust <Trond.Myklebust@netapp.com>
>> 
>> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
>> [ cel: since this is server-side, use nfsd4_ prefix instead of nfs4_ prefix. ]
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> 
>> fs/nfsd/nfsd.h |    7 +++++++
>> fs/nfsd/vfs.c  |   15 +++++++++++++++
>> 2 files changed, 22 insertions(+), 0 deletions(-)
>> 
>> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
>> index 7ecfa24..d314812 100644
>> --- a/fs/nfsd/nfsd.h
>> +++ b/fs/nfsd/nfsd.h
>> @@ -335,6 +335,13 @@ static inline u32 nfsd_suppattrs2(u32 minorversion)
>> #define NFSD_SUPPATTR_EXCLCREAT_WORD2 \
>> 	NFSD_WRITEABLE_ATTRS_WORD2
>> 
>> +extern int nfsd4_is_junction(struct dentry *dentry);
>> +#else
>> +static inline int nfsd4_is_junction(struct dentry *dentry)
>> +{
>> +	return 0;
>> +}
>> +
>> #endif /* CONFIG_NFSD_V4 */
>> 
>> #endif /* LINUX_NFSD_NFSD_H */
>> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
>> index fd0acca..5dac667 100644
>> --- a/fs/nfsd/vfs.c
>> +++ b/fs/nfsd/vfs.c
>> @@ -168,6 +168,8 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
>> {
>> 	if (d_mountpoint(dentry))
>> 		return 1;
>> +	if (nfsd4_is_junction(dentry))
>> +		return 1;
>> 	if (!(exp->ex_flags & NFSEXP_V4ROOT))
>> 		return 0;
>> 	return dentry->d_inode != NULL;
>> @@ -592,6 +594,19 @@ nfsd4_get_nfs4_acl(struct svc_rqst *rqstp, struct dentry *dentry, struct nfs4_ac
>> 	return error;
>> }
>> 
>> +#define NFSD_XATTR_JUNCTION_PREFIX XATTR_TRUSTED_PREFIX "junction."
>> +#define NFSD_XATTR_JUNCTION_TYPE NFSD_XATTR_JUNCTION_PREFIX "type"
>> +int nfsd4_is_junction(struct dentry *dentry)
>> +{
>> +	ssize_t ret;
>> +
>> +	if (dentry->d_inode != NULL) {
>> +		ret = vfs_getxattr(dentry, NFSD_XATTR_JUNCTION_TYPE, NULL, 0);
>> +		if (ret > 0)
>> +			return 1;
>> +	}
>> +	return 0;
>> +}
> 
> Won't the above check be rather expensive? You'll need to do a
> getxattr call on almost every path component of every lookup, right?
> 
> I may be misremembering your talk from connectathon, but I thought you
> were planning to use a well-known mode for junctions that would cut
> down on the number of unnecessary getxattrs...

Yes, that's the plan.  To reduce overhead, the S_ISVTX bit must be set before NFSD does the expensive xattr test.  However, I don't think this kind of filtering was ever implemented.  I got the patch from here:

  http://git.linux-nfs.org/?p=trondmy/nfs-2.6.git;a=shortlog;h=refs/heads/fedfs-for-2.6.34

and that doesn't seem to have it either.  I can implement something and repost these.

>> #endif /* defined(CONFIG_NFSD_V4) */
>> 
>> #ifdef CONFIG_NFSD_V3
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> -- 
> Jeff Layton <jlayton@poochiereds.net>
Boaz Harrosh Aug. 30, 2011, 8:24 p.m. UTC | #3 
On 08/30/2011 01:00 PM, Chuck Lever wrote:
<>
>> 
>> Won't the above check be rather expensive? You'll need to do a 
>> getxattr call on almost every path component of every lookup,
>> right?
>> 
>> I may be misremembering your talk from connectathon, but I thought
>> you were planning to use a well-known mode for junctions that would
>> cut down on the number of unnecessary getxattrs...
> 
> Yes, that's the plan.  To reduce overhead, the S_ISVTX bit must be
> set before NFSD does the expensive xattr test.  

from: stat(2) - Linux man page

The 'sticky' bit (S_ISVTX) on a directory means that a file in that
directory can be renamed or deleted only by the owner of the file, 
by the owner of the directory, and by a privileged process.

Please explain how does it work? Once the junction is followed and
mounted then the mode-bits get changed to the destination directory's
mode bits? So the Server's junction mode-bits are never exposed, except
in a local-fs file access on the server?

Thanks
Boaz

> However, I don't
> think this kind of filtering was ever implemented.  I got the patch
> from here:
> 
> http://git.linux-nfs.org/?p=trondmy/nfs-2.6.git;a=shortlog;h=refs/heads/fedfs-for-2.6.34
>
>  and that doesn't seem to have it either.  I can implement something
> and repost these.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Trond Myklebust Aug. 30, 2011, 8:34 p.m. UTC | #4 
On Tue, 2011-08-30 at 13:24 -0700, Boaz Harrosh wrote: 
> On 08/30/2011 01:00 PM, Chuck Lever wrote:
> <>
> >> 
> >> Won't the above check be rather expensive? You'll need to do a 
> >> getxattr call on almost every path component of every lookup,
> >> right?
> >> 
> >> I may be misremembering your talk from connectathon, but I thought
> >> you were planning to use a well-known mode for junctions that would
> >> cut down on the number of unnecessary getxattrs...
> > 
> > Yes, that's the plan.  To reduce overhead, the S_ISVTX bit must be
> > set before NFSD does the expensive xattr test.  

...and mode bits otherwise set to 0 so nobody can access the mounted-on
directory.

> from: stat(2) - Linux man page
> 
> The 'sticky' bit (S_ISVTX) on a directory means that a file in that
> directory can be renamed or deleted only by the owner of the file, 
> by the owner of the directory, and by a privileged process.
> 
> Please explain how does it work? Once the junction is followed and
> mounted then the mode-bits get changed to the destination directory's
> mode bits? So the Server's junction mode-bits are never exposed, except
> in a local-fs file access on the server?

Yes.
Boaz Harrosh Aug. 30, 2011, 9:02 p.m. UTC | #5 
On 08/30/2011 01:34 PM, Trond Myklebust wrote:
> On Tue, 2011-08-30 at 13:24 -0700, Boaz Harrosh wrote: 
>> On 08/30/2011 01:00 PM, Chuck Lever wrote:
>> <>
>>>>
>>>> Won't the above check be rather expensive? You'll need to do a 
>>>> getxattr call on almost every path component of every lookup,
>>>> right?
>>>>
>>>> I may be misremembering your talk from connectathon, but I thought
>>>> you were planning to use a well-known mode for junctions that would
>>>> cut down on the number of unnecessary getxattrs...
>>>
>>> Yes, that's the plan.  To reduce overhead, the S_ISVTX bit must be
>>> set before NFSD does the expensive xattr test.  
> 
> ...and mode bits otherwise set to 0 so nobody can access the mounted-on
> directory.
> 
>> from: stat(2) - Linux man page
>>
>> The 'sticky' bit (S_ISVTX) on a directory means that a file in that
>> directory can be renamed or deleted only by the owner of the file, 
>> by the owner of the directory, and by a privileged process.
>>
>> Please explain how does it work? Once the junction is followed and
>> mounted then the mode-bits get changed to the destination directory's
>> mode bits? So the Server's junction mode-bits are never exposed, except
>> in a local-fs file access on the server?
> 
> Yes.
> 
Nice trick. Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 7ecfa24..d314812 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -335,6 +335,13 @@  static inline u32 nfsd_suppattrs2(u32 minorversion)
 #define NFSD_SUPPATTR_EXCLCREAT_WORD2 \
 	NFSD_WRITEABLE_ATTRS_WORD2
 
+extern int nfsd4_is_junction(struct dentry *dentry);
+#else
+static inline int nfsd4_is_junction(struct dentry *dentry)
+{
+	return 0;
+}
+
 #endif /* CONFIG_NFSD_V4 */
 
 #endif /* LINUX_NFSD_NFSD_H */
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index fd0acca..5dac667 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -168,6 +168,8 @@  int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
 {
 	if (d_mountpoint(dentry))
 		return 1;
+	if (nfsd4_is_junction(dentry))
+		return 1;
 	if (!(exp->ex_flags & NFSEXP_V4ROOT))
 		return 0;
 	return dentry->d_inode != NULL;
@@ -592,6 +594,19 @@  nfsd4_get_nfs4_acl(struct svc_rqst *rqstp, struct dentry *dentry, struct nfs4_ac
 	return error;
 }
 
+#define NFSD_XATTR_JUNCTION_PREFIX XATTR_TRUSTED_PREFIX "junction."
+#define NFSD_XATTR_JUNCTION_TYPE NFSD_XATTR_JUNCTION_PREFIX "type"
+int nfsd4_is_junction(struct dentry *dentry)
+{
+	ssize_t ret;
+
+	if (dentry->d_inode != NULL) {
+		ret = vfs_getxattr(dentry, NFSD_XATTR_JUNCTION_TYPE, NULL, 0);
+		if (ret > 0)
+			return 1;
+	}
+	return 0;
+}
 #endif /* defined(CONFIG_NFSD_V4) */
 
 #ifdef CONFIG_NFSD_V3