Re: [PATCH] Use extents for recording what swap is allocated.

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

 



Hi,

On Wednesday, 25 October 2006 00:13, Nigel Cunningham wrote:
> 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.

Could we please focus on things that are on the table _now_?.  You are
submitting the patch aganist the current code and I can only review it
in this context.  I can't say if I like your _future_ patches at this moment! :-)

> 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.

Can we just assume there are no other patches and proceed under this
assumption?

Could you please remove the macro for now?  You can introduce it with the
other patches when you submit them (if it's still needed at that time).

Greetings,
Rafael


-- 
You never change things by fighting the existing reality.
		R. Buckminster Fuller
-
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]
  Powered by Linux