On Mon, Jan 24, 2011 at 12:50 PM, Shirish Pargaonkar
<shirishpargaonkar@xxxxxxxxx> wrote:
> On Sat, Jan 22, 2011 at 6:38 AM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
>> On Fri, 21 Jan 2011 23:49:33 -0600
>> Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
>>
>>> On Fri, Jan 21, 2011 at 4:04 PM, Jeff Layton <jlayton@xxxxxxxxx> wrote:
>>> > On Fri, 21 Jan 2011 15:44:13 -0600
>>> > shirishpargaonkar@xxxxxxxxx wrote:
>>> >
>>> >> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>>> >>
>>> >> 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@xxxxxxxxx>
>>> >> ---
>>> >> 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@xxxxxxxxx>
>>> >
>>>
>>> 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@xxxxxxxxx>
>>
>
> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
