Re: [PATCH] [RFC] vfs: don't revalidate dentries that serve as mountpoints

2013-11-05 Thread Jeff Layton
On Mon, 4 Nov 2013 21:00:23 -0600
Shirish Pargaonkar  wrote:

> For a similar issue in cifs vfs (samba bugzilla 8950), I was going to try
> unset the bit DCACHE_OP_REVALIDATE of d_flags of the
> dentry.  Would something like work for the mountpoint dentry?
> 
> 

I don't think that's the same issue at all. The problem there is that
you need to instantiate a dentry for the root of the mount on the
client, when you don't have any information about the parents.

In this case, we have dentries that are acting as mountpoints that were
getting invalidated.

> 
> On Mon, Nov 4, 2013 at 7:16 PM, Jeff Layton  wrote:
> > We had a couple of reports of people that are mounting NFS filesystems,
> > and then bind mounting certain local files onto dentries in that nfs
> > mount (sort of like a poor-man's unionmount).
> >
> > This all works well until the dentry serving as the mountpoint fails
> > d_revalidate. The dentry will end up being invalidated which makes the
> > bind mount unreachable via pathwalk.
> >
> > It doesn't make much sense to me to allow dentries to serve as
> > mountpoints to end up invalidated, so there's no real point in
> > attempting to d_revalidate them at all.
> >
> > Reported-by: Patrick McLean 
> > Reported-by: Guang Cheng Li 
> > Signed-off-by: Jeff Layton 
> > ---
> >  fs/namei.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/namei.c b/fs/namei.c
> > index caa2805..5b10ad0 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -585,6 +585,9 @@ drop_root_mnt:
> >
> >  static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
> >  {
> > +   /* dentries that serve as mountpoints are always considered valid */
> > +   if (d_mountpoint(dentry))
> > +   return 1;
> > return dentry->d_op->d_revalidate(dentry, flags);
> >  }
> >
> > --
> > 1.8.3.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Jeff Layton 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] vfs: don't revalidate dentries that serve as mountpoints

2013-11-05 Thread Jeff Layton
On Tue, 5 Nov 2013 13:39:59 +1100
NeilBrown  wrote:

> On Mon,  4 Nov 2013 20:16:26 -0500 Jeff Layton  wrote:
> 
> > We had a couple of reports of people that are mounting NFS filesystems,
> > and then bind mounting certain local files onto dentries in that nfs
> > mount (sort of like a poor-man's unionmount).
> > 
> > This all works well until the dentry serving as the mountpoint fails
> > d_revalidate. The dentry will end up being invalidated which makes the
> > bind mount unreachable via pathwalk.
> > 
> > It doesn't make much sense to me to allow dentries to serve as
> > mountpoints to end up invalidated, so there's no real point in
> > attempting to d_revalidate them at all.
> > 
> > Reported-by: Patrick McLean 
> > Reported-by: Guang Cheng Li 
> > Signed-off-by: Jeff Layton 
> > ---
> >  fs/namei.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/namei.c b/fs/namei.c
> > index caa2805..5b10ad0 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -585,6 +585,9 @@ drop_root_mnt:
> >  
> >  static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
> >  {
> > +   /* dentries that serve as mountpoints are always considered valid */
> > +   if (d_mountpoint(dentry))
> > +   return 1;
> > return dentry->d_op->d_revalidate(dentry, flags);
> >  }
> >  
> 
> Makes sense to me. But what about the parent or grandparent of the mountpoint?
> 
> Presumably we don't want to invalidate them either, and I don't think
> d_mountpoint() returns true for those, does it?
> 
> NeilBrown

Good point...

I think we're probably saved there (at least on NFS) by the
check_submounts_and_drop() call in nfs_lookup_revalidate.

Hmmm, that said...I did do most of this testing on a slightly older
kernel that doesn't have that call, and I can't reproduce this problem
on a 3.12-ish kernel. I think this is probably already fixed by Miklos'
patches that added that.

Sorry for the noise!
-- 
Jeff Layton 


signature.asc
Description: PGP signature


Re: [PATCH] [RFC] vfs: don't revalidate dentries that serve as mountpoints

2013-11-05 Thread Christoph Hellwig
On Mon, Nov 04, 2013 at 09:00:23PM -0600, Shirish Pargaonkar wrote:
> For a similar issue in cifs vfs (samba bugzilla 8950), I was going to try
> unset the bit DCACHE_OP_REVALIDATE of d_flags of the
> dentry.  Would something like work for the mountpoint dentry?

The DCACHE_OP_REVALIDATE are just caches of the available dentry
operations, please don't manipulate those flags on their own.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] vfs: don't revalidate dentries that serve as mountpoints

2013-11-05 Thread Christoph Hellwig
On Mon, Nov 04, 2013 at 09:00:23PM -0600, Shirish Pargaonkar wrote:
 For a similar issue in cifs vfs (samba bugzilla 8950), I was going to try
 unset the bit DCACHE_OP_REVALIDATE of d_flags of the
 dentry.  Would something like work for the mountpoint dentry?

The DCACHE_OP_REVALIDATE are just caches of the available dentry
operations, please don't manipulate those flags on their own.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] vfs: don't revalidate dentries that serve as mountpoints

2013-11-05 Thread Jeff Layton
On Tue, 5 Nov 2013 13:39:59 +1100
NeilBrown ne...@suse.de wrote:

 On Mon,  4 Nov 2013 20:16:26 -0500 Jeff Layton jlay...@redhat.com wrote:
 
  We had a couple of reports of people that are mounting NFS filesystems,
  and then bind mounting certain local files onto dentries in that nfs
  mount (sort of like a poor-man's unionmount).
  
  This all works well until the dentry serving as the mountpoint fails
  d_revalidate. The dentry will end up being invalidated which makes the
  bind mount unreachable via pathwalk.
  
  It doesn't make much sense to me to allow dentries to serve as
  mountpoints to end up invalidated, so there's no real point in
  attempting to d_revalidate them at all.
  
  Reported-by: Patrick McLean chutz...@gentoo.org
  Reported-by: Guang Cheng Li ligua...@cn.ibm.com
  Signed-off-by: Jeff Layton jlay...@redhat.com
  ---
   fs/namei.c | 3 +++
   1 file changed, 3 insertions(+)
  
  diff --git a/fs/namei.c b/fs/namei.c
  index caa2805..5b10ad0 100644
  --- a/fs/namei.c
  +++ b/fs/namei.c
  @@ -585,6 +585,9 @@ drop_root_mnt:
   
   static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
   {
  +   /* dentries that serve as mountpoints are always considered valid */
  +   if (d_mountpoint(dentry))
  +   return 1;
  return dentry-d_op-d_revalidate(dentry, flags);
   }
   
 
 Makes sense to me. But what about the parent or grandparent of the mountpoint?
 
 Presumably we don't want to invalidate them either, and I don't think
 d_mountpoint() returns true for those, does it?
 
 NeilBrown

Good point...

I think we're probably saved there (at least on NFS) by the
check_submounts_and_drop() call in nfs_lookup_revalidate.

Hmmm, that said...I did do most of this testing on a slightly older
kernel that doesn't have that call, and I can't reproduce this problem
on a 3.12-ish kernel. I think this is probably already fixed by Miklos'
patches that added that.

Sorry for the noise!
-- 
Jeff Layton jlay...@redhat.com


signature.asc
Description: PGP signature


Re: [PATCH] [RFC] vfs: don't revalidate dentries that serve as mountpoints

2013-11-05 Thread Jeff Layton
On Mon, 4 Nov 2013 21:00:23 -0600
Shirish Pargaonkar shirishpargaon...@gmail.com wrote:

 For a similar issue in cifs vfs (samba bugzilla 8950), I was going to try
 unset the bit DCACHE_OP_REVALIDATE of d_flags of the
 dentry.  Would something like work for the mountpoint dentry?
 
 

I don't think that's the same issue at all. The problem there is that
you need to instantiate a dentry for the root of the mount on the
client, when you don't have any information about the parents.

In this case, we have dentries that are acting as mountpoints that were
getting invalidated.

 
 On Mon, Nov 4, 2013 at 7:16 PM, Jeff Layton jlay...@redhat.com wrote:
  We had a couple of reports of people that are mounting NFS filesystems,
  and then bind mounting certain local files onto dentries in that nfs
  mount (sort of like a poor-man's unionmount).
 
  This all works well until the dentry serving as the mountpoint fails
  d_revalidate. The dentry will end up being invalidated which makes the
  bind mount unreachable via pathwalk.
 
  It doesn't make much sense to me to allow dentries to serve as
  mountpoints to end up invalidated, so there's no real point in
  attempting to d_revalidate them at all.
 
  Reported-by: Patrick McLean chutz...@gentoo.org
  Reported-by: Guang Cheng Li ligua...@cn.ibm.com
  Signed-off-by: Jeff Layton jlay...@redhat.com
  ---
   fs/namei.c | 3 +++
   1 file changed, 3 insertions(+)
 
  diff --git a/fs/namei.c b/fs/namei.c
  index caa2805..5b10ad0 100644
  --- a/fs/namei.c
  +++ b/fs/namei.c
  @@ -585,6 +585,9 @@ drop_root_mnt:
 
   static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
   {
  +   /* dentries that serve as mountpoints are always considered valid */
  +   if (d_mountpoint(dentry))
  +   return 1;
  return dentry-d_op-d_revalidate(dentry, flags);
   }
 
  --
  1.8.3.1
 
  --
  To unsubscribe from this list: send the line unsubscribe linux-nfs in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Jeff Layton jlay...@redhat.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] vfs: don't revalidate dentries that serve as mountpoints

2013-11-04 Thread Shirish Pargaonkar
For a similar issue in cifs vfs (samba bugzilla 8950), I was going to try
unset the bit DCACHE_OP_REVALIDATE of d_flags of the
dentry.  Would something like work for the mountpoint dentry?



On Mon, Nov 4, 2013 at 7:16 PM, Jeff Layton  wrote:
> We had a couple of reports of people that are mounting NFS filesystems,
> and then bind mounting certain local files onto dentries in that nfs
> mount (sort of like a poor-man's unionmount).
>
> This all works well until the dentry serving as the mountpoint fails
> d_revalidate. The dentry will end up being invalidated which makes the
> bind mount unreachable via pathwalk.
>
> It doesn't make much sense to me to allow dentries to serve as
> mountpoints to end up invalidated, so there's no real point in
> attempting to d_revalidate them at all.
>
> Reported-by: Patrick McLean 
> Reported-by: Guang Cheng Li 
> Signed-off-by: Jeff Layton 
> ---
>  fs/namei.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/fs/namei.c b/fs/namei.c
> index caa2805..5b10ad0 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -585,6 +585,9 @@ drop_root_mnt:
>
>  static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
>  {
> +   /* dentries that serve as mountpoints are always considered valid */
> +   if (d_mountpoint(dentry))
> +   return 1;
> return dentry->d_op->d_revalidate(dentry, flags);
>  }
>
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] vfs: don't revalidate dentries that serve as mountpoints

2013-11-04 Thread NeilBrown
On Mon,  4 Nov 2013 20:16:26 -0500 Jeff Layton  wrote:

> We had a couple of reports of people that are mounting NFS filesystems,
> and then bind mounting certain local files onto dentries in that nfs
> mount (sort of like a poor-man's unionmount).
> 
> This all works well until the dentry serving as the mountpoint fails
> d_revalidate. The dentry will end up being invalidated which makes the
> bind mount unreachable via pathwalk.
> 
> It doesn't make much sense to me to allow dentries to serve as
> mountpoints to end up invalidated, so there's no real point in
> attempting to d_revalidate them at all.
> 
> Reported-by: Patrick McLean 
> Reported-by: Guang Cheng Li 
> Signed-off-by: Jeff Layton 
> ---
>  fs/namei.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index caa2805..5b10ad0 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -585,6 +585,9 @@ drop_root_mnt:
>  
>  static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
>  {
> + /* dentries that serve as mountpoints are always considered valid */
> + if (d_mountpoint(dentry))
> + return 1;
>   return dentry->d_op->d_revalidate(dentry, flags);
>  }
>  

Makes sense to me. But what about the parent or grandparent of the mountpoint?

Presumably we don't want to invalidate them either, and I don't think
d_mountpoint() returns true for those, does it?

NeilBrown


signature.asc
Description: PGP signature


[PATCH] [RFC] vfs: don't revalidate dentries that serve as mountpoints

2013-11-04 Thread Jeff Layton
We had a couple of reports of people that are mounting NFS filesystems,
and then bind mounting certain local files onto dentries in that nfs
mount (sort of like a poor-man's unionmount).

This all works well until the dentry serving as the mountpoint fails
d_revalidate. The dentry will end up being invalidated which makes the
bind mount unreachable via pathwalk.

It doesn't make much sense to me to allow dentries to serve as
mountpoints to end up invalidated, so there's no real point in
attempting to d_revalidate them at all.

Reported-by: Patrick McLean 
Reported-by: Guang Cheng Li 
Signed-off-by: Jeff Layton 
---
 fs/namei.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/namei.c b/fs/namei.c
index caa2805..5b10ad0 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -585,6 +585,9 @@ drop_root_mnt:
 
 static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
 {
+   /* dentries that serve as mountpoints are always considered valid */
+   if (d_mountpoint(dentry))
+   return 1;
return dentry->d_op->d_revalidate(dentry, flags);
 }
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] [RFC] vfs: don't revalidate dentries that serve as mountpoints

2013-11-04 Thread Jeff Layton
We had a couple of reports of people that are mounting NFS filesystems,
and then bind mounting certain local files onto dentries in that nfs
mount (sort of like a poor-man's unionmount).

This all works well until the dentry serving as the mountpoint fails
d_revalidate. The dentry will end up being invalidated which makes the
bind mount unreachable via pathwalk.

It doesn't make much sense to me to allow dentries to serve as
mountpoints to end up invalidated, so there's no real point in
attempting to d_revalidate them at all.

Reported-by: Patrick McLean chutz...@gentoo.org
Reported-by: Guang Cheng Li ligua...@cn.ibm.com
Signed-off-by: Jeff Layton jlay...@redhat.com
---
 fs/namei.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/namei.c b/fs/namei.c
index caa2805..5b10ad0 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -585,6 +585,9 @@ drop_root_mnt:
 
 static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
 {
+   /* dentries that serve as mountpoints are always considered valid */
+   if (d_mountpoint(dentry))
+   return 1;
return dentry-d_op-d_revalidate(dentry, flags);
 }
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] [RFC] vfs: don't revalidate dentries that serve as mountpoints

2013-11-04 Thread NeilBrown
On Mon,  4 Nov 2013 20:16:26 -0500 Jeff Layton jlay...@redhat.com wrote:

 We had a couple of reports of people that are mounting NFS filesystems,
 and then bind mounting certain local files onto dentries in that nfs
 mount (sort of like a poor-man's unionmount).
 
 This all works well until the dentry serving as the mountpoint fails
 d_revalidate. The dentry will end up being invalidated which makes the
 bind mount unreachable via pathwalk.
 
 It doesn't make much sense to me to allow dentries to serve as
 mountpoints to end up invalidated, so there's no real point in
 attempting to d_revalidate them at all.
 
 Reported-by: Patrick McLean chutz...@gentoo.org
 Reported-by: Guang Cheng Li ligua...@cn.ibm.com
 Signed-off-by: Jeff Layton jlay...@redhat.com
 ---
  fs/namei.c | 3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/fs/namei.c b/fs/namei.c
 index caa2805..5b10ad0 100644
 --- a/fs/namei.c
 +++ b/fs/namei.c
 @@ -585,6 +585,9 @@ drop_root_mnt:
  
  static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
  {
 + /* dentries that serve as mountpoints are always considered valid */
 + if (d_mountpoint(dentry))
 + return 1;
   return dentry-d_op-d_revalidate(dentry, flags);
  }
  

Makes sense to me. But what about the parent or grandparent of the mountpoint?

Presumably we don't want to invalidate them either, and I don't think
d_mountpoint() returns true for those, does it?

NeilBrown


signature.asc
Description: PGP signature


Re: [PATCH] [RFC] vfs: don't revalidate dentries that serve as mountpoints

2013-11-04 Thread Shirish Pargaonkar
For a similar issue in cifs vfs (samba bugzilla 8950), I was going to try
unset the bit DCACHE_OP_REVALIDATE of d_flags of the
dentry.  Would something like work for the mountpoint dentry?



On Mon, Nov 4, 2013 at 7:16 PM, Jeff Layton jlay...@redhat.com wrote:
 We had a couple of reports of people that are mounting NFS filesystems,
 and then bind mounting certain local files onto dentries in that nfs
 mount (sort of like a poor-man's unionmount).

 This all works well until the dentry serving as the mountpoint fails
 d_revalidate. The dentry will end up being invalidated which makes the
 bind mount unreachable via pathwalk.

 It doesn't make much sense to me to allow dentries to serve as
 mountpoints to end up invalidated, so there's no real point in
 attempting to d_revalidate them at all.

 Reported-by: Patrick McLean chutz...@gentoo.org
 Reported-by: Guang Cheng Li ligua...@cn.ibm.com
 Signed-off-by: Jeff Layton jlay...@redhat.com
 ---
  fs/namei.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/fs/namei.c b/fs/namei.c
 index caa2805..5b10ad0 100644
 --- a/fs/namei.c
 +++ b/fs/namei.c
 @@ -585,6 +585,9 @@ drop_root_mnt:

  static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
  {
 +   /* dentries that serve as mountpoints are always considered valid */
 +   if (d_mountpoint(dentry))
 +   return 1;
 return dentry-d_op-d_revalidate(dentry, flags);
  }

 --
 1.8.3.1

 --
 To unsubscribe from this list: send the line unsubscribe linux-nfs in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/