On Mon, Dec 13, 2010 at 5:16 AM, Jeff Layton <jlayton@xxxxxxxxx> wrote: > On Mon, 13 Dec 2010 14:22:09 +1100 > Andrew Bartlett <abartlet@xxxxxxxxx> wrote: > >> On Sun, 2010-12-12 at 06:39 -0500, Jeff Layton wrote: >> > On Sun, 12 Dec 2010 14:48:04 +1100 >> > Andrew Bartlett <abartlet@xxxxxxxxx> wrote: >> > >> > > On Sat, 2010-12-11 at 22:11 -0500, Jeff Layton wrote: >> > > > On Sat, 11 Dec 2010 19:57:11 -0500 >> > > > Richard Sharpe <realrichardsharpe@xxxxxxxxx> wrote: >> > > > >> > > > > On Sat, Dec 11, 2010 at 7:30 PM, Jeff Layton <jlayton@xxxxxxxxx> wrote: >> > > > > >> >> > > > > >> Will look into this. One thing that concerns me is if a cached etnry >> > > > > >> for a SID with its name and an id (either an uid or a gid), if that SID >> > > > > >> now represents a different object and has differernt name, would >> > > > > >> not cached info be incorrect? Not sure if this can ever happen >> > > > > >> or how would it happen and if it does, what would be a trigger >> > > > > >> for a cache revalidation and purges! >> > > > > >> >> > > > > > >> > > > > > Sure, mappings can change. But, you still have the same problem with >> > > > > > what you're proposing in these patches. The userspace program isn't >> > > > > > setting a timeout on the key. Once a mapping is put in the keyring, >> > > > > > it's there until it's revoked. You probably want to set a max TTL for >> > > > > > the entries in the cache regardless of what scheme is used. >> > > > > >> > > > > I was under the impression that SIDs are never reused. Perhaps I am mistaken. >> > > > > >> > > > >> > > > That may be, but the mapping of a SID is dependent upon settings in >> > > > config files that could change. It seems reasonable to me to only cache >> > > > these mappings for a period of time in the event that they do. That >> > > > period of time could default to being rather long and be tunable. >> > > >> > > I think that instead some explicit signal should be made to indicate >> > > that a mapping has changed, so you don't have to worry about cache >> > > times. It should change *very* rarely and only on specific >> > > administrator intervention. We do a lot of things to avoid this >> > > happening in the normal course of events. >> > > >> > >> > What would provide this signal? winbindd? I suppose we could add a knob >> > or something under /sys that tells cifs to dump the idmap cache. >> >> I think a /sys knob seems appropriate, perhaps easily sent a command >> option on the same utility used for the upcall? >> >> > We would also have to consider however how to deal with someone running >> > an old winbindd that doesn't signal the kernel properly. >> >> That's a very interesting question, as after a manual reconfiguration >> perhaps even winbind might not know it changed. It depends how deeply >> the administrator changed things (changing the idmap_rid config settings >> might matter for example). I'll let others who deal with idmap more >> often comment. >> > > The other option is just to have a manual knob that flushes the cache, > and add something like this to the cifs.upcall manpage: "If you change > your idmapping configuration, then you'll probably also want to flush > the idmap cache." Maybe it's a rare enough thing that we shouldn't > sweat trying to make it too automatic. > > -- > Jeff Layton <jlayton@xxxxxxxxx> > # date; cat /proc/keys | grep cifs Tue Dec 14 16:16:20 CST 2010 26fb212d I----- 1 perm 1f030000 0 0 keyring .cifs_idmap: empty 2fb33fb5 IR---- 2 expd 1f030000 0 0 keyring .cifs_idmap: empty # date; cat /proc/keys | grep cifs Tue Dec 14 16:16:29 CST 2010 26fb212d I----- 1 perm 1f030000 0 0 keyring .cifs_idmap: 2/4 2fb33fb5 IR---- 2 expd 1f030000 0 0 keyring .cifs_idmap: empty # date; cat /proc/keys | grep cifs Tue Dec 14 16:20:10 CST 2010 26fb212d I----- 1 perm 1f030000 0 0 keyring .cifs_idmap: 2/4 2fb33fb5 IR---- 2 expd 1f030000 0 0 keyring .cifs_idmap: empty Should not these two entries expire after a minute if key->expiry = KEY_EXPIRY_TIMEOUT; in function cifs_idmap_key_instantiate() if #define KEY_EXPIRY_TIMEOUT 60*HZ I was expecting this to be empty instead of 2/4 after more than a minute of inactivity. 26fb212d I----- 1 perm 1f030000 0 0 keyring .cifs_idmap: 2/4 -- 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