On Mon, 23 Jun 2014 09:12:22 -0700 Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > On Thu, Jun 19, 2014 at 10:49:14AM -0400, Jeff Layton wrote: > > Lock atomicity requires us to check the share access mode when we > > actually open the file. Note that ideally this would also be atomic > > with file creation. > > > > With the change to make nfs4_file_get_access enforce the share mode, we > > now have a bogus WARN_ON that can fire. It's now normal to call > > nfs4_file_get_access before populating the fi_fds field for the open > > flag, so we should no longer warn on that situation. > > Which change is that? Seems like this is a previous commit, so it > would be good to refer to it explicitly. > Sorry it wasn't clear. The change is in this patch. This is just an explanation of why the WARN_ON needed to be removed. > > The other case is a WARN_ON that can occur if there's a O_RDWR open > > already present. I'm unclear on why we'd WARN_ON in that case. This > > patch removes it, but please do enlighten me if there's some reason > > we ought to keep it instead. > > I have a really hard time mapping from what's in this description > to what happens in the patch. > > Looking over the patch I can see that it: > > adds a fi_share_deny field to struct nfs4_file, which gets set up in > nfs4_file_get_access, and cleared from nfs4_file_put_access, as well > as some general refactoring around nfs4_file_get_access. Can you > split the refactor into a first patch that has a description what's > going on, and keep the addition of the deny mask separate, again > including a description of what it's trying to help with? Ok, I'll see if I can break that up. -- Jeff Layton <jlayton@xxxxxxxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
