Some comments on the patch: > + /* Don't let userspace make us allocate a huge buffer */ > + if (cmd.ne > 256) > + return -ENOMEM; > +
Is this necessary? Won't the following fail with ENOMEM anyway if
cmd.ne is too big:
> wc = kmalloc(cmd.ne * sizeof *wc, GFP_KERNEL);
> if (!wc)
> return -ENOMEM;
Same here:
> + /* Don't let userspace make us allocate a huge buffer */
> + if (cmd.wqe_size > 4096)
> + return -ENOMEM;
> +
> + user_wr = kmalloc(cmd.wqe_size, GFP_KERNEL);
> + if (!user_wr)
> + return -ENOMEM;
> +
What meaning do those numbers have, exactly? i.e. the 4096 number
above?
> + if (ret) {
> + for (next = wr; next; next = next->next) {
> + if (next == bad_wr)
> + break;
> + ++resp.bad_wr;
> + }
> + }
Will this work? If bad_wr is the first wr, then resp.bad_wr will be
zero. The current user code (which has to change anyway) assumes 0 ==
"no bad wr" and 1 == "the first wr is the bad wr", etc.
> + while (wr) {
> + next = wr->next;
> + kfree(wr);
> + wr = next;
> + }
> +
> + kfree(user_wr);
One reason why I originally allocated one big wr area instead of a bunch
of smaller ones was to keep the cost of this down. Is it a good idea to
be doing this with a bunch of kmallocs?
This is all I've had a chance to look at for the moment. More later.
Regards,
Robert.
--
Robert Walsh Email: [EMAIL PROTECTED]
PathScale, Inc. Phone: +1 650 934 8117
2071 Stierlin Court, Suite 200 Fax: +1 650 428 1969
Mountain View, CA 94043
signature.asc
Description: This is a digitally signed message part
_______________________________________________ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
