On Fri, Oct 23, 2009 at 07:58:56PM -0400, Oren Laadan wrote:
>
>
> Matt Helsley wrote:
> > Currently we allocate memory to output all of the epoll items in one
> > big chunk. At 20 bytes per item, and since epoll was designed to
> > support on the order of 10,000 items, we may find ourselves kmalloc'ing
> > 200,000 bytes. That's an order 7 allocation whereas the heuristic for
> > difficult allocations, PAGE_ALLOC_COST_ORDER, is 3.
> >
> > Instead, output the epoll header and items separately. Chunk the output
> > much like the pid array gets chunked. This ensures that even sub-order 0
> > allocations will enable checkpoint of large epoll sets. A subsequent
> > patch will do something similar for the restore path.
> >
> > Signed-off-by: Matt Helsley <matthltc@xxxxxxxxxx>
>
> Acked-by: Oren Laadan <orenl@xxxxxxxxxxxxxxx>
>
> BTW, In the future, please use checkpatch.pl before sending;
OK, I'll add it to my script.
> Otherwise eventually I get yelled at ... :p
Hmm, I was suprised this triggered anything from checkpatch.
Some of the checkpatch.pl complaints look OK (omitted). Others don't
seem right.
WARNING: braces {} are not necessary for single statement blocks
#296: FILE: fs/eventpoll.c:1492:
+ for (i = 0, rbp = rb_first(&ep->rbr); rbp; rbp = rb_next(rbp),
i++) {}
This looks like a checkpatch bug. It's a zero statement block. The
braces prevent the compiler from putting the next statement in the loop body.
Cc'ing Andy Whitcroft.
ERROR: "(foo**)" should be "(foo **)"
#397: FILE: fs/eventpoll.c:1593:
+ ret = ckpt_read_payload(ctx, (void**)&items,
num_items*sizeof(*items),
Now that just seems dumb. ) is not an identifier so the "foo *bar" pattern
which seems to inspire this pattern does not apply. The only spaces in
typecasts should be like (struct foo*) IMHO. This looks weird since typecast
is a unary operator and there should be no space between a unary
operator and its argument. Imagine:
if (! x)
do_something()
This looks just as absurd to me:
(foo **)x Or worse: (foo **) x
Also CondingStyle says nothing about this from what I could see. So it
shouldn't be an ERROR IMHO. Ignoring this one in protest.
WARNING: line over 80 characters
#462: FILE: fs/eventpoll.c:1658:
+ ret = deferqueue_add_ptr(ctx->files_deferq, ctx,
ep_items_restore, NULL);
Over by one. My understanding is it's a warning for cases like this.
Ignoring.
...
total: 5 errors, 2 warnings, 457 lines checked
./checkpatchme.patch has style problems, please review. If any of these
errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Done.
> I'll also change the auto-tune-magic to fixed chunk, unless
> somebody screams.
*grumble* OK.
Cheers,
-Matt Helsley
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
