As soon as the nfs_client gets created, its cl_rpcclient is set to
ERR_PTR(-EINVAL). The rpc client structure is allocated later. Check
for the validity of cl_rpcclient before using it.
Signed-off-by: Malahal Naineni <[email protected]>
---
fs/nfs/client.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 5833fbb..ff79e72 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1867,6 +1867,10 @@ static int nfs_server_list_show(struct seq_file *m, void *v)
/* display one transport per line on subsequent lines */
clp = list_entry(v, struct nfs_client, cl_share_link);
+ /* Check if the rpc client is initialized */
+ if (IS_ERR(clp->cl_rpcclient))
+ return 0;
+
seq_printf(m, "v%u %s %s %3d %s\n",
clp->rpc_ops->version,
rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_ADDR),
--
1.7.0.4
Malahal Naineni [[email protected]] wrote:
> Myklebust, Trond [[email protected]] wrote:
> > Shouldn't we just skip all nfs_client structures that don't have
> > cl_cons_state set to NFS_CS_READY? I see no reason to display their
> > properties even if they have got far enough as to create an rpc client.
> >
> > Cheers
> > Trond
>
> Sounds good to me. Here is a tested patch.
Any comments on this one?
Thanks, Malahal.
>
>
> As soon as the nfs_client gets created, its cl_rpcclient is set to
> ERR_PTR(-EINVAL). The rpc client structure is allocated later. Check
> if the client is ready before using the cl_rpcclient pointer.
>
> Signed-off-by: Malahal Naineni <[email protected]>
> ---
> fs/nfs/client.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 5833fbb..a54c45e 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -1867,6 +1867,10 @@ static int nfs_server_list_show(struct seq_file *m, void *v)
> /* display one transport per line on subsequent lines */
> clp = list_entry(v, struct nfs_client, cl_share_link);
>
> + /* Check if the client is initialized */
> + if (clp->cl_cons_state != NFS_CS_READY)
> + return 0;
> +
> seq_printf(m, "v%u %s %s %3d %s\n",
> clp->rpc_ops->version,
> rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_ADDR),
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Any comments on the patch below? It is very easy to reproduce the bug
(at least on SLES11 SP1).
Thanks, Malahal.
Malahal Naineni [[email protected]] wrote:
> As soon as the nfs_client gets created, its cl_rpcclient is set to
> ERR_PTR(-EINVAL). The rpc client structure is allocated later. Check
> for the validity of cl_rpcclient before using it.
>
> Signed-off-by: Malahal Naineni <[email protected]>
> ---
> fs/nfs/client.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 5833fbb..ff79e72 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -1867,6 +1867,10 @@ static int nfs_server_list_show(struct seq_file *m, void *v)
> /* display one transport per line on subsequent lines */
> clp = list_entry(v, struct nfs_client, cl_share_link);
>
> + /* Check if the rpc client is initialized */
> + if (IS_ERR(clp->cl_rpcclient))
> + return 0;
> +
> seq_printf(m, "v%u %s %s %3d %s\n",
> clp->rpc_ops->version,
> rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_ADDR),
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message-----
> From: Malahal Naineni [mailto:[email protected]]
> Sent: Monday, September 19, 2011 11:34 AM
> To: [email protected]
> Cc: Myklebust, Trond
> Subject: Re: [PATCH] Check validity of cl_rpcclient in
nfs_server_list_show
>
> Any comments on the patch below? It is very easy to reproduce the bug
(at
> least on SLES11 SP1).
Shouldn't we just skip all nfs_client structures that don't have
cl_cons_state set to NFS_CS_READY? I see no reason to display their
properties even if they have got far enough as to create an rpc client.
Cheers
Trond
> Thanks, Malahal.
>
> Malahal Naineni [[email protected]] wrote:
> > As soon as the nfs_client gets created, its cl_rpcclient is set to
> > ERR_PTR(-EINVAL). The rpc client structure is allocated later. Check
> > for the validity of cl_rpcclient before using it.
> >
> > Signed-off-by: Malahal Naineni <[email protected]>
> > ---
> > fs/nfs/client.c | 4 ++++
> > 1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/nfs/client.c b/fs/nfs/client.c index
5833fbb..ff79e72
> > 100644
> > --- a/fs/nfs/client.c
> > +++ b/fs/nfs/client.c
> > @@ -1867,6 +1867,10 @@ static int nfs_server_list_show(struct
seq_file
> *m, void *v)
> > /* display one transport per line on subsequent lines */
> > clp = list_entry(v, struct nfs_client, cl_share_link);
> >
> > + /* Check if the rpc client is initialized */
> > + if (IS_ERR(clp->cl_rpcclient))
> > + return 0;
> > +
> > seq_printf(m, "v%u %s %s %3d %s\n",
> > clp->rpc_ops->version,
> > rpc_peeraddr2str(clp->cl_rpcclient,
> RPC_DISPLAY_HEX_ADDR),
> > --
> > 1.7.0.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> > in the body of a message to [email protected] More
> majordomo
> > info at http://vger.kernel.org/majordomo-info.html
Myklebust, Trond [[email protected]] wrote:
> Shouldn't we just skip all nfs_client structures that don't have
> cl_cons_state set to NFS_CS_READY? I see no reason to display their
> properties even if they have got far enough as to create an rpc client.
>
> Cheers
> Trond
Sounds good to me. Here is a tested patch.
As soon as the nfs_client gets created, its cl_rpcclient is set to
ERR_PTR(-EINVAL). The rpc client structure is allocated later. Check
if the client is ready before using the cl_rpcclient pointer.
Signed-off-by: Malahal Naineni <[email protected]>
---
fs/nfs/client.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 5833fbb..a54c45e 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1867,6 +1867,10 @@ static int nfs_server_list_show(struct seq_file *m, void *v)
/* display one transport per line on subsequent lines */
clp = list_entry(v, struct nfs_client, cl_share_link);
+ /* Check if the client is initialized */
+ if (clp->cl_cons_state != NFS_CS_READY)
+ return 0;
+
seq_printf(m, "v%u %s %s %3d %s\n",
clp->rpc_ops->version,
rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_ADDR),
--
1.7.0.4
The following patch fixes the panic that we see often. I didn't see it
in the git://git.linux-nfs.org/projects/trondmy/linux-nfs.git git tree
yet as far as I can tell. Any comments on the patch?
Thanks, Malahal.
Malahal Naineni [[email protected]] wrote:
> Any comments on this patch?
>
> Malahal Naineni [[email protected]] wrote:
> > Malahal Naineni [[email protected]] wrote:
> > > Myklebust, Trond [[email protected]] wrote:
> > > > Shouldn't we just skip all nfs_client structures that don't have
> > > > cl_cons_state set to NFS_CS_READY? I see no reason to display their
> > > > properties even if they have got far enough as to create an rpc client.
> > > >
> > > > Cheers
> > > > Trond
> > >
> > > Sounds good to me. Here is a tested patch.
> >
> > Any comments on this one?
> >
> > Thanks, Malahal.
> >
> > >
> > >
> > > As soon as the nfs_client gets created, its cl_rpcclient is set to
> > > ERR_PTR(-EINVAL). The rpc client structure is allocated later. Check
> > > if the client is ready before using the cl_rpcclient pointer.
> > >
> > > Signed-off-by: Malahal Naineni <[email protected]>
> > > ---
> > > fs/nfs/client.c | 4 ++++
> > > 1 files changed, 4 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> > > index 5833fbb..a54c45e 100644
> > > --- a/fs/nfs/client.c
> > > +++ b/fs/nfs/client.c
> > > @@ -1867,6 +1867,10 @@ static int nfs_server_list_show(struct seq_file *m, void *v)
> > > /* display one transport per line on subsequent lines */
> > > clp = list_entry(v, struct nfs_client, cl_share_link);
> > >
> > > + /* Check if the client is initialized */
> > > + if (clp->cl_cons_state != NFS_CS_READY)
> > > + return 0;
> > > +
> > > seq_printf(m, "v%u %s %s %3d %s\n",
> > > clp->rpc_ops->version,
> > > rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_ADDR),
> > > --
> > > 1.7.0.4
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > > the body of a message to [email protected]
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Sorry about that. I'll try to fix it later, but I'm on various planes for most of today so I might not have the ability until tomorrow.
Malahal Naineni <[email protected]> wrote:
The following patch fixes the panic that we see often. I didn't see it
in the git://git.linux-nfs.org/projects/trondmy/linux-nfs.git git tree
yet as far as I can tell. Any comments on the patch?
Thanks, Malahal.
Malahal Naineni [[email protected]] wrote:
> Any comments on this patch?
>
> Malahal Naineni [[email protected]] wrote:
> > Malahal Naineni [[email protected]] wrote:
> > > Myklebust, Trond [[email protected]] wrote:
> > > > Shouldn't we just skip all nfs_client structures that don't have
> > > > cl_cons_state set to NFS_CS_READY? I see no reason to display their
> > > > properties even if they have got far enough as to create an rpc client.
> > > >
> > > > Cheers
> > > > Trond
> > >
> > > Sounds good to me. Here is a tested patch.
> >
> > Any comments on this one?
> >
> > Thanks, Malahal.
> >
> > >
> > >
> > > As soon as the nfs_client gets created, its cl_rpcclient is set to
> > > ERR_PTR(-EINVAL). The rpc client structure is allocated later. Check
> > > if the client is ready before using the cl_rpcclient pointer.
> > >
> > > Signed-off-by: Malahal Naineni <[email protected]>
> > > ---
> > > fs/nfs/client.c | 4 ++++
> > > 1 files changed, 4 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> > > index 5833fbb..a54c45e 100644
> > > --- a/fs/nfs/client.c
> > > +++ b/fs/nfs/client.c
> > > @@ -1867,6 +1867,10 @@ static int nfs_server_list_show(struct seq_file *m, void *v)
> > > /* display one transport per line on subsequent lines */
> > > clp = list_entry(v, struct nfs_client, cl_share_link);
> > >
> > > + /* Check if the client is initialized */
> > > + if (clp->cl_cons_state != NFS_CS_READY)
> > > + return 0;
> > > +
> > > seq_printf(m, "v%u %s %s %3d %s\n",
> > > clp->rpc_ops->version,
> > > rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_ADDR),
> > > --
> > > 1.7.0.4
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > > the body of a message to [email protected]
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Any comments on this patch?
Malahal Naineni [[email protected]] wrote:
> Malahal Naineni [[email protected]] wrote:
> > Myklebust, Trond [[email protected]] wrote:
> > > Shouldn't we just skip all nfs_client structures that don't have
> > > cl_cons_state set to NFS_CS_READY? I see no reason to display their
> > > properties even if they have got far enough as to create an rpc client.
> > >
> > > Cheers
> > > Trond
> >
> > Sounds good to me. Here is a tested patch.
>
> Any comments on this one?
>
> Thanks, Malahal.
>
> >
> >
> > As soon as the nfs_client gets created, its cl_rpcclient is set to
> > ERR_PTR(-EINVAL). The rpc client structure is allocated later. Check
> > if the client is ready before using the cl_rpcclient pointer.
> >
> > Signed-off-by: Malahal Naineni <[email protected]>
> > ---
> > fs/nfs/client.c | 4 ++++
> > 1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> > index 5833fbb..a54c45e 100644
> > --- a/fs/nfs/client.c
> > +++ b/fs/nfs/client.c
> > @@ -1867,6 +1867,10 @@ static int nfs_server_list_show(struct seq_file *m, void *v)
> > /* display one transport per line on subsequent lines */
> > clp = list_entry(v, struct nfs_client, cl_share_link);
> >
> > + /* Check if the client is initialized */
> > + if (clp->cl_cons_state != NFS_CS_READY)
> > + return 0;
> > +
> > seq_printf(m, "v%u %s %s %3d %s\n",
> > clp->rpc_ops->version,
> > rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_HEX_ADDR),
> > --
> > 1.7.0.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>