Message-ID: <285400e54a70548b4404ecdba9e44c4e8e6c5953.camel@kernel.org>
Subject: Re: [PATCH 4/4] nfs: fix debug message in svc_create_xprt()
From:   Jeff Layton <jlayton@kernel.org>
To:     Vasily Averin <vvs@virtuozzo.com>,
        "J. Bruce Fields" <bfields@fieldses.org>,
        Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna.schumaker@netapp.com>
Cc:     Chuck Lever <chuck.lever@oracle.com>, linux-nfs@vger.kernel.org,
        Evgenii Shatokhin <eshatokhin@virtuozzo.com>,
        Konstantin Khorenko <khorenko@virtuozzo.com>
Date:   Mon, 17 Dec 2018 12:56:44 -0500
In-Reply-To: <cff6406f-5c47-d0c6-64c8-48274c811d86@virtuozzo.com>
References: <cff6406f-5c47-d0c6-64c8-48274c811d86@virtuozzo.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.30.3 (3.30.3-1.fc29) 
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

On Mon, 2018-12-17 at 19:26 +0300, Vasily Averin wrote:
> _svc_create_xprt() returns positive port number 
> so its non-zero return value is not an error
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
> ---
>  net/sunrpc/svc_xprt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 51d36230b6e3..130af22d5c66 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -296,9 +296,9 @@ int svc_create_xprt(struct svc_serv *serv, const char *xprt_name,
>  		request_module("svc%s", xprt_name);
>  		err = _svc_create_xprt(serv, xprt_name, net, family, port, flags);
>  	}
> -	if (err)
> +	if (err < 0)
>  		dprintk("svc: transport %s not found, err %d\n",
> -			xprt_name, err);
> +			xprt_name, -err);
>  	return err;
>  }
>  EXPORT_SYMBOL_GPL(svc_create_xprt);

This patch is fine, but _svc_create_xprt does not make it clear that it
returns a positive port number.

Would you be amenable to spinning up a patch to add a kerneldoc comment
over that function (and maybe one over svc_create_xprt too), to make it
clear what it does?

Thanks,

Reviewed-by: Jeff Layton <jlayton@kernel.org>