On Wed, Nov 14, 2012 at 09:36:33PM +0000, Myklebust, Trond wrote: > On Wed, 2012-11-14 at 16:01 -0500, J. Bruce Fields wrote: > > On Mon, Nov 12, 2012 at 12:37:54PM +0400, Stanislav Kinsbursky wrote: > > > 07.11.2012 22:33, J. Bruce Fields пишет: > > > >On Tue, Nov 06, 2012 at 08:36:05AM -0500, J. Bruce Fields wrote: > > > >>On Tue, Nov 06, 2012 at 08:10:18AM -0500, Christoph Hellwig wrote: > > > >>>On Tue, Nov 06, 2012 at 08:07:06AM -0500, J. Bruce Fields wrote: > > > >>>>So you're worried that a bug in the nfs code could modify the root and > > > >>>>then not restore it? > > > >>> > > > >>>At least the link you pointed to earlier never sets it back. > > > >> > > > >>This? http://thread.gmane.org/gmane.linux.kernel/1259986/focus=47687 > > > >> > > > >> + get_fs_root(current->fs, &root); > > > >> + set_fs_root(current->fs, &transport->root); > > > >> + > > > >> status = xs_local_finish_connecting(xprt, sock); > > > >> + > > > >> + set_fs_root(current->fs, &root); > > > >> + path_put(&root); > > > >> > > > >>>Instead > > > >>>of messing with it I'd rather have the sunrpc code use vfs_path_lookup > > > >>>and not care about current->fs->root at all. > > > >> > > > >>The annoyance is that the lookup happens somewhere lower down in the > > > >>networking code (net/unix/af_unix.c:unix_find_other, I think). So we'd > > > >>need some new (internal) API. We'd likely be the only user of that new > > > >>API. > > > > > > > >So, if the only drawback is really just the risk of introducing a bug > > > >that leaves the fs_root changed--the above seems simple enough for that > > > >not to be a great risk, right? > > > > > > > > > > If we unshare rpciod fs struct (which is exported already), then we > > > > I'm not sure what you mean by that. Do workqueues actually have their > > own dedicated set of associated tasks? I thought all workqueues shared > > a common pool of tasks these days. > > > > > won't affect other kthreads by root swapping. > > > But would be great to hear Trond's opinion about this approach. > > > > > > Trond, could you tell us your feeling about all this? > > > > I think it's often easier to get people to comment on an actual patch, > > and this one would be quite short, so try that.... > > unshare() would break expectations for other users of workqueue threads > unless you "reshare()" afterwards. Either way that's going to be > seriously ugly. > > OK, let's look at this again. Do we ever use AF_LOCAL connections for > anything other than synchronous rpc calls to the local rpcbind daemon in > order to register/unregister new services? Simo's patches use them for upcalls to svcgssd. Those will always be done from server threads. > If not, then let's just move > the AF_LOCAL connection back into the process context and out of rpciod. Remind me how this helps? --b. > > That implies that the process needs to be privileged, but it needs > privileges in order to start RPC daemons anyway. -- 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