The goal here is to replace the head of a existing list pointed to by
'list' with a new head pointed to by 'nlist'.
First there is a memcpy that copies the contents of list to nlist then
this macro is called.
The macro makes sure that if the old head was empty then INIT_LIST_HEAD
the 'nlist', if not then make sure that the nodes before and after the
head now correclty point to nlist instead of list.
regards
shobhit
> ---------- Forwarded message ----------
> From: Christoph Hellwig <[email protected]>
> Date: Wed, 30 Mar 2005 12:14:39 +0100
> Subject: Re: [PATCH] Pageset Localization V2
> To: Christoph Lameter <[email protected]>
> Cc: Manfred Spraul <[email protected]>, Andrew Morton
> <[email protected]>, [email protected],
> [email protected], [email protected], [email protected]
>
>
> > +#define MAKE_LIST(list, nlist) \
> > + do { \
> > + if(list_empty(&list)) \
> > + INIT_LIST_HEAD(nlist); \
> > + else { nlist->next->prev = nlist; \
> > + nlist->prev->next = nlist; \
> > + } \
> > + }while(0)
>
> This is horrible. Where are the nlist pointers supposed to point to?
> What's so magic you need the INIT_LIST_HEAD only conditionally?
>
> -
> 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/
>
On Wed, Mar 30, 2005 at 06:36:18PM +0530, shobhit dayal wrote:
> The goal here is to replace the head of a existing list pointed to by
> 'list' with a new head pointed to by 'nlist'.
> First there is a memcpy that copies the contents of list to nlist then
> this macro is called.
> The macro makes sure that if the old head was empty then INIT_LIST_HEAD
> the 'nlist', if not then make sure that the nodes before and after the
> head now correclty point to nlist instead of list.
Which would be much nicer done using INIT_LIST_HEAD on the new head
always and then calling list_replace (of which currently only a _rcu variant
exists).
Note to Christoph: Just duplicating the code doesn't make it better ;-)
On Thu, Mar 31, 2005 at 03:32:35PM +0100, Christoph Hellwig wrote:
> Which would be much nicer done using INIT_LIST_HEAD on the new head
> always and then calling list_replace (of which currently only a _rcu variant
> exists).
INIT_LIST_HEAD followed by list_splice() should do the trick, I think.
BTW, is it a problem that the list head which the list was copied from
still points into the list?
--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain
On Thu, 31 Mar 2005, Christoph Hellwig wrote:
> On Wed, Mar 30, 2005 at 06:36:18PM +0530, shobhit dayal wrote:
> > The goal here is to replace the head of a existing list pointed to by
> > 'list' with a new head pointed to by 'nlist'.
> > First there is a memcpy that copies the contents of list to nlist then
> > this macro is called.
> > The macro makes sure that if the old head was empty then INIT_LIST_HEAD
> > the 'nlist', if not then make sure that the nodes before and after the
> > head now correclty point to nlist instead of list.
>
> Which would be much nicer done using INIT_LIST_HEAD on the new head
> always and then calling list_replace (of which currently only a _rcu variant
> exists).
>
> Note to Christoph: Just duplicating the code doesn't make it better ;-)
I will need the loop there for the prezeroing stuff later.
On Thu, 31 Mar 2005, Matthew Wilcox wrote:
> On Thu, Mar 31, 2005 at 03:32:35PM +0100, Christoph Hellwig wrote:
> > Which would be much nicer done using INIT_LIST_HEAD on the new head
> > always and then calling list_replace (of which currently only a _rcu variant
> > exists).
>
> INIT_LIST_HEAD followed by list_splice() should do the trick, I think.
> BTW, is it a problem that the list head which the list was copied from
> still points into the list?
The code runs during startup and the section containing the old pointers
is discarded at the end of the boot process.
On Thu, 31 Mar 2005, Matthew Wilcox wrote:
> On Thu, Mar 31, 2005 at 03:32:35PM +0100, Christoph Hellwig wrote:
> > Which would be much nicer done using INIT_LIST_HEAD on the new head
> > always and then calling list_replace (of which currently only a _rcu variant
> > exists).
>
> INIT_LIST_HEAD followed by list_splice() should do the trick, I think.
> BTW, is it a problem that the list head which the list was copied from
> still points into the list?
New patch replacing the old fix patch following your recipe:
Index: linux-2.6.11/mm/page_alloc.c
===================================================================
--- linux-2.6.11.orig/mm/page_alloc.c 2005-03-30 19:45:23.000000000 -0800
+++ linux-2.6.11/mm/page_alloc.c 2005-03-31 07:52:10.000000000 -0800
@@ -1613,15 +1613,6 @@ void zone_init_free_lists(struct pglist_
memmap_init_zone((size), (nid), (zone), (start_pfn))
#endif
-#define MAKE_LIST(list, nlist) \
- do { \
- if(list_empty(&list)) \
- INIT_LIST_HEAD(nlist); \
- else { nlist->next->prev = nlist; \
- nlist->prev->next = nlist; \
- } \
- }while(0)
-
/*
* Dynamicaly allocate memory for the
* per cpu pageset array in struct zone.
@@ -1629,6 +1620,7 @@ void zone_init_free_lists(struct pglist_
static inline int __devinit process_zones(int cpu)
{
struct zone *zone, *dzone;
+ int i;
for_each_zone(zone) {
struct per_cpu_pageset *npageset = NULL;
@@ -1642,10 +1634,13 @@ static inline int __devinit process_zone
if(zone->pageset[cpu]) {
memcpy(npageset, zone->pageset[cpu], sizeof(struct per_cpu_pageset));
- MAKE_LIST(zone->pageset[cpu]->pcp[0].list, (&npageset->pcp[0].list));
- MAKE_LIST(zone->pageset[cpu]->pcp[1].list, (&npageset->pcp[1].list));
- }
- else {
+
+ /* Relocate lists */
+ for(i = 0; i<2; i++) {
+ INIT_LIST_HEAD(&npageset->pcp[i].list);
+ list_splice(&zone->pageset[cpu]->pcp[i].list, &npageset->pcp[i].list);
+ }
+ } else {
struct per_cpu_pages *pcp;
unsigned long batch;
@@ -1721,11 +1716,14 @@ struct notifier_block pageset_notifier =
void __init setup_per_cpu_pageset()
{
- /*Iintialize per_cpu_pageset for cpu 0.
- A cpuup callback will do this for every cpu
- as it comes online
+ int err;
+
+ /* Initialize per_cpu_pageset for cpu 0.
+ * A cpuup callback will do this for every cpu
+ * as it comes online
*/
- BUG_ON(process_zones(smp_processor_id()));
+ err = process_zones(smp_processor_id());
+ BUG_ON(err);
register_cpu_notifier(&pageset_notifier);
}