On Tuesday October 3, [email protected] wrote:
> 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..." ?
>
Yes....
> > @@ -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 ;-)
Better safe that sorry? But yes, we would be very sorry if we tried
to allocate that many pages.
Thanks for the review.
NeilBrown
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
