Hi
> Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
>
> > Hi
> >
> > New shared snapshots are here. It incorporates Mike's changes and it has
> > reworked memory limit:
> > - a minimum of 8 buffers is guaranteed to prevent thrasing with too big
> > chunk sizes
> > - the cache for multisnapshots is limited to 2% of memory or 25% of
> > vmalloc memory (whichever is lower) [ I'm thinking about making this
> > configurable in /proc ]
> > - big chunk sizes (8MB or more) allocate memory always from vmalloc, there
> > is no point in allocating from general allocator
>
> For the benefit of others, here are your r17 changes relative to r16. I
> have some early questions/comments:
>
> - What is going on with EXPORT_SYMBOL at the top of drivers/md/dm-bufio.c?
> Do you intend to have a standalone dm-bufio module?
Maybe sometimes, but not now. That's why I mark exported symbols with
EXPORT_SYMBOL, but disable it.
> I think it makes
> sense to go one way or another. As is you're in limbo; the name of
> the _init and _exit functions don't _really_ make sense given that it
> isn't a standalone module (e.g. dm_bufio_module_init).
dm-bufio may become part of dm module --- then, dm_bufio_module_init and
dm_bufio_module_exit will be called from dm.c:_inits and _exits. Or
dm-bufio becomes a standalone module and then these functions will be
called from module_init and module_exit. Or it stays attached to
dm-store-mikulas, as it is now, and then it will be called from there.
I really don't know what will be the future of dm-bufio file --- will
fujita-daniel store use it? Will something else use it? (for example
replicator, SSD caching or whatever else). So I must be prepared for all
the alternatives.
> Makes the
> calling code in dm-multisnap-mikulas.c somewhat awkward (calling
> another _{module_init,exit}). Especially when you consider
> dm_bufio_module_exit() doesn't do anything;
It may do something in the future --- for example, unregister /sys or
/proc config files. (so far, it seems that it will be unnecessary if
sysfs is used...)
> yet you've reworked
> dm_multisnapshot_mikulas_module_init() to call it.
>
> - Please don't introduce long lines as you make new changes. Below, the
> following functions have unnecessarily long lines:
> get_memory_limit, dm_bufio_alloc_buffer_data, dm_bufio_module_init
Grr, people are still obsessed about it :-/
> - The empty newline between comment blocks and functions should be
> removed, see: get_memory_limit, adjust_client_count, dm_bufio_module_exit
OK.
Mikulas
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
