Received-SPF: pass (google.com: domain of linux-nfs+bounces-2090-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <CAPKjjnrYvzH8hEk9boaBt-fETX3VD2cjjN-Z6iNgwZpHqYUjWw@mail.gmail.com>
 <77a58302766cb6c8fac45682ede63569df80cd5d.camel@hammerspace.com>
 <1179779e2f74e3e5cb2be30cf89e6362aaab706d.camel@kernel.org>
 <16d8c8e88490ee92750b26f2c438e1329dea0061.camel@hammerspace.com> <3754ac34c55dd82a4957967ec0a4e490cdc0d989.camel@kernel.org>
In-Reply-To: <3754ac34c55dd82a4957967ec0a4e490cdc0d989.camel@kernel.org>
From: Zhitao Li <zhitao.li@smartx.com>
Date: Tue, 27 Feb 2024 10:35:18 +0800
Message-ID: <CAPKjjnq0=HG5_iC91tjqU_pJp00Q+ffo0m=7Sk-8PbbAPv1+Cw@mail.gmail.com>
Subject: Re: PROBLEM: NFS client IO fails with ERESTARTSYS when another mount
 point with the same export is unmounted with force [NFS] [SUNRPC]
To: Trond Myklebust <trondmy@hammerspace.com>, Jeff Layton <jlayton@kernel.org>
Cc: "chuck.lever@oracle.com" <chuck.lever@oracle.com>, "kolga@netapp.com" <kolga@netapp.com>, 
	"anna@kernel.org" <anna@kernel.org>, "tom@talpey.com" <tom@talpey.com>, "neilb@suse.de" <neilb@suse.de>, 
	"Dai.Ngo@oracle.com" <Dai.Ngo@oracle.com>, "huangping@smartx.com" <huangping@smartx.com>, 
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Is there any plan for this ERESTARTSYS leak issue?

--
Zhitao Li, at SmartX

On Fri, Feb 23, 2024 at 6:31=E2=80=AFPM Jeff Layton <jlayton@kernel.org> wr=
ote:
>
> On Thu, 2024-02-22 at 15:20 +0000, Trond Myklebust wrote:
> > On Thu, 2024-02-22 at 06:05 -0500, Jeff Layton wrote:
> > > On Wed, 2024-02-21 at 13:48 +0000, Trond Myklebust wrote:
> > > > On Wed, 2024-02-21 at 16:20 +0800, Zhitao Li wrote:
> > > > > [You don't often get email from zhitao.li@smartx.com. Learn why
> > > > > this
> > > > > is important at https://aka.ms/LearnAboutSenderIdentification ]
> > > > >
> > > > > Hi, everyone,
> > > > >
> > > > > - Facts:
> > > > > I have a remote NFS export and I mount the same export on two
> > > > > different directories in my OS with the same options. There is an
> > > > > inflight IO under one mounted directory. And then I unmount
> > > > > another
> > > > > mounted directory with force. The inflight IO ends up with
> > > > > "Unknown
> > > > > error 512", which is ERESTARTSYS.
> > > > >
> > > >
> > > > All of the above is well known. That's because forced umount
> > > > affects
> > > > the entire filesystem. Why are you using it here in the first
> > > > place? It
> > > > is not intended for casual use.
> > > >
> > >
> > > While I agree Trond's above statement, the kernel is not supposed to
> > > leak error codes that high into userland. Are you seeing ERESTARTSYS
> > > being returned to system calls? If so, which ones?
> >
> > The point of forced umount is to kill all RPC calls associated with the
> > filesystem in order to unblock the umount. Basically, it triggers this
> > code before the unmount starts:
> >
> > void nfs_umount_begin(struct super_block *sb)
> > {
> >         struct nfs_server *server;
> >         struct rpc_clnt *rpc;
> >
> >         server =3D NFS_SB(sb);
> >         /* -EIO all pending I/O */
> >         rpc =3D server->client_acl;
> >         if (!IS_ERR(rpc))
> >                 rpc_killall_tasks(rpc);
> >         rpc =3D server->client;
> >         if (!IS_ERR(rpc))
> >                 rpc_killall_tasks(rpc);
> > }
> >
> > So yes, that does signal all the way up to the application level, and
> > it is very much intended to do so.
>
> Returning an error to userland in this situation is fine, but userland
> programs aren't really equipped to deal with error numbers in this
> range.
>
> Emphasis on the first sentence in the comment in include/linux/errno.h:
>
> -------------------8<-----------------------
> /*
>  * These should never be seen by user programs.  To return one of ERESTAR=
T*
>  * codes, signal_pending() MUST be set.  Note that ptrace can observe the=
se
>  * at syscall exit tracing, but they will never be left for the debugged =
user
>  * process to see.
>  */
> #define ERESTARTSYS     512
> #define ERESTARTNOINTR  513
> #define ERESTARTNOHAND  514     /* restart if no handler.. */
> #define ENOIOCTLCMD     515     /* No ioctl command */
> #define ERESTART_RESTARTBLOCK 516 /* restart by calling sys_restart_sysca=
ll */
> #define EPROBE_DEFER    517     /* Driver requests probe retry */
> #define EOPENSTALE      518     /* open found a stale dentry */
> #define ENOPARAM        519     /* Parameter not supported */
> -------------------8<-----------------------
>
> If these values are leaking into userland, then that seems like a bug.
> --
> Jeff Layton <jlayton@kernel.org>