netdev - Re: [PATCH 10/10] 9P: Add cancelled() to the transport functions.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 02 Jul 2013 20:49:25 +0400
From:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:	Simon Derr <simon.derr@...l.net>
CC:	netdev@...r.kernel.org, ericvh@...il.com
Subject: Re: [PATCH 10/10] 9P: Add cancelled() to the transport functions.

Hello.

On 02-07-2013 17:11, Simon Derr wrote:

> RDMA needs to post a buffer for each incoming reply.
> Hence it needs to keep count of these and needs to be
> aware of whether a flushed request has received a reply
> or not.

> This patch adds the cancelled() callback to the transport modules.
> It is called when RFLUSH has been received and that the corresponding
> request will never receive a reply.

> Signed-off-by: Simon Derr <simon.derr@...l.net>
> ---
>   include/net/9p/transport.h |    3 +++
>   net/9p/client.c            |   11 ++++++++---
>   net/9p/trans_rdma.c        |   10 ++++++++++
>   3 files changed, 21 insertions(+), 3 deletions(-)

> diff --git a/include/net/9p/transport.h b/include/net/9p/transport.h
> index adcbb20..77a0578 100644
> --- a/include/net/9p/transport.h
> +++ b/include/net/9p/transport.h
[...]
> @@ -55,6 +57,7 @@ struct p9_trans_module {
>   	void (*close) (struct p9_client *);
>   	int (*request) (struct p9_client *, struct p9_req_t *req);
>   	int (*cancel) (struct p9_client *, struct p9_req_t *req);
> +	int (*cancelled) (struct p9_client *, struct p9_req_t *req);

    There should be no space before ( opening the parameter list.
scripts/checkpatch.pl should have warned you here.

>   	int (*zc_request)(struct p9_client *, struct p9_req_t *,
>   			  char *, char *, int , int, int, int);
>   };
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 44691b9..61e0455 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -676,11 +676,16 @@ static int p9_client_flush(struct p9_client *c, struct p9_req_t *oldreq)
>
>
>   	/* if we haven't received a response for oldreq,
> -	   remove it from the list. */
> +	   remove it from the list, and notify the transport
> +	   layer that the reply will never arrive. */

    The preferred style of the multi-line commnets in the networking 
code is:

	/* bla
	 * bla
	 */

>   	spin_lock(&c->lock);
> -	if (oldreq->status == REQ_STATUS_FLSH)
> +	if (oldreq->status == REQ_STATUS_FLSH) {
>   		list_del(&oldreq->req_list);
> -	spin_unlock(&c->lock);
> +		spin_unlock(&c->lock);
> +		if (c->trans_mod->cancelled)
> +			c->trans_mod->cancelled(c, req);
> +	} else
> +		spin_unlock(&c->lock);

    According to Documentation/CodingStyle chapter 3, both arms of the 
*if* statment should have {} if one arm has it.

> diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
> index 8f68df5..9e6c220 100644
> --- a/net/9p/trans_rdma.c
> +++ b/net/9p/trans_rdma.c
> @@ -588,6 +588,16 @@ static int rdma_cancel(struct p9_client *client, struct p9_req_t *req)
>   	return 1;
>   }
>
> +/* A request has been fully flushed without a reply.
> + * That means we have posted one buffer in excess.
> + */
> +static int rdma_cancelled(struct p9_client *client, struct p9_req_t *req)
> +{
> +	struct p9_trans_rdma *rdma = client->trans;

    Empty line wouldn't hurt here, after declaration.

> +	atomic_inc(&rdma->excess_rc);
> +	return 0;
> +}
> +

WBR, Sergei


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html