Re: [PATCH v3 3/3] Move reusable parts of memory pool into its own file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jameson Miller <jamill@xxxxxxxxxxxxx> writes:

> This moves the reusable parts of the memory pool logic used by
> fast-import.c into its own file for use by other components.
>
> Signed-off-by: Jameson Miller <jamill@xxxxxxxxxxxxx>
> ---
>  Makefile      |  1 +
>  fast-import.c | 70 +----------------------------------------------------------
>  mem-pool.c    | 55 ++++++++++++++++++++++++++++++++++++++++++++++
>  mem-pool.h    | 34 +++++++++++++++++++++++++++++
>  4 files changed, 91 insertions(+), 69 deletions(-)
>  create mode 100644 mem-pool.c
>  create mode 100644 mem-pool.h

OK.  This is indeed straight-forward line movements and nothing else,
other than obviously a few static helpers are now extern.

I said I'd anticipate that the allocation that bypasses the pool
subsystem would want to become traceable by the pool subsystem,
which would allow us to free() the pieces of memory allocated
directly with xmalloc() in mem_pool_alloc() instead of leaking.  I
am OK if the structure of this series is to make that change after
these three steps we see here.  When that happens, it will start to
make sense to bill the "this is too big so do not attempt to carve
it out from block, and do not overallocate and make the remainder
available for later requests" to the pool instance mem_pool_alloc()
is working on, as that piece of memory is also known to a specific
pool instance.

After I wrote review for 2/3, I found out that you changed the
meaning of total_allocd (which should probably be described in its
log message).  Unlike the original that counted "total", it now is
used only for memory that is allocated directly by fast-import.c and
does not account for memory obtained by calling mem-pool.

The output routine is changed in 2/3 to add fi_mem_pool's pool_alloc
to it, so billing oversized allocation that does *not* belong to any
specific pool to _some_ pool and ignoring total_allocd would cancel
things out.  It still feels a bit fishy, but I think it is OK.

So all in all, I think we are in no worse shape than the original
(we call it "bug-to-bug compatible" ;-)), and successfully extracted
a reusable piece in a separate file in a clean way so that we can
refine and extend it further.  Nicely done.

Will queue; the proposed log for step 2/3 may want to be a bit
polished, though.

Thanks.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux