From: "J. Bruce Fields" <[email protected]>
Handle NFS-specific llseek errors instead of letting them leak out to
userspace.
Reported-by: Benjamin Coddington <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfs/nfs42proc.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
I don't actually have a test case for this, but it looks right....
diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index f486b80f927a..2fec410bc50f 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -135,7 +135,7 @@ int nfs42_proc_deallocate(struct file *filep, loff_t offset, loff_t len)
return err;
}
-loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
+loff_t _nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
{
struct inode *inode = file_inode(filep);
struct nfs42_seek_args args = {
@@ -171,6 +171,23 @@ loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
return vfs_setpos(filep, res.sr_offset, inode->i_sb->s_maxbytes);
}
+loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
+{
+ struct nfs_server *server = NFS_SERVER(file_inode(filep));
+ struct nfs4_exception exception = { };
+ int err;
+
+ do {
+ err = _nfs42_proc_llseek(filep, offset, whence);
+ if (err == -ENOTSUPP)
+ return -EOPNOTSUPP;
+ err = nfs4_handle_exception(server, err, &exception);
+ } while (exception.retry);
+
+ return err;
+}
+
+
static void
nfs42_layoutstat_prepare(struct rpc_task *task, void *calldata)
{
--
2.4.3
Hi Bruce,
On 07/23/2015 11:08 AM, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <[email protected]>
>
> Handle NFS-specific llseek errors instead of letting them leak out to
> userspace.
>
> Reported-by: Benjamin Coddington <[email protected]>
> Signed-off-by: J. Bruce Fields <[email protected]>
> ---
> fs/nfs/nfs42proc.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> I don't actually have a test case for this, but it looks right....
>
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index f486b80f927a..2fec410bc50f 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -135,7 +135,7 @@ int nfs42_proc_deallocate(struct file *filep, loff_t offset, loff_t len)
> return err;
> }
>
> -loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
> +loff_t _nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
> {
> struct inode *inode = file_inode(filep);
> struct nfs42_seek_args args = {
> @@ -171,6 +171,23 @@ loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
> return vfs_setpos(filep, res.sr_offset, inode->i_sb->s_maxbytes);
> }
>
> +loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
> +{
> + struct nfs_server *server = NFS_SERVER(file_inode(filep));
> + struct nfs4_exception exception = { };
> + int err;
Can you move the nfs_server_capable() check to before the do {} while loop? I think it's cleaner if we check for support before ever trying to call _nfs42_proc_llseek().
> +
> + do {
> + err = _nfs42_proc_llseek(filep, offset, whence);
> + if (err == -ENOTSUPP)
> + return -EOPNOTSUPP;
With the change to check support moved to this function, we should also unset NFS_CAP_SEEK if we see -ENOTSUPP here. I think you'll also need to change nfs4_file_llseek() (in nfs4file.c) to check for -EOPNOTSUPP instead of -ENOTSUPP.
Thanks,
Anna
> + err = nfs4_handle_exception(server, err, &exception);
> + } while (exception.retry);
> +
> + return err;
> +}
> +
> +
> static void
> nfs42_layoutstat_prepare(struct rpc_task *task, void *calldata)
> {
>
On Thu, Jul 23, 2015 at 11:25:23AM -0400, Anna Schumaker wrote:
> Hi Bruce,
>
> On 07/23/2015 11:08 AM, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <[email protected]>
> >
> > Handle NFS-specific llseek errors instead of letting them leak out to
> > userspace.
> >
> > Reported-by: Benjamin Coddington <[email protected]>
> > Signed-off-by: J. Bruce Fields <[email protected]>
> > ---
> > fs/nfs/nfs42proc.c | 19 ++++++++++++++++++-
> > 1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > I don't actually have a test case for this, but it looks right....
> >
> > diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> > index f486b80f927a..2fec410bc50f 100644
> > --- a/fs/nfs/nfs42proc.c
> > +++ b/fs/nfs/nfs42proc.c
> > @@ -135,7 +135,7 @@ int nfs42_proc_deallocate(struct file *filep, loff_t offset, loff_t len)
> > return err;
> > }
> >
> > -loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
> > +loff_t _nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
> > {
> > struct inode *inode = file_inode(filep);
> > struct nfs42_seek_args args = {
> > @@ -171,6 +171,23 @@ loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
> > return vfs_setpos(filep, res.sr_offset, inode->i_sb->s_maxbytes);
> > }
> >
> > +loff_t nfs42_proc_llseek(struct file *filep, loff_t offset, int whence)
> > +{
> > + struct nfs_server *server = NFS_SERVER(file_inode(filep));
> > + struct nfs4_exception exception = { };
> > + int err;
>
> Can you move the nfs_server_capable() check to before the do {} while loop? I think it's cleaner if we check for support before ever trying to call _nfs42_proc_llseek().
I believe the results of that check can change on subsequent iterations,
due to a concurrent SEEK returning NOTSUPP.
The code would be correct either way, because the nfs_server_capable()
check is really just an optimization--it should always be OK just to
send the SEEK and then handle the NOTSUPP reply.
But accidentally sending an unnecessary rpc is much more expensive than
checking a flag, so the code's probably best left as is.
(Also, this is the way it's done elsewhere in nfs{4,42}proc.c, so if
there's some reason to change we'd want to do that everywhere.)
--b.