All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Banks <gnb@sgi.com>
To: Neil Brown <neilb@suse.de>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
	nfs@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 008 of 11] knfsd: Prepare knfsd for support of	rsize/wsize of up to 1MB, over TCP.
Date: Tue, 3 Oct 2006 18:02:02 +1000	[thread overview]
Message-ID: <20061003080202.GM28796@sgi.com> (raw)
In-Reply-To: <17697.63511.32591.797058@cse.unsw.edu.au>

On Tue, Oct 03, 2006 at 03:41:43PM +1000, Neil Brown wrote:
> Comments on the below?

Looks ok, except...

> @@ -57,7 +57,8 @@ struct svc_serv {
>  	struct svc_stat *	sv_stats;	/* RPC statistics */
>  	spinlock_t		sv_lock;
>  	unsigned int		sv_nrthreads;	/* # of server threads */
> -	unsigned int		sv_bufsz;	/* datagram buffer size */
> +	unsigned int		sv_max_payload;	/* datagram payload size */
> +	unsigned int		sv_max_mesg;	/* bufsz + 1 page for overheads */

Presumably the comment should read "max_payload + 1 page..." ?

> @@ -414,9 +415,11 @@ svc_init_buffer(struct svc_rqst *rqstp, 
>  	int pages;
>  	int arghi;
>  	
> -	if (size > RPCSVC_MAXPAYLOAD)
> -		size = RPCSVC_MAXPAYLOAD;
> -	pages = 2 + (size+ PAGE_SIZE -1) / PAGE_SIZE;
> +	if (size > RPCSVC_MAXPAYLOAD + PAGE_SIZE)
> +		size = RPCSVC_MAXPAYLOAD + PAGE_SIZE;
> +	pages = size + PAGE_SIZE; /* extra page as we hold both request and reply.
> +				   * We assume one is at most one page
> +				   */

Isn't there a divide by PAGE_SIZE missing here?  Looks
like we'll be allocating a *lot* of pages ;-)

Greg.
-- 
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

WARNING: multiple messages have this Message-ID (diff)
From: Greg Banks <gnb@sgi.com>
To: Neil Brown <neilb@suse.de>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
	nfs@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [NFS] [PATCH 008 of 11] knfsd: Prepare knfsd for support of	rsize/wsize of up to 1MB, over TCP.
Date: Tue, 3 Oct 2006 18:02:02 +1000	[thread overview]
Message-ID: <20061003080202.GM28796@sgi.com> (raw)
In-Reply-To: <17697.63511.32591.797058@cse.unsw.edu.au>

On Tue, Oct 03, 2006 at 03:41:43PM +1000, Neil Brown wrote:
> Comments on the below?

Looks ok, except...

> @@ -57,7 +57,8 @@ struct svc_serv {
>  	struct svc_stat *	sv_stats;	/* RPC statistics */
>  	spinlock_t		sv_lock;
>  	unsigned int		sv_nrthreads;	/* # of server threads */
> -	unsigned int		sv_bufsz;	/* datagram buffer size */
> +	unsigned int		sv_max_payload;	/* datagram payload size */
> +	unsigned int		sv_max_mesg;	/* bufsz + 1 page for overheads */

Presumably the comment should read "max_payload + 1 page..." ?

> @@ -414,9 +415,11 @@ svc_init_buffer(struct svc_rqst *rqstp, 
>  	int pages;
>  	int arghi;
>  	
> -	if (size > RPCSVC_MAXPAYLOAD)
> -		size = RPCSVC_MAXPAYLOAD;
> -	pages = 2 + (size+ PAGE_SIZE -1) / PAGE_SIZE;
> +	if (size > RPCSVC_MAXPAYLOAD + PAGE_SIZE)
> +		size = RPCSVC_MAXPAYLOAD + PAGE_SIZE;
> +	pages = size + PAGE_SIZE; /* extra page as we hold both request and reply.
> +				   * We assume one is at most one page
> +				   */

Isn't there a divide by PAGE_SIZE missing here?  Looks
like we'll be allocating a *lot* of pages ;-)

Greg.
-- 
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.

  reply	other threads:[~2006-10-03  8:03 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-24  6:36 [PATCH 000 of 11] knfsd: Introduction NeilBrown
2006-08-24  6:36 ` NeilBrown
2006-08-24  6:36 ` [PATCH 001 of 11] knfsd: nfsd: lockdep annotation fix NeilBrown
2006-08-24  6:36   ` NeilBrown
2006-08-24  6:36 ` [PATCH 002 of 11] knfsd: Fix a botched comment from the last patchset NeilBrown
2006-08-24  6:36   ` NeilBrown
2006-08-24  6:36 ` [PATCH 003 of 11] knfsd: call lockd_down when closing a socket via a write to nfsd/portlist NeilBrown
2006-08-24  6:36   ` NeilBrown
2006-08-24  6:36 ` [PATCH 004 of 11] knfsd: Protect update to sn_nrthreads with lock_kernel NeilBrown
2006-08-24  6:36   ` NeilBrown
2006-08-24  6:36 ` [PATCH 005 of 11] knfsd: Fixed handling of lockd fail when adding nfsd socket NeilBrown
2006-08-24  6:36   ` NeilBrown
2006-08-24  6:36 ` [PATCH 006 of 11] knfsd: Replace two page lists in struct svc_rqst with one NeilBrown
2006-08-24  6:36   ` NeilBrown
2006-08-24  6:37 ` [PATCH 007 of 11] knfsd: Avoid excess stack usage in svc_tcp_recvfrom NeilBrown
2006-08-24  6:37   ` NeilBrown
2006-08-24  6:37 ` [PATCH 008 of 11] knfsd: Prepare knfsd for support of rsize/wsize of up to 1MB, over TCP NeilBrown
2006-08-24  6:37   ` NeilBrown
2006-09-25 15:43   ` J. Bruce Fields
2006-09-25 15:43     ` [NFS] " J. Bruce Fields
2006-09-28  3:41     ` Neil Brown
2006-09-28  3:41       ` [NFS] " Neil Brown
2006-09-28  3:46       ` Andrew Morton
2006-09-28  3:46         ` [NFS] " Andrew Morton
2006-10-03  1:36     ` Neil Brown
2006-10-03  1:36       ` [NFS] " Neil Brown
2006-10-03  1:59       ` Greg Banks
2006-10-03  1:59         ` [NFS] " Greg Banks
2006-10-03  2:13       ` J. Bruce Fields
2006-10-03  2:13         ` [NFS] " J. Bruce Fields
2006-10-03  5:41         ` Neil Brown
2006-10-03  5:41           ` [NFS] " Neil Brown
2006-10-03  8:02           ` Greg Banks [this message]
2006-10-03  8:02             ` Greg Banks
2006-10-05  7:07             ` Neil Brown
2006-10-05  7:07               ` [NFS] " Neil Brown
2006-08-24  6:37 ` [PATCH 009 of 11] knfsd: Allow max size of NFSd payload to be configured NeilBrown
2006-08-24  6:37   ` NeilBrown
2006-09-25 21:24   ` J. Bruce Fields
2006-09-25 21:24     ` [NFS] " J. Bruce Fields
2006-09-28  4:22     ` Neil Brown
2006-09-28  4:22       ` [NFS] " Neil Brown
2006-09-28 17:09       ` Hugh Dickins
2006-09-28 17:09         ` [NFS] " Hugh Dickins
2006-09-29  1:59         ` Neil Brown
2006-09-29  1:59           ` [NFS] " Neil Brown
2006-08-24  6:37 ` [PATCH 010 of 11] knfsd: make nfsd readahead params cache SMP-friendly NeilBrown
2006-08-24  6:37   ` NeilBrown
2006-08-24  6:37 ` [PATCH 011 of 11] knfsd: knfsd: cache ipmap per TCP socket NeilBrown
2006-08-24  6:37   ` NeilBrown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20061003080202.GM28796@sgi.com \
    --to=gnb@sgi.com \
    --cc=bfields@fieldses.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=nfs@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.