On Mon, Oct 16, 2023 at 10:17:50PM +0300, Vitaliy Makkoveev wrote:
> This socket comes from userland, so this flag is never set. This makes
> SB_NOINTR flag immutable, because we only set this bit on NFS client
> socket buffers for all it's lifetime.
>
> I want to do this because this flag modifies sblock() behaviour and it's
> not clean which lock should protect it for standalone sblock().
>
> ok?
>
> I want to completely remove SB_NOINTR flag. Only NFS client sets it, but
> since the socket never passed to userland, this flag is useless, because
> we can't send singnal to kernel thread. So for this sblock()/sbwait()
> sleep is uninterruptible. But I want to not mix NFS server and NFS
> client diffs.
>
> Index: sys/nfs/nfs_syscalls.c
> ===================================================================
> RCS file: /cvs/src/sys/nfs/nfs_syscalls.c,v
> retrieving revision 1.119
> diff -u -p -r1.119 nfs_syscalls.c
> --- sys/nfs/nfs_syscalls.c 3 Aug 2023 09:49:09 -0000 1.119
> +++ sys/nfs/nfs_syscalls.c 16 Oct 2023 19:00:02 -0000
> @@ -276,9 +276,7 @@ nfssvc_addsock(struct file *fp, struct m
> m_freem(m);
> }
> solock(so);
> - so->so_rcv.sb_flags &= ~SB_NOINTR;
> so->so_rcv.sb_timeo_nsecs = INFSLP;
> - so->so_snd.sb_flags &= ~SB_NOINTR;
> so->so_snd.sb_timeo_nsecs = INFSLP;
> sounlock(so);
> if (tslp)
>
To make the possible review easier, please look at sbin/nfsd/nfsd.c. We
have two cases, for UDP case we just pass newly created socket to kernel
side nfsd, for TCP case we pass only accepted sockets:
104 main(int argc, char *argv[])
105 {
/* ... */
208 if (udpflag) {
209 if ((sock = socket(AF_INET, SOCK_DGRAM, 0)) == -1) {
210 syslog(LOG_ERR, "can't create udp socket");
211 return (1);
212 }
/* ... */
228 nfsdargs.sock = sock;
229 nfsdargs.name = NULL;
230 nfsdargs.namelen = 0;
231 if (nfssvc(NFSSVC_ADDSOCK, &nfsdargs) == -1) {
232 syslog(LOG_ERR, "can't Add UDP socket");
233 return (1);
234 }
235 (void)close(sock);
236 }
/* ... */
241 if (tcpflag) {
242 if ((tcpsock = socket(AF_INET, SOCK_STREAM, 0)) == -1) {
243 syslog(LOG_ERR, "can't create tcp socket");
244 return (1);
245 }
/* ... */
259 if (listen(tcpsock, 5) == -1) {
260 syslog(LOG_ERR, "listen failed");
261 return (1);
262 }
/* ... */
269 }
/* ... */
276 /*
277 * Loop forever accepting connections and passing the sockets
278 * into the kernel for the mounts.
279 */
280 for (;;) {
/* ... */
298 if (tcpflag) {
299 len = sizeof(inetpeer);
300 if ((msgsock = accept(tcpsock,
301 (struct sockaddr *)&inetpeer, &len)) == -1) {
/* ... */
313 nfsdargs.sock = msgsock;
314 nfsdargs.name = (caddr_t)&inetpeer;
315 nfsdargs.namelen = sizeof(inetpeer);
316 if (nfssvc(NFSSVC_ADDSOCK, &nfsdargs) == -1) {
317 syslog(LOG_ERR, "can't Add TCP socket");
318 return (1);
319 }
320 (void)close(msgsock);
321 }
322 }
For the NFSSVC_ADDSOCK command, sys_nfssvc() gets associated descriptor
and passes it to nfssvc_addsock().
141 sys_nfssvc(struct proc *p, void *v, register_t *retval)
142 {
/* ... */
171 switch (flags) {
172 case NFSSVC_ADDSOCK:
173 error = copyin(SCARG(uap, argp), &nfsdarg, sizeof(nfsdarg));
174 if (error)
175 return (error);
176
177 error = getsock(p, nfsdarg.sock, &fp);
/* ... */
194 error = nfssvc_addsock(fp, nam);
nfssvc_addsock() gets associated socket, does necessary setup and passes
it to nfsd. There is the only place where we clean this flag.
225 nfssvc_addsock(struct file *fp, struct mbuf *mynam)
226 {
/* ... */
278 solock(so);
279 so->so_rcv.sb_flags &= ~SB_NOINTR;
280 so->so_rcv.sb_timeo_nsecs = INFSLP;
281 so->so_snd.sb_flags &= ~SB_NOINTR;
282 so->so_snd.sb_timeo_nsecs = INFSLP;
283 sounlock(so);
The only place where we set this flag is the NFS client related socket.
It always created in kernel space and it never comes to the userland.
This socket can't be passed to nfsd.
234 nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
235 {
/* ... */
247 error = socreate(saddr->sa_family, &nmp->nm_so, nmp->nm_sotype,
248 nmp->nm_soproto);
/* ... */
370 so->so_rcv.sb_flags |= SB_NOINTR;
371 so->so_snd.sb_flags |= SB_NOINTR;
372 sounlock(so);
We have absolutely no methods to modify SB_NOINTR bit. All the sockets,
except nfs client related, have this bit clean. So the actions in the
nfssvc_addsock() are null ops.
