Commit faa65f07 authored by Jeff Layton's avatar Jeff Layton Committed by Steve French

cifs: simplify id_to_sid and sid_to_id mapping code

The cifs.idmap handling code currently causes the kernel to cache the
data from userspace twice. It first looks in a rbtree to see if there is
a matching entry for the given id. If there isn't then it calls
request_key which then checks its cache and then calls out to userland
if it doesn't have one. If the userland program establishes a mapping
and downcalls with that info, it then gets cached in the keyring and in
this rbtree.

Aside from the double memory usage and the performance penalty in doing
all of these extra copies, there are some nasty bugs in here too. The
code declares four rbtrees and spinlocks to protect them, but only seems
to use two of them. The upshot is that the same tree is used to hold
(eg) uid:sid and sid:uid mappings. The comparitors aren't equipped to
deal with that.

I think we'd be best off to remove a layer of caching in this code. If
this was originally done for performance reasons, then that really seems
like a premature optimization.

This patch does that -- it removes the rbtrees and the locks that
protect them and simply has the code do a request_key call on each call
into sid_to_id and id_to_sid. This greatly simplifies this code and
should roughly halve the memory utilization from using the idmapping
code.
Reviewed-by: default avatarShirish Pargaonkar <shirishpargaonkar@gmail.com>
Signed-off-by: default avatarJeff Layton <jlayton@redhat.com>
Signed-off-by: default avatarSteve French <smfrench@gmail.com>
parent 03eca704
This diff is collapsed.
......@@ -23,7 +23,7 @@
#define _CIFSACL_H
#define NUM_AUTHS 6 /* number of authority fields */
#define NUM_AUTHS (6) /* number of authority fields */
#define SID_MAX_SUB_AUTHORITIES (15) /* max number of sub authority fields */
#define NUM_WK_SIDS 7 /* number of well known sids */
#define SIDNAMELENGTH 20 /* long enough for the ones we care about */
......@@ -51,15 +51,12 @@
* u32: max 10 bytes in decimal
*
* "S-" + 3 bytes for version field + 4 bytes for each authority field (3 bytes
* per number + 1 for '-') + 11 bytes for each subauthority field (10 bytes
* per number + 1 for '-') + NULL terminator.
*
* Add 11 bytes for each subauthority field (10 bytes each + 1 for '-')
*/
#define SID_STRING_MAX (195)
#define SID_ID_MAPPED 0
#define SID_ID_PENDING 1
#define SID_MAP_EXPIRE (3600 * HZ) /* map entry expires after one hour */
#define SID_MAP_RETRY (300 * HZ) /* wait 5 minutes for next attempt to map */
#define SID_STRING_BASE_SIZE (2 + 3 + (4 * NUM_AUTHS) + 1)
#define SID_STRING_SUBAUTH_SIZE (11) /* size of a single subauth string */
struct cifs_ntsd {
__le16 revision; /* revision level */
......@@ -94,19 +91,4 @@ struct cifs_ace {
struct cifs_sid sid; /* ie UUID of user or group who gets these perms */
} __attribute__((packed));
struct cifs_wksid {
struct cifs_sid cifssid;
char sidname[SIDNAMELENGTH];
} __attribute__((packed));
struct cifs_sid_id {
unsigned int refcount; /* increment with spinlock, decrement without */
unsigned long id;
unsigned long time;
unsigned long state;
char *sidstr;
struct rb_node rbnode;
struct cifs_sid sid;
};
#endif /* _CIFSACL_H */
......@@ -1204,7 +1204,6 @@ exit_cifs(void)
unregister_filesystem(&cifs_fs_type);
cifs_dfs_release_automount_timer();
#ifdef CONFIG_CIFS_ACL
cifs_destroy_idmaptrees();
exit_cifs_idmap();
#endif
#ifdef CONFIG_CIFS_UPCALL
......
......@@ -58,7 +58,6 @@ do { \
} while (0)
extern int init_cifs_idmap(void);
extern void exit_cifs_idmap(void);
extern void cifs_destroy_idmaptrees(void);
extern char *build_path_from_dentry(struct dentry *);
extern char *cifs_build_path_to_root(struct smb_vol *vol,
struct cifs_sb_info *cifs_sb,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment