Messages in this thread | | | Subject | Re: [PATCH] Use extents for recording what swap is allocated. | From | Nigel Cunningham <> | Date | Wed, 25 Oct 2006 09:05:01 +1000 |
| |
Hi.
On Wed, 2006-10-25 at 00:45 +0200, Rafael J. Wysocki wrote: > 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! :-)
I understand that, but some things won't make sense or seem as useful if I don't give you the extra information.
> > 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).
Ok.
Nigel
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |