[PATCH 1/3]] cifs: Add idmap data structures and defines (try #3)

[PATCH 1/3]] cifs: Add idmap data structures and defines (try #3)

[shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w]

From: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

A structure stored an rb tree is defined consisting of a SID and
a id (either uid or gid) to which that SID is mapped.

Added fields needed to store this defined data structures of a
rb tree to a superblock and initialized them.

There are two separate trees, one for SID/uid and another one for SID/gid.


Signed-off-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/cifs_fs_sb.h |    4 ++++
 fs/cifs/cifsacl.h    |    6 ++++++
 fs/cifs/cifsfs.c     |    6 ++++++
 3 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
index ac51cd2..4bd0680 100644
--- a/fs/cifs/cifs_fs_sb.h
+++ b/fs/cifs/cifs_fs_sb.h
@@ -45,6 +45,10 @@
 struct cifs_sb_info {
 	struct rb_root tlink_tree;
 	spinlock_t tlink_tree_lock;
+	struct rb_root uidtree;
+	spinlock_t siduidlock;
+	struct rb_root gidtree;
+	spinlock_t sidgidlock;
 	struct tcon_link *master_tlink;
 	struct nls_table *local_nls;
 	unsigned int rsize;
diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h
index c4ae7d0..6c26e7f 100644
--- a/fs/cifs/cifsacl.h
+++ b/fs/cifs/cifsacl.h
@@ -74,6 +74,12 @@ struct cifs_wksid {
 	char sidname[SIDNAMELENGTH];
 } __attribute__((packed));
 
+struct cifs_sid_id {
+	struct rb_node rbnode;
+	struct cifs_sid sid;
+	unsigned long id;
+} __attribute__((packed));
+
 extern int match_sid(struct cifs_sid *);
 extern int compare_sids(const struct cifs_sid *, const struct cifs_sid *);
 
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index a8323f1..36182e3 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -122,6 +122,12 @@ cifs_read_super(struct super_block *sb, void *data,
 	spin_lock_init(&cifs_sb->tlink_tree_lock);
 	cifs_sb->tlink_tree = RB_ROOT;
 
+	spin_lock_init(&cifs_sb->siduidlock);
+	cifs_sb->uidtree = RB_ROOT;
+
+	spin_lock_init(&cifs_sb->sidgidlock);
+	cifs_sb->gidtree = RB_ROOT;
+
 	rc = bdi_setup_and_register(&cifs_sb->bdi, "cifs", BDI_CAP_MAP_COPY);
 	if (rc) {
 		kfree(cifs_sb);
-- 
1.6.0.2

Re: [PATCH 1/3]] cifs: Add idmap data structures and defines (try #3)

[Jeff Layton]

On Fri, 21 Jan 2011 15:44:13 -0600
shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:

> From: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> A structure stored an rb tree is defined consisting of a SID and
> a id (either uid or gid) to which that SID is mapped.
> 
> Added fields needed to store this defined data structures of a
> rb tree to a superblock and initialized them.
> 
> There are two separate trees, one for SID/uid and another one for SID/gid.
> 
> 
> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  fs/cifs/cifs_fs_sb.h |    4 ++++
>  fs/cifs/cifsacl.h    |    6 ++++++
>  fs/cifs/cifsfs.c     |    6 ++++++
>  3 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
> index ac51cd2..4bd0680 100644
> --- a/fs/cifs/cifs_fs_sb.h
> +++ b/fs/cifs/cifs_fs_sb.h
> @@ -45,6 +45,10 @@
>  struct cifs_sb_info {
>  	struct rb_root tlink_tree;
>  	spinlock_t tlink_tree_lock;
> +	struct rb_root uidtree;
> +	spinlock_t siduidlock;
> +	struct rb_root gidtree;
> +	spinlock_t sidgidlock;
>  	struct tcon_link *master_tlink;
>  	struct nls_table *local_nls;
>  	unsigned int rsize;
> diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h
> index c4ae7d0..6c26e7f 100644
> --- a/fs/cifs/cifsacl.h
> +++ b/fs/cifs/cifsacl.h
> @@ -74,6 +74,12 @@ struct cifs_wksid {
>  	char sidname[SIDNAMELENGTH];
>  } __attribute__((packed));
>  
> +struct cifs_sid_id {
> +	struct rb_node rbnode;
> +	struct cifs_sid sid;
> +	unsigned long id;
> +} __attribute__((packed));
> +
>  extern int match_sid(struct cifs_sid *);
>  extern int compare_sids(const struct cifs_sid *, const struct cifs_sid *);
>  
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index a8323f1..36182e3 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -122,6 +122,12 @@ cifs_read_super(struct super_block *sb, void *data,
>  	spin_lock_init(&cifs_sb->tlink_tree_lock);
>  	cifs_sb->tlink_tree = RB_ROOT;
>  
> +	spin_lock_init(&cifs_sb->siduidlock);
> +	cifs_sb->uidtree = RB_ROOT;
> +
> +	spin_lock_init(&cifs_sb->sidgidlock);
> +	cifs_sb->gidtree = RB_ROOT;
> +
>  	rc = bdi_setup_and_register(&cifs_sb->bdi, "cifs", BDI_CAP_MAP_COPY);
>  	if (rc) {
>  		kfree(cifs_sb);

Why do you need per-sb caches? The upcall isn't specific to a
particular superblock, so if you have more than one mount you'll have
to do multiple upcalls for the same SID and have mutiple copies of the
same cifs_sid_id in various caches. It seems like a global cache would
be better.

-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

Re: [PATCH 1/3]] cifs: Add idmap data structures and defines (try #3)

[Shirish Pargaonkar]

On Fri, Jan 21, 2011 at 4:04 PM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> On Fri, 21 Jan 2011 15:44:13 -0600
> shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>
>> From: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> A structure stored an rb tree is defined consisting of a SID and
>> a id (either uid or gid) to which that SID is mapped.
>>
>> Added fields needed to store this defined data structures of a
>> rb tree to a superblock and initialized them.
>>
>> There are two separate trees, one for SID/uid and another one for SID/gid.
>>
>>
>> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  fs/cifs/cifs_fs_sb.h |    4 ++++
>>  fs/cifs/cifsacl.h    |    6 ++++++
>>  fs/cifs/cifsfs.c     |    6 ++++++
>>  3 files changed, 16 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
>> index ac51cd2..4bd0680 100644
>> --- a/fs/cifs/cifs_fs_sb.h
>> +++ b/fs/cifs/cifs_fs_sb.h
>> @@ -45,6 +45,10 @@
>>  struct cifs_sb_info {
>>       struct rb_root tlink_tree;
>>       spinlock_t tlink_tree_lock;
>> +     struct rb_root uidtree;
>> +     spinlock_t siduidlock;
>> +     struct rb_root gidtree;
>> +     spinlock_t sidgidlock;
>>       struct tcon_link *master_tlink;
>>       struct nls_table *local_nls;
>>       unsigned int rsize;
>> diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h
>> index c4ae7d0..6c26e7f 100644
>> --- a/fs/cifs/cifsacl.h
>> +++ b/fs/cifs/cifsacl.h
>> @@ -74,6 +74,12 @@ struct cifs_wksid {
>>       char sidname[SIDNAMELENGTH];
>>  } __attribute__((packed));
>>
>> +struct cifs_sid_id {
>> +     struct rb_node rbnode;
>> +     struct cifs_sid sid;
>> +     unsigned long id;
>> +} __attribute__((packed));
>> +
>>  extern int match_sid(struct cifs_sid *);
>>  extern int compare_sids(const struct cifs_sid *, const struct cifs_sid *);
>>
>> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> index a8323f1..36182e3 100644
>> --- a/fs/cifs/cifsfs.c
>> +++ b/fs/cifs/cifsfs.c
>> @@ -122,6 +122,12 @@ cifs_read_super(struct super_block *sb, void *data,
>>       spin_lock_init(&cifs_sb->tlink_tree_lock);
>>       cifs_sb->tlink_tree = RB_ROOT;
>>
>> +     spin_lock_init(&cifs_sb->siduidlock);
>> +     cifs_sb->uidtree = RB_ROOT;
>> +
>> +     spin_lock_init(&cifs_sb->sidgidlock);
>> +     cifs_sb->gidtree = RB_ROOT;
>> +
>>       rc = bdi_setup_and_register(&cifs_sb->bdi, "cifs", BDI_CAP_MAP_COPY);
>>       if (rc) {
>>               kfree(cifs_sb);
>
> Why do you need per-sb caches? The upcall isn't specific to a
> particular superblock, so if you have more than one mount you'll have
> to do multiple upcalls for the same SID and have mutiple copies of the
> same cifs_sid_id in various caches. It seems like a global cache would
> be better.
>
> --
> Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>

Yes, I will change it to a per server structure instead of a per superblock.

Re: [PATCH 1/3]] cifs: Add idmap data structures and defines (try #3)

[Jeff Layton]

On Fri, 21 Jan 2011 23:49:33 -0600
Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On Fri, Jan 21, 2011 at 4:04 PM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> > On Fri, 21 Jan 2011 15:44:13 -0600
> > shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> >
> >> From: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>
> >> A structure stored an rb tree is defined consisting of a SID and
> >> a id (either uid or gid) to which that SID is mapped.
> >>
> >> Added fields needed to store this defined data structures of a
> >> rb tree to a superblock and initialized them.
> >>
> >> There are two separate trees, one for SID/uid and another one for SID/gid.
> >>
> >>
> >> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> ---
> >>  fs/cifs/cifs_fs_sb.h |    4 ++++
> >>  fs/cifs/cifsacl.h    |    6 ++++++
> >>  fs/cifs/cifsfs.c     |    6 ++++++
> >>  3 files changed, 16 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
> >> index ac51cd2..4bd0680 100644
> >> --- a/fs/cifs/cifs_fs_sb.h
> >> +++ b/fs/cifs/cifs_fs_sb.h
> >> @@ -45,6 +45,10 @@
> >>  struct cifs_sb_info {
> >>       struct rb_root tlink_tree;
> >>       spinlock_t tlink_tree_lock;
> >> +     struct rb_root uidtree;
> >> +     spinlock_t siduidlock;
> >> +     struct rb_root gidtree;
> >> +     spinlock_t sidgidlock;
> >>       struct tcon_link *master_tlink;
> >>       struct nls_table *local_nls;
> >>       unsigned int rsize;
> >> diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h
> >> index c4ae7d0..6c26e7f 100644
> >> --- a/fs/cifs/cifsacl.h
> >> +++ b/fs/cifs/cifsacl.h
> >> @@ -74,6 +74,12 @@ struct cifs_wksid {
> >>       char sidname[SIDNAMELENGTH];
> >>  } __attribute__((packed));
> >>
> >> +struct cifs_sid_id {
> >> +     struct rb_node rbnode;
> >> +     struct cifs_sid sid;
> >> +     unsigned long id;
> >> +} __attribute__((packed));
> >> +
> >>  extern int match_sid(struct cifs_sid *);
> >>  extern int compare_sids(const struct cifs_sid *, const struct cifs_sid *);
> >>
> >> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> >> index a8323f1..36182e3 100644
> >> --- a/fs/cifs/cifsfs.c
> >> +++ b/fs/cifs/cifsfs.c
> >> @@ -122,6 +122,12 @@ cifs_read_super(struct super_block *sb, void *data,
> >>       spin_lock_init(&cifs_sb->tlink_tree_lock);
> >>       cifs_sb->tlink_tree = RB_ROOT;
> >>
> >> +     spin_lock_init(&cifs_sb->siduidlock);
> >> +     cifs_sb->uidtree = RB_ROOT;
> >> +
> >> +     spin_lock_init(&cifs_sb->sidgidlock);
> >> +     cifs_sb->gidtree = RB_ROOT;
> >> +
> >>       rc = bdi_setup_and_register(&cifs_sb->bdi, "cifs", BDI_CAP_MAP_COPY);
> >>       if (rc) {
> >>               kfree(cifs_sb);
> >
> > Why do you need per-sb caches? The upcall isn't specific to a
> > particular superblock, so if you have more than one mount you'll have
> > to do multiple upcalls for the same SID and have mutiple copies of the
> > same cifs_sid_id in various caches. It seems like a global cache would
> > be better.
> >
> > --
> > Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> >
> 
> Yes, I will change it to a per server structure instead of a per superblock.

Why a per-server cache though? Are you passing any information that's
specific to the server to the upcall? It looks to me like all you pass
is the SID. If two servers have files owned by the same SID, is there
any need for a separate upcall and separate cache entries for them?

-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

Re: [PATCH 1/3]] cifs: Add idmap data structures and defines (try #3)

[Shirish Pargaonkar]

On Sat, Jan 22, 2011 at 6:38 AM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> On Fri, 21 Jan 2011 23:49:33 -0600
> Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On Fri, Jan 21, 2011 at 4:04 PM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>> > On Fri, 21 Jan 2011 15:44:13 -0600
>> > shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>> >
>> >> From: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> >>
>> >> A structure stored an rb tree is defined consisting of a SID and
>> >> a id (either uid or gid) to which that SID is mapped.
>> >>
>> >> Added fields needed to store this defined data structures of a
>> >> rb tree to a superblock and initialized them.
>> >>
>> >> There are two separate trees, one for SID/uid and another one for SID/gid.
>> >>
>> >>
>> >> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> >> ---
>> >>  fs/cifs/cifs_fs_sb.h |    4 ++++
>> >>  fs/cifs/cifsacl.h    |    6 ++++++
>> >>  fs/cifs/cifsfs.c     |    6 ++++++
>> >>  3 files changed, 16 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
>> >> index ac51cd2..4bd0680 100644
>> >> --- a/fs/cifs/cifs_fs_sb.h
>> >> +++ b/fs/cifs/cifs_fs_sb.h
>> >> @@ -45,6 +45,10 @@
>> >>  struct cifs_sb_info {
>> >>       struct rb_root tlink_tree;
>> >>       spinlock_t tlink_tree_lock;
>> >> +     struct rb_root uidtree;
>> >> +     spinlock_t siduidlock;
>> >> +     struct rb_root gidtree;
>> >> +     spinlock_t sidgidlock;
>> >>       struct tcon_link *master_tlink;
>> >>       struct nls_table *local_nls;
>> >>       unsigned int rsize;
>> >> diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h
>> >> index c4ae7d0..6c26e7f 100644
>> >> --- a/fs/cifs/cifsacl.h
>> >> +++ b/fs/cifs/cifsacl.h
>> >> @@ -74,6 +74,12 @@ struct cifs_wksid {
>> >>       char sidname[SIDNAMELENGTH];
>> >>  } __attribute__((packed));
>> >>
>> >> +struct cifs_sid_id {
>> >> +     struct rb_node rbnode;
>> >> +     struct cifs_sid sid;
>> >> +     unsigned long id;
>> >> +} __attribute__((packed));
>> >> +
>> >>  extern int match_sid(struct cifs_sid *);
>> >>  extern int compare_sids(const struct cifs_sid *, const struct cifs_sid *);
>> >>
>> >> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> >> index a8323f1..36182e3 100644
>> >> --- a/fs/cifs/cifsfs.c
>> >> +++ b/fs/cifs/cifsfs.c
>> >> @@ -122,6 +122,12 @@ cifs_read_super(struct super_block *sb, void *data,
>> >>       spin_lock_init(&cifs_sb->tlink_tree_lock);
>> >>       cifs_sb->tlink_tree = RB_ROOT;
>> >>
>> >> +     spin_lock_init(&cifs_sb->siduidlock);
>> >> +     cifs_sb->uidtree = RB_ROOT;
>> >> +
>> >> +     spin_lock_init(&cifs_sb->sidgidlock);
>> >> +     cifs_sb->gidtree = RB_ROOT;
>> >> +
>> >>       rc = bdi_setup_and_register(&cifs_sb->bdi, "cifs", BDI_CAP_MAP_COPY);
>> >>       if (rc) {
>> >>               kfree(cifs_sb);
>> >
>> > Why do you need per-sb caches? The upcall isn't specific to a
>> > particular superblock, so if you have more than one mount you'll have
>> > to do multiple upcalls for the same SID and have mutiple copies of the
>> > same cifs_sid_id in various caches. It seems like a global cache would
>> > be better.
>> >
>> > --
>> > Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>> >
>>
>> Yes, I will change it to a per server structure instead of a per superblock.
>
> Why a per-server cache though? Are you passing any information that's
> specific to the server to the upcall? It looks to me like all you pass
> is the SID. If two servers have files owned by the same SID, is there
> any need for a separate upcall and separate cache entries for them?
>
> --
> Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>

Yes, I think global cache will work since all SIDs are either unique or
well-known and same across all servers/domains/active directory.
Will make the change.

Re: [PATCH 1/3]] cifs: Add idmap data structures and defines (try #3)

[Shirish Pargaonkar]

On Mon, Jan 24, 2011 at 12:50 PM, Shirish Pargaonkar
<shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Sat, Jan 22, 2011 at 6:38 AM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>> On Fri, 21 Jan 2011 23:49:33 -0600
>> Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>>> On Fri, Jan 21, 2011 at 4:04 PM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
>>> > On Fri, 21 Jan 2011 15:44:13 -0600
>>> > shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>> >
>>> >> From: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> >>
>>> >> A structure stored an rb tree is defined consisting of a SID and
>>> >> a id (either uid or gid) to which that SID is mapped.
>>> >>
>>> >> Added fields needed to store this defined data structures of a
>>> >> rb tree to a superblock and initialized them.
>>> >>
>>> >> There are two separate trees, one for SID/uid and another one for SID/gid.
>>> >>
>>> >>
>>> >> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> >> ---
>>> >>  fs/cifs/cifs_fs_sb.h |    4 ++++
>>> >>  fs/cifs/cifsacl.h    |    6 ++++++
>>> >>  fs/cifs/cifsfs.c     |    6 ++++++
>>> >>  3 files changed, 16 insertions(+), 0 deletions(-)
>>> >>
>>> >> diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
>>> >> index ac51cd2..4bd0680 100644
>>> >> --- a/fs/cifs/cifs_fs_sb.h
>>> >> +++ b/fs/cifs/cifs_fs_sb.h
>>> >> @@ -45,6 +45,10 @@
>>> >>  struct cifs_sb_info {
>>> >>       struct rb_root tlink_tree;
>>> >>       spinlock_t tlink_tree_lock;
>>> >> +     struct rb_root uidtree;
>>> >> +     spinlock_t siduidlock;
>>> >> +     struct rb_root gidtree;
>>> >> +     spinlock_t sidgidlock;
>>> >>       struct tcon_link *master_tlink;
>>> >>       struct nls_table *local_nls;
>>> >>       unsigned int rsize;
>>> >> diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h
>>> >> index c4ae7d0..6c26e7f 100644
>>> >> --- a/fs/cifs/cifsacl.h
>>> >> +++ b/fs/cifs/cifsacl.h
>>> >> @@ -74,6 +74,12 @@ struct cifs_wksid {
>>> >>       char sidname[SIDNAMELENGTH];
>>> >>  } __attribute__((packed));
>>> >>
>>> >> +struct cifs_sid_id {
>>> >> +     struct rb_node rbnode;
>>> >> +     struct cifs_sid sid;
>>> >> +     unsigned long id;
>>> >> +} __attribute__((packed));
>>> >> +
>>> >>  extern int match_sid(struct cifs_sid *);
>>> >>  extern int compare_sids(const struct cifs_sid *, const struct cifs_sid *);
>>> >>
>>> >> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>>> >> index a8323f1..36182e3 100644
>>> >> --- a/fs/cifs/cifsfs.c
>>> >> +++ b/fs/cifs/cifsfs.c
>>> >> @@ -122,6 +122,12 @@ cifs_read_super(struct super_block *sb, void *data,
>>> >>       spin_lock_init(&cifs_sb->tlink_tree_lock);
>>> >>       cifs_sb->tlink_tree = RB_ROOT;
>>> >>
>>> >> +     spin_lock_init(&cifs_sb->siduidlock);
>>> >> +     cifs_sb->uidtree = RB_ROOT;
>>> >> +
>>> >> +     spin_lock_init(&cifs_sb->sidgidlock);
>>> >> +     cifs_sb->gidtree = RB_ROOT;
>>> >> +
>>> >>       rc = bdi_setup_and_register(&cifs_sb->bdi, "cifs", BDI_CAP_MAP_COPY);
>>> >>       if (rc) {
>>> >>               kfree(cifs_sb);
>>> >
>>> > Why do you need per-sb caches? The upcall isn't specific to a
>>> > particular superblock, so if you have more than one mount you'll have
>>> > to do multiple upcalls for the same SID and have mutiple copies of the
>>> > same cifs_sid_id in various caches. It seems like a global cache would
>>> > be better.
>>> >
>>> > --
>>> > Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>>> >
>>>
>>> Yes, I will change it to a per server structure instead of a per superblock.
>>
>> Why a per-server cache though? Are you passing any information that's
>> specific to the server to the upcall? It looks to me like all you pass
>> is the SID. If two servers have files owned by the same SID, is there
>> any need for a separate upcall and separate cache entries for them?
>>
>> --
>> Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
>>
>
> Yes, I think global cache will work since all SIDs are either unique or
> well-known and same across all servers/domains/active directory.
> Will make the change.
>

One advantage to per server cache is, once a last SMB connection to
that server is taken down, the mapping entries in the cache related to SIDs
specific to that server would be gone. For a global cache, we have
to rely on shrinker to shrink/prune the cache.