headers
Stephen Rothwell | 1 Aug 2011 03:01
Picon
Picon

Re: Please pull NFS client changes

On Mon, 1 Aug 2011 10:57:39 +1000 Stephen Rothwell <sfr@...> wrote:
>
> I have applied this the linux-next today ..

And then I checked Linus' tree again to find it already there, so I have
started again from the his newer tree.

--

-- 
Cheers,
Stephen Rothwell                    sfr@...
http://www.canb.auug.org.au/~sfr/
Permalink | Reply | 0 comments |
headers
Sachin Prabhu | 1 Aug 2011 13:10
Picon
Favicon

[PATCH] nfs: Do not allow multiple mounts on same mountpoint when using -o noac

Do not allow multiple mounts on same mountpoint when using -o noac

When you normally attempt to mount a share twice on the same mountpoint,
a check in do_add_mount causes it to return an error

# mount localhost:/nfsv3 /mnt
# mount localhost:/nfsv3 /mnt
mount.nfs: /mnt is already mounted or busy

However when using the option 'noac', the user is able to mount the same
share on the same mountpoint multiple times. This happens because a
share mounted with the noac option is automatically assigned the 'sync'
flag MS_SYNCHRONOUS in nfs_initialise_sb(). This flag is set after the
check for already existing superblocks is done in sget(). The check for
the mount flags in nfs_compare_mount_options() does not take into
account the 'sync' flag applied later on in the code path. This means
that when using 'noac', a new superblock structure is assigned for every
new mount of the same share and multiple shares on the same mountpoint
are allowed.

ie. 
# mount -onoac localhost:/nfsv3 /mnt
can be run multiple times.

The patch checks for noac and assigns the sync flag before sget() is
called to obtain an already existing superblock structure.

Signed-off-by: Sachin Prabhu <sprabhu@...>
Reviewed-by: Jeff Layton <jlayton@...>
(Continue reading)

Permalink | Reply | 2 comments |
headers
Sachin Prabhu | 1 Aug 2011 13:13
Picon
Favicon

Re: [PATCH] nfs: Do not allow multiple mounts on same mountpoint when using -o noac

On Sat, 2011-07-30 at 14:33 -0400, Trond Myklebust wrote:
> On Fri, 2011-07-29 at 14:50 +0100, Sachin Prabhu wrote: 
> > When you normally attempt to mount a share twice on the same mountpoint,
> > a check in do_add_mount causes it to return an error
> > 
> > # mount localhost:/nfsv3 /mnt
> > # mount localhost:/nfsv3 /mnt
> > mount.nfs: /mnt is already mounted or busy
> > 
> > However when using the option 'noac', the user is able to mount the same
> > share on the same mountpoint multiple times. This happens because a
> > share mounted with the noac option is automatically assigned the 'sync'
> > flag MS_SYNCHRONOUS in nfs_initialise_sb(). This flag is set after the
> > check for already existing superblocks is done in sget(). The check for
> > the mount flags in nfs_compare_mount_options() does not take into
> > account the 'sync' flag applied later on in the code path. This means
> > that when using 'noac', a new superblock structure is assigned for every
> > new mount of the same share and multiple shares on the same mountpoint
> > are allowed.
> > 
> > ie. 
> > # mount -onoac localhost:/nfsv3 /mnt
> > can be run multiple times.
> > 
> > The patch checks for noac and assigns the sync flag before sget() is
> > called to obtain an already existing superblock structure.
> > 
> > 
> > Signed-off-by: Sachin Prabhu <sprabhu@...>
>
(Continue reading)

Permalink | Reply | 0 comments |
headers
Christoph Hellwig | 1 Aug 2011 13:19
Favicon

Re: [PATCH] nfs: Do not allow multiple mounts on same mountpoint when using -o noac

On Mon, Aug 01, 2011 at 12:10:12PM +0100, Sachin Prabhu wrote:
> Do not allow multiple mounts on same mountpoint when using -o noac

The patch content really doesn't seem to match the subject line and
most of the content.

> However when using the option 'noac', the user is able to mount the same
> share on the same mountpoint multiple times. This happens because a
> share mounted with the noac option is automatically assigned the 'sync'
> flag MS_SYNCHRONOUS in nfs_initialise_sb(). This flag is set after the
> check for already existing superblocks is done in sget(). The check for
> the mount flags in nfs_compare_mount_options() does not take into
> account the 'sync' flag applied later on in the code path. This means
> that when using 'noac', a new superblock structure is assigned for every
> new mount of the same share and multiple shares on the same mountpoint
> are allowed.
> 
> ie. 
> # mount -onoac localhost:/nfsv3 /mnt
> can be run multiple times.

> The patch checks for noac and assigns the sync flag before sget() is
> called to obtain an already existing superblock structure.

That's a fine fix, but the the whole patch subject and description
focusses on a side effect rather than the underlying real problem.

The underlying issue is that youwant to share superblocks when
having multiple mounts with -o noac, which requires assigning the
sync flag ealier.
(Continue reading)

Permalink | Reply | 1 comment |
headers
Sachin Prabhu | 1 Aug 2011 15:29
Picon
Favicon

Re: [PATCH] nfs: Do not allow multiple mounts on same mountpoint when using -o noac

On Mon, 2011-08-01 at 07:19 -0400, Christoph Hellwig wrote:
> On Mon, Aug 01, 2011 at 12:10:12PM +0100, Sachin Prabhu wrote:
> > Do not allow multiple mounts on same mountpoint when using -o noac
> 
> The patch content really doesn't seem to match the subject line and
> most of the content.
> 
> > However when using the option 'noac', the user is able to mount the same
> > share on the same mountpoint multiple times. This happens because a
> > share mounted with the noac option is automatically assigned the 'sync'
> > flag MS_SYNCHRONOUS in nfs_initialise_sb(). This flag is set after the
> > check for already existing superblocks is done in sget(). The check for
> > the mount flags in nfs_compare_mount_options() does not take into
> > account the 'sync' flag applied later on in the code path. This means
> > that when using 'noac', a new superblock structure is assigned for every
> > new mount of the same share and multiple shares on the same mountpoint
> > are allowed.
> > 
> > ie. 
> > # mount -onoac localhost:/nfsv3 /mnt
> > can be run multiple times.
> 
> > The patch checks for noac and assigns the sync flag before sget() is
> > called to obtain an already existing superblock structure.
> 
> That's a fine fix, but the the whole patch subject and description
> focusses on a side effect rather than the underlying real problem.
> 
> The underlying issue is that youwant to share superblocks when
> having multiple mounts with -o noac, which requires assigning the
(Continue reading)

Permalink | Reply | 0 comments |
headers
Adrien Kunysz | 1 Aug 2011 15:54
Favicon

using non contiguous memory in nfs_idmap_new()

Dear linux-nfs,

I have a box that has recently been victim of an OOM caused by
nfs_idmap_new() attempting to allocate a large amount of contiguous
memory. This problem has been reported previously here [0] and I
understand the real fix is to use CONFIG_NFS_USE_NEW_IDMAPPER. However
I was wondering whether using non contiguous memory instead would be a
valid workaround, especially considering the new ID mapper is not
enabled by default. I have actually written a patch to that effect as
I don't see any specific reason why that struct would need to be in
contiguous memory.

So before I do anything stupid with that patch,
a) is there any good reason to use kzalloc() instead of vzalloc() in
nfs_idmap_new()?
b) would you consider a patch that would make the function use vzalloc()?

Thanks,
Adrien Kunysz

[0] http://www.spinics.net/lists/linux-nfs/msg22248.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 2 comments |
headers
Trond Myklebust | 1 Aug 2011 16:42
Picon

Re: using non contiguous memory in nfs_idmap_new()

On Mon, 2011-08-01 at 14:54 +0100, Adrien Kunysz wrote: 
> Dear linux-nfs,
> 
> I have a box that has recently been victim of an OOM caused by
> nfs_idmap_new() attempting to allocate a large amount of contiguous
> memory. This problem has been reported previously here [0] and I
> understand the real fix is to use CONFIG_NFS_USE_NEW_IDMAPPER. However
> I was wondering whether using non contiguous memory instead would be a
> valid workaround, especially considering the new ID mapper is not
> enabled by default. I have actually written a patch to that effect as
> I don't see any specific reason why that struct would need to be in
> contiguous memory.
> 
> So before I do anything stupid with that patch,
> a) is there any good reason to use kzalloc() instead of vzalloc() in
> nfs_idmap_new()?

vmalloc memory is a limited resource too.

> b) would you consider a patch that would make the function use vzalloc()?

Is there any reason why you can't use the new idmapper now that it has
been integrated into the upstream nfs-utils? There are so many
scalability issues with the old idmapper that I'd rather deprecate its
use as quickly as possible.

--

-- 
Trond Myklebust
Linux NFS client maintainer
(Continue reading)

Permalink | Reply | 1 comment |
headers
Adrien Kunysz | 1 Aug 2011 18:03
Favicon

Re: using non contiguous memory in nfs_idmap_new()

On Mon, Aug 1, 2011 at 3:42 PM, Trond Myklebust
<Trond.Myklebust@...> wrote:
> On Mon, 2011-08-01 at 14:54 +0100, Adrien Kunysz wrote:
>> Dear linux-nfs,
>>
>> I have a box that has recently been victim of an OOM caused by
>> nfs_idmap_new() attempting to allocate a large amount of contiguous
>> memory. This problem has been reported previously here [0] and I
>> understand the real fix is to use CONFIG_NFS_USE_NEW_IDMAPPER. However
>> I was wondering whether using non contiguous memory instead would be a
>> valid workaround, especially considering the new ID mapper is not
>> enabled by default. I have actually written a patch to that effect as
>> I don't see any specific reason why that struct would need to be in
>> contiguous memory.
>>
>> So before I do anything stupid with that patch,
>> a) is there any good reason to use kzalloc() instead of vzalloc() in
>> nfs_idmap_new()?
>
> vmalloc memory is a limited resource too.

It certainly is but from my understanding it is much less prone to run
into problem when the memory is fragmented (which is my problem here).
I don't have a problem with how much memory it uses but with the size
of the contiguous allocation.

>> b) would you consider a patch that would make the function use vzalloc()?
>
> Is there any reason why you can't use the new idmapper now that it has
> been integrated into the upstream nfs-utils? There are so many
(Continue reading)

Permalink | Reply | 0 comments |
headers
Matthew Treinish | 1 Aug 2011 21:02
Picon

[PATCH 2/2] mountd: Removed duplicate check from insert_groups

Upon further inspection of mountd the duplicate check in insert group is not
needed. It seems that export_read() already filters out duplicates so the
check for duplicates again in insert groups isn't needed.

Signed-off-by: Matthew Treinish <treinish@...>
---
 utils/mountd/mountd.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index bcf5080..9c27d6c 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
 <at>  <at>  -631,11 +631,6  <at>  <at>  static exportnode *lookup_or_create_elist_entry(exports *elist, nfs_export *exp)
 static void insert_group(struct exportnode *e, char *newname)
 {
 	struct groupnode *g;
-
-	for (g = e->ex_groups; g; g = g->gr_next)
-		if (!strcmp(g->gr_name, newname))
-			return;
-
 	g = xmalloc(sizeof(*g));
 	g->gr_name = xstrdup(newname);
 	g->gr_next = e->ex_groups;
--

-- 
1.7.4.4

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
(Continue reading)

Permalink | Reply | 1 comment |
headers
Matthew Treinish | 1 Aug 2011 21:02
Picon

[PATCH 1/2] mountd: Fixed strcmp usage in in insert groups.

Fixed the usage of strcmp in the duplicate check in insert groups. Fixes an
issue with showmount and other commands that required the group information.

Signed-off-by: Matthew Treinish <treinish@...>
---
 utils/mountd/mountd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index 035624c..bcf5080 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
 <at>  <at>  -633,7 +633,7  <at>  <at>  static void insert_group(struct exportnode *e, char *newname)
 	struct groupnode *g;

 	for (g = e->ex_groups; g; g = g->gr_next)
-		if (strcmp(g->gr_name, newname))
+		if (!strcmp(g->gr_name, newname))
 			return;

 	g = xmalloc(sizeof(*g));
--

-- 
1.7.4.4

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

Permalink | Reply | 3 comments |

Gmane