Hi.
On Tue, 2006-10-24 at 22:08 +0200, Rafael J. Wysocki wrote:
> On Monday, 23 October 2006 06:14, Nigel Cunningham wrote:
> > Switch from bitmaps to using extents to record what swap is allocated;
> > they make more efficient use of memory, particularly where the allocated
> > storage is small and the swap space is large.
>
> As I said before, I like the overall idea, but I have a bunch of comments.
Thanks for them. Just a quick reply for the moment to say they're
appreciated and I will revise accordingly.
I should also mention that this isn't the only use of these functions in
Suspend2. There I also use extents to record the blocks to which the
image will be written. I hope to submit modifications to swsusp to do
that too in the near future.
> > +/* Simplify iterating through all the values in an extent chain */
> > +#define suspend_extent_for_each(extent_chain, extentpointer, value) \
> > +if ((extent_chain)->first) \
> > + for ((extentpointer) = (extent_chain)->first, (value) = \
> > + (extentpointer)->minimum; \
> > + ((extentpointer) && ((extentpointer)->next || (value) <= \
> > + (extentpointer)->maximum)); \
> > + (((value) == (extentpointer)->maximum) ? \
> > + ((extentpointer) = (extentpointer)->next, (value) = \
> > + ((extentpointer) ? (extentpointer)->minimum : 0)) : \
> > + (value)++))
>
> This macro doesn't look very nice and is used only once, so I think you
> can drop it and just write the loop where it belongs.
With the modifications I mentioned just above, this would also be used
for getting the blocks which match each swap extent. I can remove the
macro, but just want to make you aware that it does serve a purpose,
you're just not seeing it fully yet.
> > +
> > +void suspend_put_extent_chain(struct extent_chain *chain);
> > +int suspend_add_to_extent_chain(struct extent_chain *chain,
> > + unsigned long minimum, unsigned long maximum);
> > +
> > +/* swap_entry_to_extent_val & extent_val_to_swap_entry:
> > + * We are putting offset in the low bits so consecutive swap entries
> > + * make consecutive extent values */
> > +#define swap_entry_to_extent_val(swp_entry) (swp_entry.val)
> > +#define extent_val_to_swap_entry(val) (swp_entry_t) { (val) }
>
> These two macros are also used only once each. I'd just use the values
> directly.
Ok. Thanks. I think they're a leftover from 2.4 support :)
Nigel
-
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]
