On Wed, 2013-05-08 at 14:41 -0400, Steve Dickson wrote: > > On 08/05/13 14:31, Myklebust, Trond wrote: > > On Wed, 2013-05-08 at 14:27 -0400, Steve Dickson wrote: > >> > >> On 08/05/13 14:07, Myklebust, Trond wrote: > >>> On Wed, 2013-05-08 at 13:39 -0400, Steve Dickson wrote: > >>>> > >>>> On 08/05/13 12:43, Myklebust, Trond wrote: > >>>>> On Wed, 2013-05-08 at 12:39 -0400, Steve Dickson wrote: > >>>>>> > >>>>>> On 01/05/13 15:03, Myklebust, Trond wrote: > >>>>>>>> @@ -2409,10 +2468,26 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f > >>>>>>>>> server->caps |= NFS_CAP_CTIME; > >>>>>>>>> if (res.attr_bitmask[1] & FATTR4_WORD1_TIME_MODIFY) > >>>>>>>>> server->caps |= NFS_CAP_MTIME; > >>>>>>>>> +#ifdef CONFIG_NFS_V4_SECURITY_LABEL > >>>>>>>>> + if (res.attr_bitmask[2] & FATTR4_WORD2_SECURITY_LABEL) > >>>>>>>>> + server->caps |= NFS_CAP_SECURITY_LABEL; > >>>>>>>>> +#endif > >>>>>>>>> + memcpy(server->attr_bitmask_nl, res.attr_bitmask, > >>>>>>>>> + sizeof(server->attr_bitmask)); > >>>>>>>>> + > >>>>>>>>> + if (server->caps & NFS_CAP_SECURITY_LABEL) > >>>>>>>>> + server->attr_bitmask_nl[2] &= ~FATTR4_WORD2_SECURITY_LABEL; > >>>>>>>>> > >>>>>>>>> memcpy(server->cache_consistency_bitmask, res.attr_bitmask, sizeof(server->cache_consistency_bitmask)); > >>>>>>>>> server->cache_consistency_bitmask[0] &= FATTR4_WORD0_CHANGE|FATTR4_WORD0_SIZE; > >>>>>>>>> - server->cache_consistency_bitmask[1] &= FATTR4_WORD1_TIME_METADATA|FATTR4_WORD1_TIME_MODIFY; > >>>>>>>>> + server->cache_consistency_bitmask[1] &= FATTR4_WORD1_TIME_METADATA | > >>>>>>>>> + FATTR4_WORD1_TIME_MODIFY; > >>>>>>>>> +#ifdef CONFIG_NFS_V4_SECURITY_LABEL > >>>>>>>>> + server->cache_consistency_bitmask[2] &= FATTR4_WORD2_SECURITY_LABEL; > >>>>>>> Why? How is the security label relevant to cache consistency? > >>>>>> Its used to the set label bit in the GETATTR that goes out with ACCESS compound. > >>>>> > >>>>> The GETATTR that goes out with ACCESS is only there in order to get the > >>>>> change attribute so that we know when to invalidate the access cache. It > >>>>> is _only_ for cache consistency. > >>>>> > >>>>> Why do we need to fetch the label too? > >>>>> > >>>> I think I answer this in the other thread but in short > >>>> access updates the inode and so it appears the goal > >>>> is to synchronize inode updates and label updates. > >>> > >>> Those are not inode updates. > >> Ah... OK... but they all end up calling nfs_refresh_inode() with > >> the valid label pointer... So there is an effort to keep the > >> inode attribute cache updates synchronized with label updates... > >> > >> So I guess the question is that needed... Is the setting of > >> the label in nfs_fhget() and/or _nfs4_do_open() good enough. > > > > Until someone comes up with a different cache consistency model, then > > I'd say yes. The only other case that comes to mind, is when our client > > actively changes the label... > On the server side as well, correct? What stops a process on the server side > from changing the label? Nothing, but what is the expectation for how that will change client behaviour? > I'm thinking that is reason the label was put in all those GETATTRs They are not part of a well thought-out caching model. Instead, these are just random revalidations. If someone changes the label on the server, then I can still do a million things on the client through caching until I discover that the label has changed. -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@xxxxxxxxxx www.netapp.com -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
