Re: [Announce] [patch] Modular Scheduler Core and Completely Fair Scheduler [CFS]
Ingo Molnar wrote: [announce] [patch] Modular Scheduler Core and Completely Fair Scheduler [CFS] i'm pleased to announce the first release of the Modular Scheduler Core and Completely Fair Scheduler [CFS] patchset: http://redhat.com/~mingo/cfs-scheduler/sched-modular+cfs.patch This project is a complete rewrite of the Linux task scheduler. My goal is to address various feature requests and to fix deficiencies in the vanilla scheduler that were suggested/found in the past few years, both for desktop scheduling and for server scheduling workloads. [ QuickStart: apply the patch to v2.6.21-rc6, recompile, reboot. The new scheduler will be active by default and all tasks will default to the new SCHED_FAIR interactive scheduling class. ] Highlights are: - the introduction of Scheduling Classes: an extensible hierarchy of scheduler modules. These modules encapsulate scheduling policy details and are handled by the scheduler core without the core code assuming about them too much. - sched_fair.c implements the 'CFS desktop scheduler': it is a replacement for the vanilla scheduler's SCHED_OTHER interactivity code. i'd like to give credit to Con Kolivas for the general approach here: he has proven via RSDL/SD that 'fair scheduling' is possible and that it results in better desktop scheduling. Kudos Con! The CFS patch uses a completely different approach and implementation from RSDL/SD. My goal was to make CFS's interactivity quality exceed that of RSDL/SD, which is a high standard to meet :-) Testing feedback is welcome to decide this one way or another. [ and, in any case, all of SD's logic could be added via a kernel/sched_sd.c module as well, if Con is interested in such an approach. ] CFS's design is quite radical: it does not use runqueues, it uses a time-ordered rbtree to build a 'timeline' of future task execution, and thus has no 'array switch' artifacts (by which both the vanilla scheduler and RSDL/SD are affected). CFS uses nanosecond granularity accounting and does not rely on any jiffies or other HZ detail. Thus the CFS scheduler has no notion of 'timeslices' and has no heuristics whatsoever. There is only one central tunable: /proc/sys/kernel/sched_granularity_ns which can be used to tune the scheduler from 'desktop' (low latencies) to 'server' (good batching) workloads. It defaults to a setting suitable for desktop workloads. SCHED_BATCH is handled by the CFS scheduler module too. due to its design, the CFS scheduler is not prone to any of the 'attacks' that exist today against the heuristics of the stock scheduler: fiftyp.c, thud.c, chew.c, ring-test.c, massive_intr.c all work fine and do not impact interactivity and produce the expected behavior. the CFS scheduler has a much stronger handling of nice levels and SCHED_BATCH: both types of workloads should be isolated much more agressively than under the vanilla scheduler. ( another rdetail: due to nanosec accounting and timeline sorting, sched_yield() support is very simple under CFS, and in fact under CFS sched_yield() behaves much better than under any other scheduler i have tested so far. ) - sched_rt.c implements SCHED_FIFO and SCHED_RR semantics, in a simpler way than the vanilla scheduler does. It uses 100 runqueues (for all 100 RT priority levels, instead of 140 in the vanilla scheduler) and it needs no expired array. - reworked/sanitized SMP load-balancing: the runqueue-walking assumptions are gone from the load-balancing code now, and iterators of the scheduling modules are used. The balancing code got quite a bit simpler as a result. the core scheduler got smaller by more than 700 lines: kernel/sched.c | 1454 1 file changed, 372 insertions(+), 1082 deletions(-) and even adding all the scheduling modules, the total size impact is relatively small: 18 files changed, 1454 insertions(+), 1133 deletions(-) most of the increase is due to extensive comments. The kernel size impact is in fact a small negative: textdata bss dec hex filename 233664001 24 273916aff kernel/sched.o.vanilla 241592705 56 269206928 kernel/sched.o.CFS (this is mainly due to the benefit of getting rid of the expired array and its data structure overhead.) thanks go to Thomas Gleixner and Arjan van de Ven for review of this patchset. as usual, any sort of feedback, bugreports, fixes and suggestions are more than welcome, Pushed this through the test.kernel.org and nothing new blew up. Notably the kernbench figures are within expectations even on the bigger numa systems, commonly badly affected by balancing problems in the schedular. I see there is a second one out,
[PATCH 2/3] lumpy: increase pressure at the end of the inactive list
Having selected an area at the end of the inactive list, reclaim is attempted for all LRU pages within that contiguous area. Currently, any pages in this area found to still be active or referenced are rotated back to the active list as normal and the rest reclaimed. At low orders there is a reasonable likelyhood of finding contigious inactive areas for reclaim. However when reclaiming at higher order there is a very low chance all pages in the area being inactive, unreferenced and therefore reclaimable. This patch modifies behaviour when reclaiming at higher order (order = 4). All LRU pages within the target area are reclaimed, including both active and recently referenced pages. [EMAIL PROTECTED]: additionally apply pressure to referenced paged] Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] Acked-by: Mel Gorman [EMAIL PROTECTED] --- diff --git a/mm/vmscan.c b/mm/vmscan.c index 466435f..e5e77fb 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -472,7 +472,7 @@ static unsigned long shrink_page_list(struct list_head *page_list, referenced = page_referenced(page, 1); /* In active use or really unfreeable? Activate it. */ - if (referenced page_mapping_inuse(page)) + if (sc-order = 3 referenced page_mapping_inuse(page)) goto activate_locked; #ifdef CONFIG_SWAP @@ -505,7 +505,7 @@ static unsigned long shrink_page_list(struct list_head *page_list, } if (PageDirty(page)) { - if (referenced) + if (sc-order = 3 referenced) goto keep_locked; if (!may_enter_fs) goto keep_locked; @@ -599,6 +599,7 @@ keep: * * returns 0 on success, -ve errno on failure. */ +#define ISOLATE_BOTH -1/* Isolate both active and inactive pages. */ static int __isolate_lru_page(struct page *page, int active) { int ret = -EINVAL; @@ -608,7 +609,8 @@ static int __isolate_lru_page(struct page *page, int active) * dealing with comparible boolean values. Take the logical not * of each. */ - if (PageLRU(page) (!PageActive(page) == !active)) { + if (PageLRU(page) (active == ISOLATE_BOTH || + (!PageActive(page) == !active))) { ret = -EBUSY; if (likely(get_page_unless_zero(page))) { /* @@ -729,6 +731,26 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, } /* + * deactivate_pages() is a helper for shrink_active_list(), it deactivates + * all active pages on the passed list. + */ +static unsigned long deactivate_pages(struct list_head *page_list) +{ + int nr_active = 0; + struct list_head *entry; + + list_for_each(entry, page_list) { + struct page *page = list_entry(entry, struct page, lru); + if (PageActive(page)) { + ClearPageActive(page); + nr_active++; + } + } + + return nr_active; +} + +/* * shrink_inactive_list() is a helper for shrink_zone(). It returns the number * of reclaimed pages */ @@ -749,11 +771,17 @@ static unsigned long shrink_inactive_list(unsigned long max_scan, unsigned long nr_taken; unsigned long nr_scan; unsigned long nr_freed; + unsigned long nr_active; nr_taken = isolate_lru_pages(sc-swap_cluster_max, zone-inactive_list, -page_list, nr_scan, sc-order, 0); - __mod_zone_page_state(zone, NR_INACTIVE, -nr_taken); +page_list, nr_scan, sc-order, +(sc-order 3)? ISOLATE_BOTH : 0); + nr_active = deactivate_pages(page_list); + + __mod_zone_page_state(zone, NR_ACTIVE, -nr_active); + __mod_zone_page_state(zone, NR_INACTIVE, + -(nr_taken - nr_active)); zone-pages_scanned += nr_scan; zone-total_scanned += nr_scan; spin_unlock_irq(zone-lru_lock); - 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/
[PATCH 3/3] introduce HIGH_ORDER delineating easily reclaimable orders
The memory allocator treats lower order (order = 3) and higher order (order = 4) allocations in slightly different ways. As lower orders are much more likely to be available and also more likely to be simply reclaimed it is deemed reasonable to wait longer for those. Lumpy reclaim also changes behaviour at this same boundary, more agressivly targetting pages in reclaim at higher order. This patch removes all these magical numbers and replaces with with a constant HIGH_ORDER. Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] Acked-by: Mel Gorman [EMAIL PROTECTED] --- diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h index 8c87d79..f9d2ced 100644 --- a/include/linux/mmzone.h +++ b/include/linux/mmzone.h @@ -25,6 +25,13 @@ #endif #define MAX_ORDER_NR_PAGES (1 (MAX_ORDER - 1)) +/* + * The boundary between small and large allocations. That is between + * allocation orders which should colesce naturally under reasonable + * reclaim pressure and those which will not. + */ +#define HIGH_ORDER 3 + #ifdef CONFIG_PAGE_GROUP_BY_MOBILITY #define MIGRATE_UNMOVABLE 0 #define MIGRATE_RECLAIMABLE 1 diff --git a/mm/page_alloc.c b/mm/page_alloc.c index d7e33cb..44786d9 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1768,7 +1768,7 @@ nofail_alloc: */ do_retry = 0; if (!(gfp_mask __GFP_NORETRY)) { - if ((order = 3) || (gfp_mask __GFP_REPEAT)) + if ((order = HIGH_ORDER) || (gfp_mask __GFP_REPEAT)) do_retry = 1; if (gfp_mask __GFP_NOFAIL) do_retry = 1; diff --git a/mm/vmscan.c b/mm/vmscan.c index e5e77fb..79aedcb 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -472,7 +472,8 @@ static unsigned long shrink_page_list(struct list_head *page_list, referenced = page_referenced(page, 1); /* In active use or really unfreeable? Activate it. */ - if (sc-order = 3 referenced page_mapping_inuse(page)) + if (sc-order = HIGH_ORDER + referenced page_mapping_inuse(page)) goto activate_locked; #ifdef CONFIG_SWAP @@ -505,7 +506,7 @@ static unsigned long shrink_page_list(struct list_head *page_list, } if (PageDirty(page)) { - if (sc-order = 3 referenced) + if (sc-order = HIGH_ORDER referenced) goto keep_locked; if (!may_enter_fs) goto keep_locked; @@ -774,9 +775,9 @@ static unsigned long shrink_inactive_list(unsigned long max_scan, unsigned long nr_active; nr_taken = isolate_lru_pages(sc-swap_cluster_max, -zone-inactive_list, -page_list, nr_scan, sc-order, -(sc-order 3)? ISOLATE_BOTH : 0); +zone-inactive_list, +page_list, nr_scan, sc-order, +(sc-order HIGH_ORDER)? ISOLATE_BOTH : 0); nr_active = deactivate_pages(page_list); __mod_zone_page_state(zone, NR_ACTIVE, -nr_active); - 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/
[PATCH 1/3] kswapd: use reclaim order in background reclaim
When an allocator has to dip below the low water mark for a zone, kswapd is awoken to start background reclaim. The highest order of these dipping allocations are accumulated on the zone. With this patch kswapd uses this hint to force reclaim at that order via balance_pgdat(). Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] Acked-by: Mel Gorman [EMAIL PROTECTED] --- diff --git a/mm/vmscan.c b/mm/vmscan.c index 428da1a..466435f 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1212,6 +1212,7 @@ static unsigned long balance_pgdat(pg_data_t *pgdat, int order) .may_swap = 1, .swap_cluster_max = SWAP_CLUSTER_MAX, .swappiness = vm_swappiness, + .order = order, }; /* * temp_priority is used to remember the scanning priority at which - 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/
[PATCH 0/3] Lumpy Reclaim V6
Following this email are three patches to the lumpy reclaim algorithm. These apply on top of the lumpy patches in 2.6.21-rc6-mm1 (lumpy V5); making lumpy V6. The first enables kswapd to apply reclaim at the order of the allocations which trigger background reclaim. The second increases pressure on the area at the end of the inactive list. The last introduces a new symbolic constant representing the boundary between easily reclaimed areas and those where extra pressure is applicable. Andrew, please consider for -mm. Comparitive testing between lumpy-V5 and lumpy-V6 shows a considerable improvement when under extreme load. lumpy-V5 relies on the pages in an area being rotated onto the inactive list together and remaining inactive long enough to be reclaimed from that list. Under high load a significant portion of the pages return to active or are referenced before this can occur. Lumpy-V6 targets all LRU pages in the area greatly increasing the chance of reclaiming it completely. kswapd-use-reclaim-order-in-background-reclaim: When an allocator has to dip below the low water mark for a zone, kswapd is awoken to start background reclaim. Make kswapd use the highest order of these allocations to define the order at which it reclaims. lumpy-increase-pressure-at-the-end-of-the-inactive-list: when reclaiming at higher order target all pages in the contigious area for reclaim, including active and recently referenced pages. This increases the chances of that area becoming free. introduce-HIGH_ORDER-delineating-easily-reclaimable-orders: The memory allocator treats lower and higher order allocations slightly differently. Lumpy reclaim also changes behaviour at this same boundary. Pull out the magic numbers and replace them with a symbolic constant. Against: 2.6.21-rc6-mm1 -apw Changes in lumpy V5: Andy Whitcroft: lumpy: back out removal of active check in isolate_lru_pages lumpy: only count taken pages as scanned Changes in lumpy V4: Andy Whitcroft: lumpy: isolate_lru_pages wants to specifically take active or inactive pages lumpy: ensure that we compare PageActive and active safely lumpy: update commentry on subtle comparisons and rounding assumptions lumpy: only check for valid pages when holes are present Changes in lumpy V3: Adrian Bunk: lumpy-reclaim-cleanup Andrew Morton: lumpy-reclaim-v2-page_to_pfn-fix lumpy-reclaim-v2-tidy Andy Whitcroft: lumpy: ensure we respect zone boundaries lumpy: take the other active/inactive pages in the area - 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/
Re: [PATCH 3/3] introduce HIGH_ORDER delineating easily reclaimable orders
Andrew Morton wrote: On Sat, 21 Apr 2007 01:28:43 -0700 Andrew Morton [EMAIL PROTECTED] wrote: It would have been better to have patched page_alloc.c independently, then to have used HIGH_ORDER in lumpy: increase pressure at the end of the inactive list. Actually that doesn't matter, because I plan on lumping all the lumpy patches together into one lump. I was going to duck patches #2 and #3, such was my outrage. But given that it's all lined up to be a single patch, followup cleanup patches will fit in OK. Please. Yes. Its funny how you can get so close to a change that you can no longer see the obvious warts on it. I am actually travelling today, so it'll be tommorrow now. But I'll roll the cleanups and get them to you. I can also offer you a clean drop in lumpy stack with the HIGH_ORDER change pulled out to the top once you are happy. -apw - 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/
[PATCH 0/3] Lumpy Reclaim V5
Following this email are three patches which represent the current state of the lumpy reclaim patches; collectively lumpy V5. This patch kit is against 2.6.21-rc3-mm2. This stack is split out to show the incremental changes in this version. This contains one fixup following on from Christoph Lameters feedback and one change affecting scan rates. Andrew, please consider for -mm. Comparitive testing between lumpy-V4 and lump-V5 generally shows a small improvement, coming from the slight increase in scanning coming from second of the patches. I have taken the lumpy-V3 patches and the last batch of changes and folded them back into a single patch (collectively lumpy-V4), updating attribution. On top of this are are two patches the first the result of feedback from Christoph and the latter a change which I believe is a correctness issue for scanning rates: lumpy-reclaim-V4: folded back base, changes incorporated are listed in the changelog which is included in the patch. lumpy-back-out-removal-of-active-check-in-isolate_lru_pages: reinstating a BUG where the active state missmatched the lru we are scanning. As pointed out by Christoph Lameter, there should not be a missmatch and testing confirms with this base there are none. lumpy-only-count-taken-pages-as-scanned: when scanning an area around a target page taken from the LRU we will only take pages which match the active state. Previously we would count the missmatching pages passed over as 'scanned'. Prior to lumpy a page was only counted as 'scanned' if we had removed it from the LRU and reclaimed or rotated it back to the list. This leads to reduced reclaim scanning and affects reclaim performance. Move to counting pages as scanned only when actually touched. Against: 2.6.21-rc3-mm2 -apw - 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/
[PATCH 1/3] Lumpy Reclaim V4
When we are out of memory of a suitable size we enter reclaim. The current reclaim algorithm targets pages in LRU order, which is great for fairness at order-0 but highly unsuitable if you desire pages at higher orders. To get pages of higher order we must shoot down a very high proportion of memory; 95% in a lot of cases. This patch set adds a lumpy reclaim algorithm to the allocator. It targets groups of pages at the specified order anchored at the end of the active and inactive lists. This encourages groups of pages at the requested orders to move from active to inactive, and active to free lists. This behaviour is only triggered out of direct reclaim when higher order pages have been requested. This patch set is particularly effective when utilised with an anti-fragmentation scheme which groups pages of similar reclaimability together. This patch set is based on Peter Zijlstra's lumpy reclaim V2 patch which forms the foundation. Credit to Mel Gorman for sanitity checking. [EMAIL PROTECTED]: ia64 pfn_to_nid fixes and loop cleanup] [EMAIL PROTECTED]: static declarations for internal functions] [EMAIL PROTECTED]: initial lumpy V2 implementation] Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] Acked-by: Peter Zijlstra [EMAIL PROTECTED] Acked-by: Mel Gorman [EMAIL PROTECTED] Signed-off-by: Andrew Morton [EMAIL PROTECTED] --- 8 Changes in lumpy V4: Andy Whitcroft: lumpy: isolate_lru_pages wants to specifically take active or inactive pages lumpy: ensure that we compare PageActive and active safely lumpy: update commentry on subtle comparisons and rounding assumptions lumpy: only check for valid pages when holes are present Changes in lumpy V3: Adrian Bunk: lumpy-reclaim-cleanup Andrew Morton: lumpy-reclaim-v2-page_to_pfn-fix lumpy-reclaim-v2-tidy Andy Whitcroft: lumpy: ensure we respect zone boundaries lumpy: take the other active/inactive pages in the area --- diff --git a/fs/buffer.c b/fs/buffer.c index 8666f62..a9273ab 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -371,7 +371,7 @@ static void free_more_memory(void) for_each_online_pgdat(pgdat) { zones = pgdat-node_zonelists[gfp_zone(GFP_NOFS)].zones; if (*zones) - try_to_free_pages(zones, GFP_NOFS); + try_to_free_pages(zones, 0, GFP_NOFS); } } diff --git a/include/linux/swap.h b/include/linux/swap.h index a461480..5b9ee5a 100644 --- a/include/linux/swap.h +++ b/include/linux/swap.h @@ -189,7 +189,8 @@ extern int rotate_reclaimable_page(struct page *page); extern void swap_setup(void); /* linux/mm/vmscan.c */ -extern unsigned long try_to_free_pages(struct zone **, gfp_t); +extern unsigned long try_to_free_pages(struct zone **zones, int order, + gfp_t gfp_mask); extern unsigned long shrink_all_memory(unsigned long nr_pages); extern int vm_swappiness; extern int remove_mapping(struct address_space *mapping, struct page *page); diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 6abe947..23917f8 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1694,7 +1694,7 @@ nofail_alloc: reclaim_state.reclaimed_slab = 0; p-reclaim_state = reclaim_state; - did_some_progress = try_to_free_pages(zonelist-zones, gfp_mask); + did_some_progress = try_to_free_pages(zonelist-zones, order, gfp_mask); p-reclaim_state = NULL; p-flags = ~PF_MEMALLOC; diff --git a/mm/vmscan.c b/mm/vmscan.c index 963a1c4..bda63a0 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -68,6 +68,8 @@ struct scan_control { int swappiness; int all_unreclaimable; + + int order; }; /* @@ -611,6 +613,41 @@ keep: } /* + * Attempt to remove the specified page from its LRU. Only take this page + * if it is of the appropriate PageActive status. Pages which are being + * freed elsewhere are also ignored. + * + * page: page to consider + * active: active/inactive flag only take pages of this type + * + * returns 0 on success, -ve errno on failure. + */ +static int __isolate_lru_page(struct page *page, int active) +{ + int ret = -EINVAL; + + /* +* When checking the active state, we need to be sure we are +* dealing with comparible boolean values. Take the logical not +* of each. +*/ + if (PageLRU(page) (!PageActive(page) == !active)) { + ret = -EBUSY; + if (likely(get_page_unless_zero(page))) { + /* +* Be careful not to clear PageLRU until after we're +* sure the page is not being freed elsewhere -- the +* page release code relies on it. +*/ + ClearPageLRU(page); + ret = 0; + } + } + + return ret; +} + +/* * zone-lru_lock is heavily
[PATCH 2/3] lumpy: back out removal of active check in isolate_lru_pages
As pointed out by Christop Lameter it should not be possible for a page to change its active/inactive state without taking the lru_lock. Reinstate this safety net. Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] Acked-by: Mel Gorman [EMAIL PROTECTED] --- diff --git a/mm/vmscan.c b/mm/vmscan.c index bda63a0..d7a0860 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -691,10 +691,13 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, nr_taken++; break; - default: - /* page is being freed, or is a missmatch */ + case -EBUSY: + /* else it is being freed elsewhere */ list_move(page-lru, src); continue; + + default: + BUG(); } if (!order) - 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/
[PATCH 3/3] lumpy: only count taken pages as scanned
When scanning the order sized area around the tag page we pull all pages of the matching active state; the non-matching pages are not otherwise affected. We currently count these as scanned increasing the apparent scan rates. Previously we would only count a page scanned if it was actually removed from the LRU, either then being reclaimed or rotated back onto the head of the LRU. The effect of this is to cause reclaim to terminate artificially early when the scan count is reached, reducing effectivness. Move to counting only those pages we actually remove from the LRU as scanned. Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] Acked-by: Mel Gorman [EMAIL PROTECTED] --- diff --git a/mm/vmscan.c b/mm/vmscan.c index d7a0860..c3dc544 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -732,11 +732,11 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, /* Check that we have not crossed a zone boundary. */ if (unlikely(page_zone_id(cursor_page) != zone_id)) continue; - scan++; switch (__isolate_lru_page(cursor_page, active)) { case 0: list_move(cursor_page-lru, dst); nr_taken++; + scan++; break; case -EBUSY: - 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/
Re: [PATCH 1/3] Lumpy Reclaim V4
Dave Hansen wrote: On Mon, 2007-03-12 at 18:23 +, Andy Whitcroft wrote: + /* The target page is in the block, ignore it. */ + if (unlikely(pfn == page_pfn)) + continue; +#ifdef CONFIG_HOLES_IN_ZONE + /* Avoid holes within the zone. */ + if (unlikely(!pfn_valid(pfn))) + break; +#endif Would having something like: static inline int pfn_in_zone_hole(unsigned long pfn) { #ifdef CONFIG_HOLES_IN_ZONE if (unlikely(!pfn_valid(pfn))) return 1; #endif return 0; } help us out? page_is_buddy() and page_is_consistent() appear to do the exact same thing, with the same #ifdef. Funny you mention that. I have a patch hanging around which basically does that. I'd been planning to send it up. It adds a pfn_valid_within() which you use when you already know a relative page within the MAX_ORDER block is valid. I'd not sent it cause I thought the name sucked. -apw - 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/
Re: [PATCH 0/3] Lumpy Reclaim V5
Andrew Morton wrote: On Mon, 12 Mar 2007 18:22:45 + Andy Whitcroft [EMAIL PROTECTED] wrote: Following this email are three patches which represent the current state of the lumpy reclaim patches; collectively lumpy V5. So where do we stand with this now?Does it make anything get better? I am still working to fairly compare the various combinations. One of the problems is that if you push any reclaim algorithm to its physical limits you will get the same overall success rates. I think there is still some work to do refining lumpy, and reclaim in general. But I feel what we have now is pretty solid base for that. I (continue to) think that if this is to be truly useful, we need some way of using it from kswapd to keep a certain minimum number of order-1, order-2, etc pages in the freelists. I think this is a key component of the mix and am just starting to play with this. I hope that this can provide improvements in the instantaneous availability of these higher orders and improve average latency. -apw - 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/
Re: [PATCH 00/18] Make common x86 arch area for i386 and x86_64 - Take 2
Martin Bligh wrote: Christoph Lameter wrote: On Fri, 16 Mar 2007, Martin Bligh wrote: You have to do some sort of lookup anyway, and Andy seemed to have them all folded into one. What lookup would you need to do? On x86_64 even the TLB use is hidden by the existing 2M entries for 1-1 mappings. Or are you trying to avoid this by going to back to the crud we had in 2.4 where we pretend mem_map is one big array, indexed by pfn with huge sparsely mapped holes in it? Yes that the advanced way of doing it rather than adding useless custom lookups. For starters, you can't do that sparse a mapping on a 32 bit system. I'll let Andy explain the rest of it. Would be nice to work out (and document somewhere) what the pros and cons of virtual memmap vs sparsemem were - ISTR one of the arguments was extremely sparsely layed out machines, and you needed sparsemem for that. But right now we have 3 solutions, which is not a good situation. Please read my posts to linux-mm on that subject. We discussed it last year in detail and the agreement was that the sparsemem crud needs to be taken out. Kame-san posted patches to do that. the agreement? So Andy agreed to taking it out? Or you and Kame did? The discussions centred around some patches from Kame which introduced a SPARSMEM sub-model with a virtual memory map. That was a supprisingly clean change which if followed through to its logical conclusion would remove a significant chunk of architecture specific vmemmap implementation from ia64, and (as I understand it) was likely to allow the same to be reused in s390x as well. SPARSEMEM would still have its useful modes for smaller memory systems. -apw - 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/
2.6.21-rc3-mm2: PG_booked and PG_readahead
[applogies in advance if this has already been asked] I note that PG_booked and PG_readahead are both using bit 20 in 2.6.21-rc3-mm2. Is this intentional or perhaps a miss-merge. They do not sound obviously non-overlapping to my mind. -apw - 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/
Re: 2.6.21-rc4-mm1
Andrew Morton wrote: Temporarily at http://userweb.kernel.org/~akpm/2.6.21-rc4-mm1/ Will appear later at ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc4/2.6.21-rc4-mm1/ [All of the below is from the pre hot-fix runs. The very few results which are in for the hot-fix runs seem worse if anything. :( All results should be out on TKO.] - Restored the RSDL CPU scheduler (a new version thereof) Unsure if the above is the culprit but there seems to be a smattering of BUG's in kernbench from the schedular on several systems, and panics which do not fully dump out. elm3b239 is about 2/4 kernbench being the test in progress when we blammo in both failed tests, elm3b234 doesn't boot at all. From elm3b239: [ cut here ] kernel BUG at kernel/sched.c:3505! invalid opcode: [1] SMP last sysfs file: devices/pci:00/:00:00.0/irq CPU 19 Modules linked in: loop dm_mod md_mod sg Pid: 59, comm: migration/19 Not tainted 2.6.21-rc4-mm1-autokern1 #1 RIP: 0010:[804924f6] [804924f6] __sched_text_start+0x3a6/0x882 RSP: 0018:810100cefe20 EFLAGS: 00010002 RAX: 008c RBX: 81002b0f64d8 RCX: 000c RDX: RSI: 008c RDI: 81002b0f6da8 RBP: 810100cefeb0 R08: 008c R09: 81002b0f6d98 R10: 0034 R11: 8021ab20 R12: 81002b0f5a40 R13: 0002 R14: 00725eb99ef7 R15: 0013 FS: () GS:810100c42bc0() knlGS: CS: 0010 DS: 0018 ES: 0018 CR0: 8005003b CR2: 2ba9c431ab70 CR3: 0001060fc000 CR4: 06e0 Process migration/19 (pid: 59, threadinfo 810100cee000, task 810100ced8e0) Stack: 0001 0001 81010b681e98 810100ced8e0 810100cefe80 810100ceda78 0003 81010b681e88 81010b681e90 0286 0013 Call Trace: [80224a00] migration_thread+0x1b0/0x250 [80224850] migration_thread+0x0/0x250 [8023c85b] kthread+0xdb/0x120 [8020a7a8] child_rip+0xa/0x12 [8023c780] kthread+0x0/0x120 [8020a79e] child_rip+0x0/0x12 Code: 0f 0b eb fe 49 8b 94 24 b8 01 00 00 49 8b 84 24 b0 01 00 00 RIP [804924f6] __sched_text_start+0x3a6/0x882 RSP 810100cefe20 [ cut here ] kernel BUG at kernel/sched.c:3505! invalid opcode: [1] SMP last sysfs file: devices/pci:00/:00:00.0/irq CPU 21 Modules linked in: loop dm_mod md_mod sg Pid: 15583, comm: cc1 Not tainted 2.6.21-rc4-mm1-autokern1 #1 RIP: 0010:[80496686] [80496686] __sched_text_start+0x3a6/0x882 RSP: :81010aca7ee0 EFLAGS: 00010002 RAX: 008c RBX: 81002b111358 RCX: 000c RDX: RSI: 008c RDI: 81002b111c28 RBP: 81010aca7f70 R08: 008c R09: 81002b111c18 R10: 0034 R11: 0001 R12: 81002b1108c0 R13: 0002 R14: 006cb9bdff1a R15: 0015 FS: 2b6ef0bc66d0() GS:810100d02e40() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 2b6ef20c1000 CR3: 000106e2b000 CR4: 06e0 Process cc1 (pid: 15583, threadinfo 81010aca6000, task 810105b03620) Stack: 0100 0100 81010964c148 810105b03620 8054c9b7 810105b037c0 000b000e 81010b216d80 2b6ef20b7d00 2b6ef20c1000 0400 Call Trace: [80209f17] retint_careful+0xd/0x21 Code: 0f 0b eb fe 49 8b 94 24 b8 01 00 00 49 8b 84 24 b0 01 00 00 RIP [80496686] __sched_text_start+0x3a6/0x882 RSP 81010aca7ee0 From elm3b245: [ cut here ] kernel BUG at kernel/sched.c:3505! invalid opcode: [1] SMP last sysfs file: CPU 0 Modules linked in: Pid: 1, comm: init Not tainted 2.6.21-rc4-mm1-autokern1 #1 RIP: 0010:[8049cbb7] [8049cbb7] __sched_text_start+0x377/0x819 RSP: 0018:81010037dee0 EFLAGS: 00010002 RAX: 008c RBX: 0002 RCX: RDX: 0034 RSI: 008c RDI: 810002c15210 RBP: 81010037df70 R08: 008c R09: 000c R10: 810002c15200 R11: 8020968e R12: 810002c14940 R13: 7fff1da16360 R14: 810002c14780 R15: FS: 2b428d88d6d0() GS:805af000() knlGS: CS: 0010 DS: ES: CR0: 8005003b CR2: 00586a48 CR3: 08c2f000 CR4: 06e0 Process init (pid: 1, threadinfo 81010037c000, task 810003369450) Stack: 03e10059d1b0 81010037df28 80276dd3 810003369450 810008313880 003727988fe6 8100033695f0 7fff1da16250 7fff1da16360 0059bb70 8020968e 81010037df48 Call Trace: [80276dd3]
[PATCH 0/3] pfn_valid_within() HOLES_WITHIN_ZONES helper
The thought of having a helper for the holes within zones code has come up on two different threads in the last couple of days. So I took the pfn_valid_within() patch I had developed for the linear reclaim series and pulled it forward to 2.6.21-rc4-mm1. I have split it into a three patch series to better align with the affected patch sets within -mm. add-pfn_valid_within-helper-for-sub-MAX_ORDER-hole-detection -- adds the base helper and utilises it within the buddy allocator, anti-fragmentation-switch-over-to-pfn_valid_within() -- changes references within Mel Gormans anti-fragmentation patch series, and lumpy-move-to-using-pfn_valid_within() -- changes references with my lumpy reclaim patch series. -apw - 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/
[PATCH 1/3] add pfn_valid_within helper for sub-MAX_ORDER hole detection
Generally we work under the assumption that memory the mem_map array is contigious and valid out to MAX_ORDER_NR_PAGES block of pages, ie. that if we have validated any page within this MAX_ORDER_NR_PAGES block we need not check any other. This is not true when CONFIG_HOLES_IN_ZONE is set and we must check each and every reference we make from a pfn. Add a pfn_valid_within() helper which should be used when scanning pages within a MAX_ORDER_NR_PAGES block when we have already checked the validility of the block normally with pfn_valid(). This can then be optimised away when we do not have holes within a MAX_ORDER_NR_PAGES block of pages. Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] Acked-by: Mel Gorman [EMAIL PROTECTED] --- diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h index 7206c77..8c87d79 100644 --- a/include/linux/mmzone.h +++ b/include/linux/mmzone.h @@ -837,6 +837,18 @@ void sparse_init(void); void memory_present(int nid, unsigned long start, unsigned long end); unsigned long __init node_memmap_size_bytes(int, unsigned long, unsigned long); +/* + * If it is possible to have holes within a MAX_ORDER_NR_PAGES, then we + * need to check pfn validility within that MAX_ORDER_NR_PAGES block. + * pfn_valid_within() should be used in this case; we optimise this away + * when we have no holes within a MAX_ORDER_NR_PAGES block. + */ +#ifdef CONFIG_HOLES_IN_ZONE +#define pfn_valid_within(pfn) pfn_valid(pfn) +#else +#define pfn_valid_within(pfn) (1) +#endif + #endif /* !__ASSEMBLY__ */ #endif /* __KERNEL__ */ #endif /* _LINUX_MMZONE_H */ diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 3d7a9e2..695b5a6 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -206,10 +206,8 @@ static int page_outside_zone_boundaries(struct zone *zone, struct page *page) static int page_is_consistent(struct zone *zone, struct page *page) { -#ifdef CONFIG_HOLES_IN_ZONE - if (!pfn_valid(page_to_pfn(page))) + if (!pfn_valid_within(page_to_pfn(page))) return 0; -#endif if (zone != page_zone(page)) return 0; @@ -411,10 +409,8 @@ __find_combined_index(unsigned long page_idx, unsigned int order) static inline int page_is_buddy(struct page *page, struct page *buddy, int order) { -#ifdef CONFIG_HOLES_IN_ZONE - if (!pfn_valid(page_to_pfn(buddy))) + if (!pfn_valid_within(page_to_pfn(buddy))) return 0; -#endif if (page_zone_id(page) != page_zone_id(buddy)) return 0; - 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/
[PATCH 2/3] anti-fragmentation: switch over to pfn_valid_within()
Move to using pfn_valid_within(). Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] Acked-by: Mel Gorman [EMAIL PROTECTED] --- diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 695b5a6..3d7c29e 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -747,12 +747,10 @@ int move_freepages(struct zone *zone, #endif for (page = start_page; page end_page;) { -#ifdef CONFIG_HOLES_IN_ZONE - if (!pfn_valid(page_to_pfn(page))) { + if (!pfn_valid_within(page_to_pfn(page))) { page++; continue; } -#endif if (!PageBuddy(page)) { page++; - 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/
[PATCH 3/3] lumpy: move to using pfn_valid_within()
Switch to using pfn_valid_within() in lumpy reclaim. Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] Acked-by: Mel Gorman [EMAIL PROTECTED] --- diff --git a/mm/vmscan.c b/mm/vmscan.c index c3dc544..cf55c57 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -722,11 +722,10 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, /* The target page is in the block, ignore it. */ if (unlikely(pfn == page_pfn)) continue; -#ifdef CONFIG_HOLES_IN_ZONE + /* Avoid holes within the zone. */ - if (unlikely(!pfn_valid(pfn))) + if (unlikely(!pfn_valid_within(pfn))) break; -#endif cursor_page = pfn_to_page(pfn); /* Check that we have not crossed a zone boundary. */ - 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/
Re: 2.6.21-rc4-mm1
Andy Whitcroft wrote: Andrew Morton wrote: Temporarily at http://userweb.kernel.org/~akpm/2.6.21-rc4-mm1/ Will appear later at ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc4/2.6.21-rc4-mm1/ [All of the below is from the pre hot-fix runs. The very few results which are in for the hot-fix runs seem worse if anything. :( All results should be out on TKO.] - Restored the RSDL CPU scheduler (a new version thereof) Unsure if the above is the culprit but there seems to be a smattering of BUG's in kernbench from the schedular on several systems, and panics which do not fully dump out. elm3b239 is about 2/4 kernbench being the test in progress when we blammo in both failed tests, elm3b234 doesn't boot at all. Well I have one result through for backing RSDL out on elm3b239 and that does indeed seem to give us a successful boot and test. peterz has pointed me to an incremental patch from Con which I'll push through testing and see if that sorts it out. -apw - 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/
Re: 2.6.21-rc4-mm1
Andy Whitcroft wrote: Andy Whitcroft wrote: Andrew Morton wrote: Temporarily at http://userweb.kernel.org/~akpm/2.6.21-rc4-mm1/ Will appear later at ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc4/2.6.21-rc4-mm1/ [All of the below is from the pre hot-fix runs. The very few results which are in for the hot-fix runs seem worse if anything. :( All results should be out on TKO.] - Restored the RSDL CPU scheduler (a new version thereof) Unsure if the above is the culprit but there seems to be a smattering of BUG's in kernbench from the schedular on several systems, and panics which do not fully dump out. elm3b239 is about 2/4 kernbench being the test in progress when we blammo in both failed tests, elm3b234 doesn't boot at all. Well I have one result through for backing RSDL out on elm3b239 and that does indeed seem to give us a successful boot and test. peterz has pointed me to an incremental patch from Con which I'll push through testing and see if that sorts it out. Ok, tested the patch below on top of 2.6.21-rc4-mm1 and this seems to fix the problem: http://ck.kolivas.org/patches/staircase-deadline/2.6.21-rc4-mm1-rsdl-0.32.patch Hard to tell from that patch whether it will be fixed in the changes already committed to the next -mm. Its possible that it may be fixed by the following patch: sched-rsdl-improvements.patch Which has the following slipped in at the end of the changelog: A tiny change checking for MAX_PRIO in normal_prio() may prevent oopses on bootup on large SMP due to forking off the idle task. Con, are all the changes in the 0.32 patch above with akpm? -apw - 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/
Re: 2.6.21-rc4-mm1
Con Kolivas wrote: On Thursday 22 March 2007 20:48, Andy Whitcroft wrote: Andy Whitcroft wrote: Andy Whitcroft wrote: Andrew Morton wrote: Temporarily at http://userweb.kernel.org/~akpm/2.6.21-rc4-mm1/ Will appear later at ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc 4/2.6.21-rc4-mm1/ [All of the below is from the pre hot-fix runs. The very few results which are in for the hot-fix runs seem worse if anything. :( All results should be out on TKO.] - Restored the RSDL CPU scheduler (a new version thereof) Unsure if the above is the culprit but there seems to be a smattering of BUG's in kernbench from the schedular on several systems, and panics which do not fully dump out. elm3b239 is about 2/4 kernbench being the test in progress when we blammo in both failed tests, elm3b234 doesn't boot at all. Well I have one result through for backing RSDL out on elm3b239 and that does indeed seem to give us a successful boot and test. peterz has pointed me to an incremental patch from Con which I'll push through testing and see if that sorts it out. Ok, tested the patch below on top of 2.6.21-rc4-mm1 and this seems to fix the problem: http://ck.kolivas.org/patches/staircase-deadline/2.6.21-rc4-mm1-rsdl-0.32.p atch Hard to tell from that patch whether it will be fixed in the changes already committed to the next -mm. Its possible that it may be fixed by the following patch: sched-rsdl-improvements.patch Which has the following slipped in at the end of the changelog: A tiny change checking for MAX_PRIO in normal_prio() may prevent oopses on bootup on large SMP due to forking off the idle task. Con, are all the changes in the 0.32 patch above with akpm? Yes he's queued everything in that patch you tested for the next -mm. Thanks very much for testing it. No worries. I've just got through the results on the other machine in the mix. That machine seems to be fixed by backing out RSDL and not by the fixup 0.32 patch ... This second machine seems to had hard very soon after user space starts executing but without a panic. I can't say that the symptoms are very definitive, but I do have a good result from that machine without RSDL and not with rsdl-0.32. The machine is a dual-core x86_64 machine: Dual Core AMD Opteron(tm) Processor 275. I'll let you know if I find out anything else. Shout if you want any information or have anything you want poked or tested. -apw - 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/
Re: 2.6.21-rc4-mm1
Andy Whitcroft wrote: Con Kolivas wrote: On Thursday 22 March 2007 20:48, Andy Whitcroft wrote: Andy Whitcroft wrote: Andy Whitcroft wrote: Andrew Morton wrote: Temporarily at http://userweb.kernel.org/~akpm/2.6.21-rc4-mm1/ Will appear later at ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc 4/2.6.21-rc4-mm1/ [All of the below is from the pre hot-fix runs. The very few results which are in for the hot-fix runs seem worse if anything. :( All results should be out on TKO.] - Restored the RSDL CPU scheduler (a new version thereof) Unsure if the above is the culprit but there seems to be a smattering of BUG's in kernbench from the schedular on several systems, and panics which do not fully dump out. elm3b239 is about 2/4 kernbench being the test in progress when we blammo in both failed tests, elm3b234 doesn't boot at all. Well I have one result through for backing RSDL out on elm3b239 and that does indeed seem to give us a successful boot and test. peterz has pointed me to an incremental patch from Con which I'll push through testing and see if that sorts it out. Ok, tested the patch below on top of 2.6.21-rc4-mm1 and this seems to fix the problem: http://ck.kolivas.org/patches/staircase-deadline/2.6.21-rc4-mm1-rsdl-0.32.p atch Hard to tell from that patch whether it will be fixed in the changes already committed to the next -mm. Its possible that it may be fixed by the following patch: sched-rsdl-improvements.patch Which has the following slipped in at the end of the changelog: A tiny change checking for MAX_PRIO in normal_prio() may prevent oopses on bootup on large SMP due to forking off the idle task. Con, are all the changes in the 0.32 patch above with akpm? Yes he's queued everything in that patch you tested for the next -mm. Thanks very much for testing it. No worries. I've just got through the results on the other machine in the mix. That machine seems to be fixed by backing out RSDL and not by the fixup 0.32 patch ... This second machine seems to had hard very soon after user space starts executing but without a panic. I can't say that the symptoms are very definitive, but I do have a good result from that machine without RSDL and not with rsdl-0.32. The machine is a dual-core x86_64 machine: Dual Core AMD Opteron(tm) Processor 275. I'll let you know if I find out anything else. Shout if you want any information or have anything you want poked or tested. Ok, I have yet a third x86_64 machine is is blowing up with the latest 2.6.21-rc4-mm1+hotfixes+rsdl-0.32 but working with 2.6.21-rc4-mm1+hotfixes-RSDL. I have results on various hotfix levels so I have just fired off a set of tests across the affected machines on that latest hotfix stack plus the RSDL backout and the results should be in in the next hour or two. I think there is a strong correlation between RSDL and these hangs. Any suggestions as to the next step. -apw - 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/
Re: 2.6.21-rc4-mm1
Con Kolivas wrote: On Friday 23 March 2007 05:17, Andy Whitcroft wrote: Ok, I have yet a third x86_64 machine is is blowing up with the latest 2.6.21-rc4-mm1+hotfixes+rsdl-0.32 but working with 2.6.21-rc4-mm1+hotfixes-RSDL. I have results on various hotfix levels so I have just fired off a set of tests across the affected machines on that latest hotfix stack plus the RSDL backout and the results should be in in the next hour or two. I think there is a strong correlation between RSDL and these hangs. Any suggestions as to the next step. Found a nasty in requeue_task + if (list_empty(old_array-queue + old_prio)) + __clear_bit(old_prio, p-array-prio_bitmap); see anything wrong there? I do :P I'll queue that up with the other changes pending and hopefully that will fix your bug. Tests queued with your rdsl-0.33 patch (I am assuming its in there). Will let you know how it looks. -apw - 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/
Re: 2.6.21-rc4-mm1
Andy Whitcroft wrote: Con Kolivas wrote: On Friday 23 March 2007 05:17, Andy Whitcroft wrote: Ok, I have yet a third x86_64 machine is is blowing up with the latest 2.6.21-rc4-mm1+hotfixes+rsdl-0.32 but working with 2.6.21-rc4-mm1+hotfixes-RSDL. I have results on various hotfix levels so I have just fired off a set of tests across the affected machines on that latest hotfix stack plus the RSDL backout and the results should be in in the next hour or two. I think there is a strong correlation between RSDL and these hangs. Any suggestions as to the next step. Found a nasty in requeue_task +if (list_empty(old_array-queue + old_prio)) +__clear_bit(old_prio, p-array-prio_bitmap); see anything wrong there? I do :P I'll queue that up with the other changes pending and hopefully that will fix your bug. Tests queued with your rdsl-0.33 patch (I am assuming its in there). Will let you know how it looks. Hmmm, this is good for the original machine (as was 0.32) but not for either of the other two. I am seeing panics as below on those two. -apw elm3b245: NULL pointer dereference at 0020 RIP: [80497d94] __sched_text_start+0x424/0x8a5 PGD 0 Oops: [1] SMP last sysfs file: block/ram0/uevent CPU 0 Modules linked in: Pid: 1038, comm: udevd Not tainted 2.6.21-rc4-mm1-autokern1 #1 RIP: 0010:[80497d94] [80497d94] __sched_text_start+0x424/0x8a5 RSP: 0018:81000316de68 EFLAGS: 00010017 RAX: 06c6 RBX: 0001 RCX: RDX: RSI: 008c RDI: ffd0 RBP: 81000316def8 R08: 0064 R09: 0024 R10: 810001014ad8 R11: 0286 R12: 810001014218 R13: 810001013780 R14: 810001769450 R15: FS: 2b75d89c66d0() GS:805aa000() knlGS: CS: 0010 DS: ES: CR0: 8005003b CR2: 0020 CR3: 00201000 CR4: 06e0 Process udevd (pid: 1038, threadinfo 81000316c000, task 8100031cebb0) Stack: 0001 8100031cebb0 ffd0 0036e28ef568 8100031ced48 0292 81000316def8 0246 81000316def8 8022af3d Call Trace: [8022af3d] put_files_struct+0xbd/0xc9 [8022c773] do_exit+0x7d2/0x7d6 [8022c801] sys_exit_group+0x0/0x14 [8022c813] sys_exit_group+0x12/0x14 [8020968e] system_call+0x7e/0x83 Code: 48 39 47 50 74 51 48 c7 47 40 00 00 00 00 8b 52 f4 48 b9 40 RIP [80497d94] __sched_text_start+0x424/0x8a5 RSP 81000316de68 CR2: 0020 Fixing recursive fault but reboot is needed! elm3b6: Unable to handle kernel paging request at fb6c RIP: [8020c573] convert_rip_to_linear+0x53/0x91 PGD 180780067 PUD 182242067 PMD 0 Oops: [1] SMP last sysfs file: devices/pci:00/:00:0a.0/:02:04.0/host0/target0:0:6/0:0:6:0/type CPU 0 Modules linked in: Pid: 2442, comm: autorun Not tainted 2.6.21-rc4-mm1-autokern1 #1 RIP: 0010:[8020c573] [8020c573] convert_rip_to_linear+0x53/0x91 RSP: :810181a53cf8 EFLAGS: 00010002 RAX: fb68 RBX: 810181a53e28 RCX: 8101823d6930 RDX: 8049fb6d RSI: 810182342180 RDI: 810182342440 RBP: 810181a53cf8 R08: 80209bb9 R09: 008c R10: R11: 01200011 R12: R13: 810182342180 R14: 810181a53e28 R15: FS: () GS:805b2000(0063) knlGS:f7f1cb80 CS: 0010 DS: 002b ES: 002b CR0: 8005003b CR2: fb6c CR3: 000181a5b000 CR4: 06e0 Process autorun (pid: 2442, threadinfo 810181a52000, task 8101823d6930) Stack: 810181a53d18 80219075 8101823d84a8 0020 810181a53e18 80219ab4 8101fff654d8 810181a53d48 80264291 8101823d6930 810181a53e28 0046 Call Trace: [80219075] is_prefetch+0x29/0x217 [80219ab4] do_page_fault+0x608/0x7f0 [80264291] page_dup_rmap+0x1d/0x24 [8024567c] search_module_extables+0x83/0x8f [80229b43] oops_enter+0xe/0x10 [8020ae62] oops_begin+0x3c/0x70 [80219b31] do_page_fault+0x685/0x7f0 [8022404d] task_running_tick+0xad/0x290 [8049fb6d] error_exit+0x0/0x84 [8049fb6d] error_exit+0x0/0x84 [8049dc11] thread_return+0x22/0xd3 [80209802] int_careful+0xd/0x11 Code: 8b 48 04 0f b7 50 02 0f b6 c1 c1 e0 10 09 c2 89 c8 25 00 00 RIP [8020c573] convert_rip_to_linear+0x53/0x91 RSP 810181a53cf8 CR2: fb6c - 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/
Re: [PATCH] slab: NUMA kmem_cache diet
Eric Dumazet wrote: Some NUMA machines have a big MAX_NUMNODES (possibly 1024), but fewer possible nodes. This patch dynamically sizes the 'struct kmem_cache' to allocate only needed space. I moved nodelists[] field at the end of struct kmem_cache, and use the following computation in kmem_cache_init() cache_cache.buffer_size = offsetof(struct kmem_cache, nodelists) + nr_node_ids * sizeof(struct kmem_list3 *); On my two nodes x86_64 machine, kmem_cache.obj_size is now 192 instead of 704 (This is because on x86_64, MAX_NUMNODES is 64) On bigger NUMA setups, this might reduce the gfporder of cache_cache That is a dramatic size difference, and I seem to have 128 slabs wow. I'll try and find some time to test this on some of our numa kit. Signed-off-by: Eric Dumazet [EMAIL PROTECTED] diff --git a/mm/slab.c b/mm/slab.c index abf46ae..b187618 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -389,7 +389,6 @@ struct kmem_cache { unsigned int buffer_size; u32 reciprocal_buffer_size; /* 3) touched by every alloc free from the backend */ - struct kmem_list3 *nodelists[MAX_NUMNODES]; unsigned int flags; /* constant flags */ unsigned int num; /* # of objs per slab */ @@ -444,6 +443,17 @@ #if DEBUG int obj_offset; int obj_size; #endif + /* + * We put nodelists[] at the end of kmem_cache, because we want to size + * this array to nr_node_ids slots instead of MAX_NUMNODES + * (see kmem_cache_init()) + * We still use [MAX_NUMNODES] and not [1] or [0] because cache_cache + * is statically defined, so we reserve the max number of nodes. + */ + struct kmem_list3 *nodelists[MAX_NUMNODES]; + /* + * Do not add fields after nodelists[] + */ }; #define CFLGS_OFF_SLAB (0x8000UL) @@ -678,9 +688,6 @@ static struct kmem_cache cache_cache = { .shared = 1, .buffer_size = sizeof(struct kmem_cache), .name = kmem_cache, -#if DEBUG - .obj_size = sizeof(struct kmem_cache), -#endif Is there any reason to not initialise the .obj_size here? You are initialising both .buffer_size and .obj_size in kmem_cache_init anyhow so I would expect either both or neither to be initialised in your new scheme. Doing only one seems very strange. }; #define BAD_ALIEN_MAGIC 0x01020304ul @@ -1437,6 +1444,15 @@ void __init kmem_cache_init(void) cache_cache.array[smp_processor_id()] = initarray_cache.cache; cache_cache.nodelists[node] = initkmem_list3[CACHE_CACHE]; + /* + * struct kmem_cache size depends on nr_node_ids, which + * can be less than MAX_NUMNODES. + */ + cache_cache.buffer_size = offsetof(struct kmem_cache, nodelists) + + nr_node_ids * sizeof(struct kmem_list3 *); +#if DEBUG + cache_cache.obj_size = cache_cache.buffer_size; +#endif cache_cache.buffer_size = ALIGN(cache_cache.buffer_size, cache_line_size()); cache_cache.reciprocal_buffer_size = -apw - 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/
Re: debug rsdl 0.33
Con Kolivas wrote: On Saturday 24 March 2007 08:45, Con Kolivas wrote: On Friday 23 March 2007 23:28, Andy Whitcroft wrote: Andy Whitcroft wrote: Con Kolivas wrote: On Friday 23 March 2007 05:17, Andy Whitcroft wrote: Ok, I have yet a third x86_64 machine is is blowing up with the latest 2.6.21-rc4-mm1+hotfixes+rsdl-0.32 but working with 2.6.21-rc4-mm1+hotfixes-RSDL. I have results on various hotfix levels so I have just fired off a set of tests across the affected machines on that latest hotfix stack plus the RSDL backout and the results should be in in the next hour or two. I think there is a strong correlation between RSDL and these hangs. Any suggestions as to the next step. Found a nasty in requeue_task + if (list_empty(old_array-queue + old_prio)) + __clear_bit(old_prio, p-array-prio_bitmap); see anything wrong there? I do :P I'll queue that up with the other changes pending and hopefully that will fix your bug. Tests queued with your rdsl-0.33 patch (I am assuming its in there). Will let you know how it looks. Hmmm, this is good for the original machine (as was 0.32) but not for either of the other two. I am seeing panics as below on those two. This machine seems most sensitive to it (first column): elm3b6 amd64 newisys 4cpu config: amd64 Can you throw this debugging patch at it please? The console output might be very helpful. On top of sched-rsdl-0.33 thanks! Better yet this one which checks the expired array as well and after pull_task. If anyone's getting a bug they think might be due to rsdl please try this (on rsdl 0.33). Ok, new round of tests across the sensitive machines with 0.33 plus the above debug patch are in the queue. Will let you know how they pan out. The tests with -rc4 + 0.33 are also in. Failing there also. Both out of __sched_text_start, so I'd guess the same cause and the schedular is fingered. -apw - 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/
Re: debug rsdl 0.33
Con Kolivas wrote: This is about the only place I can see the run_list is looked at unlocked. Can you see if this simple patch helps? The debug patch is unnecessary now. Tests queued with this patch. Will let you know. -apw - 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/
Re: debug rsdl 0.33
Andy Whitcroft wrote: Con Kolivas wrote: This is about the only place I can see the run_list is looked at unlocked. Can you see if this simple patch helps? The debug patch is unnecessary now. Tests queued with this patch. Will let you know. That patch had no effect on the problem. ... Since then we have performed some more debugging on the issue and it appears that the first stanza in next_dynamic_task is tripping, triggering a major_priority_rotation and the resulting runq bitmap indicating there is nothing to run. Discussions with Con seem to indicate that this is not possible :/. Subsequent to that Con suggested testing a refactored RSDL patch. That patch seemed to work on the machine at hand, so tests have been submitted for all the affected machines. http://ck.kolivas.org/patches/staircase-deadline/2.6.21-rc4-mm1-rsdl-0.34-test.patch ... Ok, the preliminary results are in and we seem to have good boots in the three machines I was hitting early boot oops. So I think we can say that the new stack is a lot better than the old. Con, have a Tested-by: :/ -apw - 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/
Re: [PATCH 1/4] Generic Virtual Memmap suport for SPARSEMEM V3
Christoph Lameter wrote: On Thu, 5 Apr 2007, David Miller wrote: Hey Christoph, here is sparc64 support for this stuff. Great! After implementing this and seeing more and more how it works, I really like it :-) Thanks a lot for doing this work Christoph! Thanks for the appreciation. CCing Andy Whitcroft who will hopefully merge this all of this together into sparsemem including the S/390 implementation. Yep grabbed this one and added it to the stack. Now to find a sparc to test it with! -apw - 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/
Re: 2.6.20-mm2
Andrew Morton wrote: - Judging by the number of times I get asked is there a git tree for -mm, nobody is reading the boilerplate. Here it is again: The git tree version of -mm seems to be sick. A fetch of the tag gives you something but it is significantly (200k lines of diff) away from what patching a virgin tree gives you. The 'last' patch seems to be: scsi-megaraid_sas-throttle-io-if-fw-is-busy -apw - 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/
[PATCH 1/5] Lumpy Reclaim V3
When we are out of memory of a suitable size we enter reclaim. The current reclaim algorithm targets pages in LRU order, which is great for fairness but highly unsuitable if you desire pages at higher orders. To get pages of higher order we must shoot down a very high proportion of memory; 95% in a lot of cases. This patch set adds a lumpy reclaim algorithm to the allocator. It targets groups of pages at the specified order anchored at the end of the active and inactive lists. This encourages groups of pages at the requested orders to move from active to inactive, and active to free lists. This behaviour is only triggered out of direct reclaim when higher order pages have been requested. This patch set is particularly effective when utilised with an anti-fragmentation scheme which groups pages of similar reclaimability together. This patch set is based on Peter Zijlstra's lumpy reclaim V2 patch which forms the foundation. [EMAIL PROTECTED]: ia64 pfn_to_nid fixes and loop cleanup] [EMAIL PROTECTED]: static declarations for internal functions] Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] Acked-by: Peter Zijlstra [EMAIL PROTECTED] Signed-off-by: Andrew Morton [EMAIL PROTECTED] --- diff --git a/fs/buffer.c b/fs/buffer.c index 1b69d61..19d1d2c 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -363,7 +363,7 @@ static void free_more_memory(void) for_each_online_pgdat(pgdat) { zones = pgdat-node_zonelists[gfp_zone(GFP_NOFS)].zones; if (*zones) - try_to_free_pages(zones, GFP_NOFS); + try_to_free_pages(zones, 0, GFP_NOFS); } } diff --git a/include/linux/swap.h b/include/linux/swap.h index eae023a..382a30b 100644 --- a/include/linux/swap.h +++ b/include/linux/swap.h @@ -190,7 +190,8 @@ extern int rotate_reclaimable_page(struct page *page); extern void swap_setup(void); /* linux/mm/vmscan.c */ -extern unsigned long try_to_free_pages(struct zone **, gfp_t); +extern unsigned long try_to_free_pages(struct zone **zones, int order, + gfp_t gfp_mask); extern unsigned long shrink_all_memory(unsigned long nr_pages); extern int vm_swappiness; extern int remove_mapping(struct address_space *mapping, struct page *page); diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 96451c5..887e435 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1415,7 +1415,7 @@ nofail_alloc: reclaim_state.reclaimed_slab = 0; p-reclaim_state = reclaim_state; - did_some_progress = try_to_free_pages(zonelist-zones, gfp_mask); + did_some_progress = try_to_free_pages(zonelist-zones, order, gfp_mask); p-reclaim_state = NULL; p-flags = ~PF_MEMALLOC; diff --git a/mm/vmscan.c b/mm/vmscan.c index 2c8f197..f15ffcb 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -68,6 +68,8 @@ struct scan_control { int swappiness; int all_unreclaimable; + + int order; }; /* @@ -617,6 +619,36 @@ keep: } /* + * Attempt to remove the specified page from its LRU. Only take this page + * if it is of the appropriate PageActive status. Pages which are being + * freed elsewhere are also ignored. + * + * page: page to consider + * active: active/inactive flag only take pages of this type + * + * returns 0 on success, -ve errno on failure. + */ +static int __isolate_lru_page(struct page *page, int active) +{ + int ret = -EINVAL; + + if (PageLRU(page) (PageActive(page) == active)) { + ret = -EBUSY; + if (likely(get_page_unless_zero(page))) { + /* +* Be careful not to clear PageLRU until after we're +* sure the page is not being freed elsewhere -- the +* page release code relies on it. +*/ + ClearPageLRU(page); + ret = 0; + } + } + + return ret; +} + +/* * zone-lru_lock is heavily contended. Some of the functions that * shrink the lists perform better by taking out a batch of pages * and working on them outside the LRU lock. @@ -630,38 +662,83 @@ keep: * @src: The LRU list to pull pages off. * @dst: The temp list to put pages on to. * @scanned: The number of pages that were scanned. + * @order: The caller's attempted allocation order * * returns how many pages were moved onto [EMAIL PROTECTED] */ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, struct list_head *src, struct list_head *dst, - unsigned long *scanned) + unsigned long *scanned, int order) { unsigned long nr_taken = 0; - struct page *page; unsigned long scan; for (scan = 0; scan nr_to_scan !list_empty(src); scan++) { - struct list_head *target; + struct page *page; + unsigned long pfn
[PATCH 2/5] lumpy: isolate_lru_pages wants to specifically take active or inactive pages
The caller of isolate_lru_pages specifically knows whether it wants to take either inactive or active pages. Currently we take the state of the LRU page at hand and use that to scan for matching pages in the order sized block. If that page is transiting we can scan for the wrong type. The caller knows what they want and should be telling us. Pass in the required active/inactive state and match against that. Note, that now we pass the expected active state when scanning the active/inactive lists we may find missmatching target pages, pages which are in the process of changing state. This is no longer an error and we should simply ignore them. Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] --- diff --git a/mm/vmscan.c b/mm/vmscan.c index f15ffcb..b878d54 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -663,12 +663,13 @@ static int __isolate_lru_page(struct page *page, int active) * @dst: The temp list to put pages on to. * @scanned: The number of pages that were scanned. * @order: The caller's attempted allocation order + * @active:The caller's trying to obtain active or inactive pages * * returns how many pages were moved onto [EMAIL PROTECTED] */ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, struct list_head *src, struct list_head *dst, - unsigned long *scanned, int order) + unsigned long *scanned, int order, int active) { unsigned long nr_taken = 0; unsigned long scan; @@ -678,7 +679,6 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, unsigned long pfn; unsigned long end_pfn; unsigned long page_pfn; - int active; int zone_id; page = lru_to_page(src); @@ -686,20 +686,16 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, VM_BUG_ON(!PageLRU(page)); - active = PageActive(page); switch (__isolate_lru_page(page, active)) { case 0: list_move(page-lru, dst); nr_taken++; break; - case -EBUSY: - /* else it is being freed elsewhere */ + default: + /* page is being freed, or is a missmatch */ list_move(page-lru, src); continue; - - default: - BUG(); } if (!order) @@ -768,8 +764,8 @@ static unsigned long shrink_inactive_list(unsigned long max_scan, unsigned long nr_freed; nr_taken = isolate_lru_pages(sc-swap_cluster_max, -zone-inactive_list, -page_list, nr_scan, sc-order); +zone-inactive_list, +page_list, nr_scan, sc-order, 0); __mod_zone_page_state(zone, NR_INACTIVE, -nr_taken); zone-pages_scanned += nr_scan; zone-total_scanned += nr_scan; @@ -916,7 +912,7 @@ force_reclaim_mapped: lru_add_drain(); spin_lock_irq(zone-lru_lock); pgmoved = isolate_lru_pages(nr_pages, zone-active_list, - l_hold, pgscanned, sc-order); + l_hold, pgscanned, sc-order, 1); zone-pages_scanned += pgscanned; __mod_zone_page_state(zone, NR_ACTIVE, -pgmoved); spin_unlock_irq(zone-lru_lock); - 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/
[PATCH 0/5] Lumpy Reclaim V4
Following this email are five patches which represent the current state of the lumpy reclaim patches; collectivly lumpy v4. This patch kit is designed as a complete drop-in replacement for the lumpy patches in 2.6.20-mm2. This stack is split out to show the incremental changes in this version. Andrew please replace your current lumpy stack with this one, you may prefer to fold this kit into a single patch lumpy-v4. Comparitive testing between lumpy-v3 and lump-v4 generally shows a small improvement, coming from the improved matching of pages taken from the end of the active/inactive list (patch 2 in this series). I have taken the lumpy-v2 patches and fixes as found in 2.6.20-rc6-mm2 and folded them back into a single patch (collectivly lumpy v3), updating attribution. On top of this are are four patches which represent the updates mainly coming from the detailed review comments from Andrew Morton: lumpy-reclaim-v3: folded back base, lumpy-v3, lumpy-isolate_lru_pages-wants-to-specifically-take-active-or-inactive-pages: ensure we take pages of the expected type from the end of the active/ inactive lists. This is both a performance and correctness fix, lumpy-ensure-that-we-compare-PageActive-and-active-safely: ensure comparisons between PageActive() and coded booleans are safe should PageActive() not return 1/0, lumpy-update-commentry-on-subtle-comparisons-and-rounding-assumptions: update the code commentary to explain the subtle exit conditions, and lumpy-only-check-for-valid-pages-when-holes-are-present: remove expensive check for invalid pages within MAX_ORDER blocks where those cannot exist. Against: 2.6.20-mm2 -apw - 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/
[PATCH 3/5] lumpy: ensure that we compare PageActive and active safely
Now that we are passing in a boolean active flag we need to ensure that the result of PageActive(page) is comparible to that boolean. Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] --- diff --git a/mm/vmscan.c b/mm/vmscan.c index b878d54..2bfad79 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -632,7 +632,12 @@ static int __isolate_lru_page(struct page *page, int active) { int ret = -EINVAL; - if (PageLRU(page) (PageActive(page) == active)) { + /* +* When checking the active state, we need to be sure we are +* dealing with comparible boolean values. Take the logical not +* of each. +*/ + if (PageLRU(page) (!PageActive(page) == !active)) { ret = -EBUSY; if (likely(get_page_unless_zero(page))) { /* - 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/
[PATCH 4/5] lumpy: update commentry on subtle comparisons and rounding assumptions
We have a number of subtle comparisons when scanning a block, and we make use of a lot of buddy mem_map guarentees. Add commentary about each. Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] --- diff --git a/mm/vmscan.c b/mm/vmscan.c index 2bfad79..bef7e92 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -709,7 +709,11 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, /* * Attempt to take all pages in the order aligned region * surrounding the tag page. Only take those pages of -* the same active state as that tag page. +* the same active state as that tag page. We may safely +* round the target page pfn down to the requested order +* as the mem_map is guarenteed valid out to MAX_ORDER, +* where that page is in a different zone we will detect +* it from its zone id and abort this block scan. */ zone_id = page_zone_id(page); page_pfn = page_to_pfn(page); @@ -718,12 +722,15 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, for (; pfn end_pfn; pfn++) { struct page *cursor_page; + /* The target page is in the block, ignore it. */ if (unlikely(pfn == page_pfn)) continue; + /* Avoid holes within the zone. */ if (unlikely(!pfn_valid(pfn))) break; cursor_page = pfn_to_page(pfn); + /* Check that we have not crossed a zone boundary. */ if (unlikely(page_zone_id(cursor_page) != zone_id)) continue; scan++; - 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/
[PATCH 5/5] lumpy: only check for valid pages when holes are present
We only need to check that each page is valid with pfn_valid when we are on an architecture which had holes within zones. Make this check conditional. Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] --- diff --git a/mm/vmscan.c b/mm/vmscan.c index bef7e92..f249ad7 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -725,9 +725,11 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, /* The target page is in the block, ignore it. */ if (unlikely(pfn == page_pfn)) continue; +#ifdef CONFIG_HOLES_IN_ZONE /* Avoid holes within the zone. */ if (unlikely(!pfn_valid(pfn))) break; +#endif cursor_page = pfn_to_page(pfn); /* Check that we have not crossed a zone boundary. */ - 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/
Re: [PATCH 1/5] Lumpy Reclaim V3
Christoph Lameter wrote: On Tue, 27 Feb 2007, Andy Whitcroft wrote: +static int __isolate_lru_page(struct page *page, int active) +{ +int ret = -EINVAL; + +if (PageLRU(page) (PageActive(page) == active)) { +ret = -EBUSY; +if (likely(get_page_unless_zero(page))) { +/* + * Be careful not to clear PageLRU until after we're + * sure the page is not being freed elsewhere -- the + * page release code relies on it. + */ +ClearPageLRU(page); +ret = 0; Is that really necessary? PageLRU is clear when a page is freed right? And clearing PageLRU requires the zone-lru_lock since we have to move it off the LRU. Although the PageLRU is stable as we have the zone-lru_lock we cannot take the page off the LRU unless we can take a reference to it preventing it from being released. The page release code relies on the the caller who takes the reference count to zero having exclusive access to the page to release it. To prevent a race with put_page/__page_cache_release we cannot touch the page once its count drops to zero, which occurs before the PageLRU is cleared. PageLRU being sampled outside the zone-lru_lock in that path to avoid taking the lock if not required. -ClearPageLRU(page); -target = dst; +active = PageActive(page); Why are we saving the active state? Page cannot be moved between LRUs while we hold the lru lock anyways. This is used later in the function for comparison against all of the other pages in the 'order' sized area rooted about the target page. Its mearly an optimisation. -apw - 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/
Re: [PATCH 2/5] lumpy: isolate_lru_pages wants to specifically take active or inactive pages
Christoph Lameter wrote: On Tue, 27 Feb 2007, Andy Whitcroft wrote: The caller of isolate_lru_pages specifically knows whether it wants to take either inactive or active pages. Currently we take the state of the LRU page at hand and use that to scan for matching pages in the order sized block. If that page is transiting we can scan for the wrong type. The caller knows what they want and should be telling us. Pass in the required active/inactive state and match against that. The page cannot be transiting since we hold the lru lock? As you say it should be gated by lru_lock and we should not expect to see pages with the wrong type on the list. I would swear that I was seeing pages on the wrong list there for a bit in testing and mistakenly thought they were in transition. A quick review at least says thats false. So I'll reinstate the BUG() and retest to see if I am smoking crack or there is a bigger bug out there. -apw - 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/
[PATCH] i386 alternative_io implementation
Add an i386 implementation of alternative_io modelled on the x86_64 version. Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] --- Ok, this seems to fix things up here. I have only boot tested this on an SMP so I'd not call that 'heavily tested' in any real sense but it might get you going until akpm has a chance to comment. -apw --- diff --git a/include/asm-i386/alternative.h b/include/asm-i386/alternative.h index 4a77e93..8b6d9ed 100644 --- a/include/asm-i386/alternative.h +++ b/include/asm-i386/alternative.h @@ -100,6 +100,23 @@ static inline void alternatives_smp_switch(int smp) {} 663:\n\t newinstr \n664:\n /* replacement */\ .previous :: i (feature), ##input) +#define alternative_io(oldinstr, newinstr, feature, output, input...) \ + asm volatile (661:\n\t oldinstr \n662:\n\ + .section .altinstructions,\a\\n \ + .align 4\n\ + .long 661b\n/* label */ \ + .long 663f\n/* new instruction */ \ + .long 0x00\n \ + .byte %c[feat]\n/* feature bit */ \ + .byte 0x00\n \ + .byte 662b-661b\n /* sourcelen */ \ + .byte 664f-663f\n /* replacementlen */ \ + .byte 0x00\n \ + .previous\n \ + .section .altinstr_replacement,\ax\\n \ + 663:\n\t newinstr \n664:\n /* replacement */\ + .previous : output : [feat] i (feature), ##input) + /* Like alternative_io, but supports 2 possible alternatives */ #define alternative_io_two(oldinstr, newinstr, feat, newinstr2, feat2,\ output, input...) \ diff --git a/include/asm-i386/tsc.h b/include/asm-i386/tsc.h index 3469766..a031b56 100644 --- a/include/asm-i386/tsc.h +++ b/include/asm-i386/tsc.h @@ -6,6 +6,7 @@ #ifndef _ASM_i386_TSC_H #define _ASM_i386_TSC_H +#include asm/alternative.h #include asm/processor.h /* - 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/
Re: [PATCH 1/4] x86_64: Switch to SPARSE_VIRTUAL
Christoph Lameter wrote: On Mon, 2 Apr 2007, Martin Bligh wrote: Its just the opposite. The vmemmap code is so efficient that we can remove lots of other code and gops of these alternate implementations. On x86_64 its even superior to FLATMEM since FLATMEM still needs a memory reference for the mem_map area. So if we make SPARSE standard for all configurations then there is no need anymore for FLATMEM DISCONTIG etc etc. Can we not cleanup all this mess? Get rid of all the gazillions of #ifdefs please? This would ease code maintenance significantly. I hate having to constantly navigate my way through all the alternatives. The original plan when this was first merged was pretty much that - for sparsemem to replace discontigmem once it was well tested. Seems to have got stalled halfway through ;-( But you made big boboo in SPARSEMEM. Virtual memmap is a textbook case that was not covered. Instead this horrible stuff that involves calling functions in VM primitives. We could have been there years ago... Not sure we'll get away with replacing flatmem for all arches, but we could at least get rid of discontigmem, it seems. I think we could start with x86_64 and ia64. Both will work fine with SPARSE VIRTUAL (and SGIs concerns about performance are addressed) and we could remove the other alternatives. That is going to throw out lots of stuff. Then proceed to other arches Could the SPARSEMEM folks take this over this patch? You have more resources and I am all alone on this. I will post another patchset today that also includes an IA64 implementation. That would be me. I have been offline writing for OLS and did not get to respond before this. When we first saw these patches the reaction was a general positive despite skepticism on the general utility of vmemmmap. The patches appear to provide an architecture neutral implementation. At that time s390 was just starting to use vmemmap bringing 2x implementations into the kernel. Now we have three. Clearly, even if vemmemap was a net performance loss having only one implementation has to be a good thing for maintainability/coverage. Without some benchmarking and some general testing it certainly is premature to be removing the other memory models, but for sure that was the original plan. The original assumption that as time went on the other models would wither on the vine, this has only happened on powerpc so far. So I will go and: 1) talk to s390 and find out if they can use this same form, as one implementation of vmemmap only should be the goal, and 2) take the current patches and try and get some benchmarks for them against other memory models Christoph if you could let us know which benchmarks you are seeing gains with that would be a help. -apw - 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/
Re: [PATCH 1/4] x86_64: Switch to SPARSE_VIRTUAL
Andi Kleen wrote: On Monday 02 April 2007 23:56:08 Dave Hansen wrote: On Mon, 2007-04-02 at 14:28 -0700, Christoph Lameter wrote: I do not care what its called as long as it covers all the bases and is not a glaring performance regresssion (like SPARSEMEM so far). I honestly don't doubt that there are regressions, somewhere. Could you elaborate, and perhaps actually show us some numbers on this? Perhaps instead of adding a completely new model, we can adapt the existing ones somehow. If it works I would be inclined to replaced old sparsemem with Christoph's new one on x86-64. Perhaps that could cut down the bewildering sparsemem ifdef jungle that is there currently. But I presume it won't work on 32bit because of the limited address space? Right. But we might be able to do switch SPARSEMEM_EXTREME users here if performance is better and no other regressions are detected. There seems to be a theme, we need to get some numbers. I will try and get what I can with the hardware I have and see whats missing. But, without some cold, hard, data, we mere mortals without the 1024-way machines can only guess. ;) -apw - 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/
Re: Checkpatch.pl failure
On Mon, Jan 14, 2008 at 09:48:53AM -0600, James Bottomley wrote: This error: ERROR: no space before that close parenthesis ')' #501: FILE: drivers/scsi/dpt_i2o.c:2299: + if (dev_status == 0x02 /*CHECK_CONDITION*/) { Is definitely wrong. I think it's stripped the comments so now the if looks to have a space before the bracket, but stylistically the complaint it has errored out for is wrong. Yes, that is kinda wrong. Its a difficult one to deal with nicely as basically spacing goes to hell when comments are wedged in the middle. The rules basically go out the window. In the next version I do at least have a handle on where comments are, but we have not yet progressed to where we can simply get the spacing checks right. I'll think more on it, and see what we can do. -apw -- 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/
Re: [PATCH] checkpatch.pl: show how to read from stdin
On Fri, Jan 11, 2008 at 06:06:35PM +0100, Stefan Richter wrote: Signed-off-by: Stefan Richter [EMAIL PROTECTED] Acked-by: Jiri Slaby [EMAIL PROTECTED] As an absolute minimum this seems reasonable to me. I guess we could make no arguments default to '-' also. There are up and downsides to doing that, as currently no arguments currently tell you the usage and with this patch would point clearly out the '-' option. Just assuming stding would lose easy access to usage, which may actually be more confusing for the beginner. Hmmm. Cirtainly will include this documentation change if nothing else. --- scripts/checkpatch.pl |1 + 1 file changed, 1 insertion(+) Index: linux/scripts/checkpatch.pl === --- linux.orig/scripts/checkpatch.pl +++ linux/scripts/checkpatch.pl @@ -53,6 +53,7 @@ if ($#ARGV 0) { print --file = check a source file\n; print --strict = enable more subjective tests\n; print --root = path to the kernel tree root\n; + print When patchfile is -, read standard input.\n; exit(1); } -apw -- 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/
Re: checkpatch: add filename before the summary line
On Sun, Jan 13, 2008 at 11:34:05PM +0100, Paolo Ciarrocchi wrote: With the patch applied the output of checkpatch.pl is as follow: ./arch/arm/mach-footbridge/ebsa285-pci.c total: 1 errors, 0 warnings, 48 lines checked Adding the file name allowed me to collects stats running: find . -name *.c |xargs ./scripts/checkpatch.pl --file |grep -B 1 total but I though it might me useful for other purposes to add the file name after the list of errors and warnings and before the summary, when the list is long I find handy to print out the filename as a reminder. Hmmm, that being unconfitional would probabally break a raft of other users. Also would it be more useful to put it on the front of the summary line? So that if you have a bunch of files to check then you get something more like: foo: total: 1 errors, ... bar: total: 0 errors, ... -apw -- 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/
Re: [PATCH] checkpatch.pl: show how to read from stdin
On Mon, Jan 14, 2008 at 09:35:15AM -0800, Daniel Walker wrote: On Mon, 2008-01-14 at 17:17 +, Andy Whitcroft wrote: On Fri, Jan 11, 2008 at 06:06:35PM +0100, Stefan Richter wrote: Signed-off-by: Stefan Richter [EMAIL PROTECTED] Acked-by: Jiri Slaby [EMAIL PROTECTED] As an absolute minimum this seems reasonable to me. I guess we could make no arguments default to '-' also. There are up and downsides to doing that, as currently no arguments currently tell you the usage and with this patch would point clearly out the '-' option. Just assuming stding would lose easy access to usage, which may actually be more confusing for the beginner. Hmmm. Cirtainly will include this documentation change if nothing else. The patch that I submitted checks STDIN for piped data, and if there is any it will default to checking that incoming data .. That's regardless of the number of arguments given .. So it does, however that of itself differs from the unix norm; as with this I cannot run checkpatch and type (ie paste) a patch fragment to check it. So I don't think we want the semantics as you have there, as its confusing to the experienced user and inconsistent with the norm. Either we should document the standard '-' usage as has been suggested elsewhere or always assume stdin with no parameters. -apw -- 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/
Re: EXPORTS_SYMBOLs that are in assembly?
On Sun, Jan 13, 2008 at 08:41:20PM -0500, Steven Rostedt wrote: Hi guys, Just wondering what the proper way to export a symbol that is defined in assembly? Or is there some kind of annotation I can add in comment form that will let checkpatch know the variable is not in C? Here's what I'm getting: ./scripts/checkpatch.pl patches/`quilt top` WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable #197: FILE: lib/tracing/mcount.c:42: +EXPORT_SYMBOL_GPL(mcount); total: 0 errors, 1 warnings, 316 lines checked Could you send me a fuller example fragment defining 'foo' as an example? It is entirly possible you are doing it right and this is an 'ignore checkpatch' situation. -apw -- 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/
[PATCH] update checkpatch.pl to version 0.10
This version brings a number of new checks, and a number of bug fixes. Of note: - better categorisation and space checks for dual use unary/binary operators - warn on deprecated use of {SPIN,RW}_LOCK_UNLOCKED - check if/for/while with trailing ';' for hanging statements - detect DOS line endings - detect redundant casts for kalloc() Andy Whitcroft (18): Version: 0.10 asmlinkage is also a storage type pull out inline specifiers allow only some operators before a unary operator parenthesised values may span line ends add additional attribute matching handle sparse annotations within pointer type space checks support alternative function definition syntax for typedefs check if/for/while with trailing ';' for hanging statements fix output format for case checks deprecate SPIN_LOCK_UNLOCKED and RW_LOCK_UNLOCKED allow complex macros with bracketing braces detect and report DOS line endings fastcall is a valid function attribute bracket spacing is ok for 'for' categorise operators into unary/binary/definitions add heuristic to pick up on unannotated types remove spurious warnings from cat_vet Dave Jones (1): Make checkpatch warn about pointless casting of kalloc returns. Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] --- scripts/checkpatch.pl | 254 +--- 1 files changed, 196 insertions(+), 58 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index dae7d30..59ad83c 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -9,7 +9,7 @@ use strict; my $P = $0; $P =~ [EMAIL PROTECTED]/@@g; -my $V = '0.09'; +my $V = '0.10'; use Getopt::Long qw(:config no_auto_abbrev); @@ -212,6 +212,11 @@ sub ctx_block_level { return ctx_block_get($linenr, $remain, 0, '{', '}', 0); } +sub ctx_statement_level { + my ($linenr, $remain, $off) = @_; + + return ctx_block_get($linenr, $remain, 0, '(', ')', $off); +} sub ctx_locate_comment { my ($first_line, $end_line) = @_; @@ -255,13 +260,42 @@ sub ctx_has_comment { return ($cmt ne ''); } +sub ctx_expr_before { + my ($line) = @_; + + ##print CHECK$line\n; + + my $pos = length($line) - 1; + my $count = 0; + my $c; + + for (; $pos = 0; $pos--) { + $c = substr($line, $pos, 1); + ##print CHECK: c$c count$count\n; + if ($c eq ')') { + $count++; + } elsif ($c eq '(') { + last if (--$count == 0); + } + } + + ##print CHECK: result . substr($line, 0, $pos) . \n; + + return substr($line, 0, $pos); +} + sub cat_vet { my ($vet) = @_; + my ($res, $coded); - $vet =~ s/\t/^I/; - $vet =~ s/$/\$/; + $res = ''; + while ($vet =~ /([^[:cntrl:]]*)([[:cntrl:]])/g) { + $coded = sprintf(^%c, unpack('C', $2) + 64); + $res .= $1 . $coded; + } + $res =~ s/$/\$/; - return $vet; + return $res; } my @report = (); @@ -310,8 +344,17 @@ sub process { my $first_line = 0; my $Ident = qr{[A-Za-z\d_]+}; - my $Storage = qr{extern|static}; - my $Sparse = qr{__user|__kernel|__force|__iomem|__must_check|__init_refok}; + my $Storage = qr{extern|static|asmlinkage}; + my $Sparse = qr{ + __user| + __kernel| + __force| + __iomem| + __must_check| + __init_refok| + fastcall + }x; + my $Inline = qr{inline|__always_inline|noinline}; my $NonptrType = qr{ \b (?:const\s+)? @@ -345,11 +388,18 @@ sub process { (?:\s+$Sparse)* }x; my $Declare = qr{(?:$Storage\s+)?$Type}; - my $Attribute = qr{const|__read_mostly|__init|__initdata|__meminit}; - + my $Attribute = qr{ + const| + __read_mostly| + __(?:mem|cpu|dev|)(?:initdata|init) + }x; my $Member = qr{-$Ident|\.$Ident|\[[^]]*\]}; my $Lval= qr{$Ident(?:$Member)*}; + # Possible bare types. + my @bare = (); + my $Bare = $NonptrType; + # Pre-scan the patch looking for any __setup documentation. my @setup_docs = (); my $setup_docs = 0; @@ -477,7 +527,11 @@ sub process { next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/); #trailing whitespace - if ($line =~ /^\+.*\S\s+$/ || $line =~ /^\+\s
Re: Kernel Panic - 2.6.23-rc4-mm1 ia64 - was Re: Update: [Automatic] NUMA replicated pagecache ...
On Wed, Sep 12, 2007 at 11:09:47AM -0400, Lee Schermerhorn wrote: Interesting, I don't see a memory controller function in the stack trace, but I'll double check to see if I can find some silly race condition in there. right. I noticed that after I sent the mail. Also, config available at: http://free.linux.hp.com/~lts/Temp/config-2.6.23-rc4-mm1-gwydyr-nomemcont Be interested to know the outcome of any bisect you do. Given its tripping in reclaim. What size of box is this? Wondering if we have anything big enough to test with. -apw - 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/
Re: [2/4] 2.6.23-rc6: known regressions
On Wed, Sep 12, 2007 at 06:58:54PM +0200, Michal Piotrowski wrote: FS Subject : hanging ext3 dbench tests References : http://lkml.org/lkml/2007/9/11/176 Last known good : ? Submitter : Andy Whitcroft [EMAIL PROTECTED] Caused-By : ? Handled-By : ? Status : unknown A number of attempts to reproduce this on these machines has yet to re-trigger it. I guess the Status is therefore under test -- unreproducible at present -apw - 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/
Re: [patch] add some Blackfin specific checks to checkpatch.pl
On Tue, Aug 21, 2007 at 06:29:59PM -0400, Mike Frysinger wrote: Check for a few common errors in Blackfin-specific code wrt MMR loading in assembly and doing core/system syncs. If we are going to pull arch specific things into checkpatch I think we need to make sure we are pretty specific about where we apply them. I am assuming these checks only make sense againt some files in the arch tree? -apw - 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/
Re: 2.6.23-rc4-mm1
On Tue, Sep 11, 2007 at 04:10:47AM +0900, FUJITA Tomonori wrote: The only patch which touches qla1280 is git-block.patch. From a quick squizz the change looks OK, although it's tricky and something might have broken. Can you try this patch (against 2.6.23-rc4-mm1)? Yep this patch seems to sort out booting on these boxes. The other one is also testing. Results later. From 592bd2049cb3e6e1f1dde7cf631879f26ddffeaa Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori [EMAIL PROTECTED] Date: Mon, 10 Sep 2007 04:17:13 +0100 Subject: [PATCH] qla1280: sg chaining fixes Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED] --- drivers/scsi/qla1280.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c index bd805ec..7c1eaec 100644 --- a/drivers/scsi/qla1280.c +++ b/drivers/scsi/qla1280.c @@ -2977,8 +2977,8 @@ qla1280_64bit_start_scsi(struct scsi_qla_host *ha, struct srb * sp) cpu_to_le32(pci_dma_hi32(dma_handle)), cpu_to_le32(pci_dma_lo32(dma_handle)), cpu_to_le32(sg_dma_len(s))); - remseg--; } + remseg -= cnt; dprintk(5, qla1280_64bit_start_scsi: continuation packet data - b %i, t %i, l %i \n, SCSI_BUS_32(cmd), @@ -3250,6 +3250,8 @@ qla1280_32bit_start_scsi(struct scsi_qla_host *ha, struct srb * sp) /* Load continuation entry data segments. */ for_each_sg(sg, s, remseg, cnt) { + if (cnt == 7) + break; *dword_ptr++ = cpu_to_le32(pci_dma_lo32(sg_dma_address(s))); *dword_ptr++ = @@ -3260,6 +3262,7 @@ qla1280_32bit_start_scsi(struct scsi_qla_host *ha, struct srb * sp) cpu_to_le32(pci_dma_lo32(sg_dma_address(s))), cpu_to_le32(sg_dma_len(s))); } + remseg -= cnt; dprintk(5, qla1280_32bit_start_scsi: continuation packet data - scsi(%i:%i:%i)\n, SCSI_BUS_32(cmd), -- 1.5.2.4 -apw - 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/
Re: 2.6.23-rc4-mm1
On Tue, Sep 11, 2007 at 04:31:12AM +0900, FUJITA Tomonori wrote: [...] The only patch which touches qla1280 is git-block.patch. From a quick squizz the change looks OK, although it's tricky and something might have broken. (the dprintk at line 2929 needs to print remseg, not seg_cnt). Can you retest with that change reverted (below)? If it's not that then perhaps something in scsi core broke, dunno. Even if we revert the qla1280 patch, scsi-ml still sends chaining sg list. So it doesn't work. The following patch disables chaining sg list for qla1280. If the fix that I've just sent doesn't work, please try this. Ok, the other patch _did_ work, but this got tested anyhow and it did _not_ fix things. - From: FUJITA Tomonori [EMAIL PROTECTED] Subject: [PATCH] add use_sg_chaining option to scsi_host_template This option is true if a low-level driver can support sg chaining. This will be removed eventually when all the drivers are converted to support sg chaining. q-max_phys_segments is set to SCSI_MAX_SG_SEGMENTS if false. Signed-off-by: FUJITA Tomonori [EMAIL PROTECTED] --- arch/ia64/hp/sim/simscsi.c|1 + drivers/scsi/3w-9xxx.c|1 + drivers/scsi/3w-.c|1 + drivers/scsi/BusLogic.c |1 + drivers/scsi/NCR53c406a.c |3 ++- drivers/scsi/a100u2w.c|1 + drivers/scsi/aacraid/linit.c |1 + drivers/scsi/aha1740.c|1 + drivers/scsi/aic7xxx/aic79xx_osm.c|1 + drivers/scsi/aic7xxx/aic7xxx_osm.c|1 + drivers/scsi/aic7xxx_old.c|1 + drivers/scsi/arcmsr/arcmsr_hba.c |1 + drivers/scsi/dc395x.c |1 + drivers/scsi/dpt_i2o.c|1 + drivers/scsi/eata.c |3 ++- drivers/scsi/hosts.c |1 + drivers/scsi/hptiop.c |1 + drivers/scsi/ibmmca.c |1 + drivers/scsi/ibmvscsi/ibmvscsi.c |1 + drivers/scsi/initio.c |1 + drivers/scsi/ipr.c|1 + drivers/scsi/lpfc/lpfc_scsi.c |2 ++ drivers/scsi/mac53c94.c |1 + drivers/scsi/megaraid.c |1 + drivers/scsi/megaraid/megaraid_mbox.c |1 + drivers/scsi/megaraid/megaraid_sas.c |1 + drivers/scsi/mesh.c |1 + drivers/scsi/nsp32.c |1 + drivers/scsi/pcmcia/sym53c500_cs.c|1 + drivers/scsi/qla2xxx/qla_os.c |2 ++ drivers/scsi/qla4xxx/ql4_os.c |1 + drivers/scsi/qlogicfas.c |1 + drivers/scsi/scsi_lib.c |5 - drivers/scsi/stex.c |1 + drivers/scsi/sym53c416.c |1 + drivers/scsi/sym53c8xx_2/sym_glue.c |1 + drivers/scsi/u14-34f.c|1 + drivers/scsi/ultrastor.c |1 + drivers/scsi/wd7000.c |1 + include/scsi/scsi_host.h | 13 + 40 files changed, 59 insertions(+), 3 deletions(-) diff --git a/arch/ia64/hp/sim/simscsi.c b/arch/ia64/hp/sim/simscsi.c index 4552a1c..e711657 100644 --- a/arch/ia64/hp/sim/simscsi.c +++ b/arch/ia64/hp/sim/simscsi.c @@ -360,6 +360,7 @@ static struct scsi_host_template driver_template = { .max_sectors= 1024, .cmd_per_lun= SIMSCSI_REQ_QUEUE_LEN, .use_clustering = DISABLE_CLUSTERING, + .use_sg_chaining= ENABLE_SG_CHAINING, }; static int __init diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c index efd9d8d..fb14014 100644 --- a/drivers/scsi/3w-9xxx.c +++ b/drivers/scsi/3w-9xxx.c @@ -1990,6 +1990,7 @@ static struct scsi_host_template driver_template = { .max_sectors= TW_MAX_SECTORS, .cmd_per_lun= TW_MAX_CMDS_PER_LUN, .use_clustering = ENABLE_CLUSTERING, + .use_sg_chaining= ENABLE_SG_CHAINING, .shost_attrs= twa_host_attrs, .emulated = 1 }; diff --git a/drivers/scsi/3w-.c b/drivers/scsi/3w-.c index c7995fc..a64153b 100644 --- a/drivers/scsi/3w-.c +++ b/drivers/scsi/3w-.c @@ -2261,6 +2261,7 @@ static struct scsi_host_template driver_template = { .max_sectors= TW_MAX_SECTORS, .cmd_per_lun= TW_MAX_CMDS_PER_LUN, .use_clustering = ENABLE_CLUSTERING, + .use_sg_chaining= ENABLE_SG_CHAINING, .shost_attrs= tw_host_attrs, .emulated = 1 }; diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c index 9b20617..49e1ffa 100644 --- a/drivers/scsi/BusLogic.c +++ b/drivers/scsi/BusLogic.c @@ -3575,6 +3575,7 @@ static struct scsi_host_template Bus_Logic_template = { .unchecked_isa_dma = 1, .max_sectors = 128,
Re: 2.6.23-rc6: hanging ext3 dbench tests
On Tue, Sep 11, 2007 at 06:30:49PM +0100, Andy Whitcroft wrote: Annoyingly this seems to be intermittent, and I have not managed to get a machine into this state again yet. Will keep trying. Ok, I have been completly unsuccessful in reproducing this. Dispite having two distinct machines showing this behaviour. I have neither been able to reproduce it with those machine on 2.6.23-rc6 nor has any of the testing of any of the -git releases which follow thrown this error. I have run about 10 repeats of the jobs which failed too and none of those have thrown the same error. It is pretty clear from the dbench output that the problem is/was real, that its not some artifact of the test harness. I am a loss as to how to get this to trigger again. I guess I will keep monitoring the ongoing tests for new instances. I will also look to getting the sysrq-* stuff triggered automatically on job timeout as that seems like a sane plan in all cases. Frustrated. -apw - 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/
Re: [BUG][2.6.23-rc6] Badness at arch/powerpc/kernel/smp.c:202
Anton, this seems a little reminicient of that bug which popped up in 2.6.23-rc3 so do with SLB loading (if memory serves), with machine checks and signal 7's. Of course that is _supposed_ to be fixed by this time ... I believe it was Paul who fixed up that one, and he is already copied. -apw On Fri, Sep 14, 2007 at 04:07:37PM +0530, Satyam Sharma wrote: With 2.6.23-rc6 running on the ppc64 box, following oops is hit Oops: Machine check, sig: 7 [#1] SMP NR_CPUS=128 pSeries Modules linked in: binfmt_misc ipv6 dm_mod ehci_hcd ohci_hcd usbcore NIP: c00ed560 LR: c00efc7c CTR: c00ed504 REGS: cffef680 TRAP: 0200 Not tainted (2.6.23-rc6-autokern1) MSR: 80109032 EE,ME,IR,DR CR: 28002042 XER: 0010 TASK = c000ecf9f000[0] 'swapper' THREAD: cff8c000 CPU: 2 GPR00: cffef900 c06fe598 c000d7a8f200 GPR04: 1000 1000 80c26393 GPR08: c06b43d0 0001 1000 GPR12: 4848 c05f1700 07a8dcd0 GPR16: 0002 GPR20: 1000 1000 GPR24: 1000 c63234e8 GPR28: 1000 c0689c08 cff3a480 NIP [c00ed560] .end_bio_bh_io_sync+0x5c/0xac LR [c00efc7c] .bio_endio+0xb4/0xd4 Call Trace: [cffef900] [cffef990] 0xcffef990 (unreliable) [cffef980] [c00efc7c] .bio_endio+0xb4/0xd4 [cffefa10] [c0290060] .__end_that_request_first+0x154/0x548 [cffefae0] [c035af10] .scsi_end_request+0x40/0x138 [cffefb80] [c035b234] .scsi_io_completion+0x188/0x454 [cffefc60] [c0372a24] .sd_rw_intr+0x2e4/0x338 [cffefd30] [c0354548] .scsi_finish_command+0xbc/0xe0 [cffefdc0] [c035bdf0] .scsi_softirq_done+0x140/0x188 [cffefe60] [c0293184] .blk_done_softirq+0xa0/0xd0 [cffefef0] [c0055e1c] .__do_softirq+0xa8/0x164 [cffeff90] [c0023f14] .call_do_softirq+0x14/0x24 [cff8f960] [c000bd30] .do_softirq+0x68/0xac [cff8f9f0] [c0055f70] .irq_exit+0x54/0x6c [cff8fa70] [c000c358] .do_IRQ+0x170/0x1ac [cff8fb00] [c0004780] hardware_interrupt_entry+0x18/0x98 --- Exception: 501 at .pseries_dedicated_idle_sleep+0xe0/0x194 LR = .pseries_dedicated_idle_sleep+0xd0/0x194 [cff8fdf0] [] .__start+0x4000/0x8 (unreliable) [cff8fe80] [c0010bd4] .cpu_idle+0x104/0x1d8 [cff8ff00] [c002672c] .start_secondary+0x160/0x184 [cff8ff90] [c0008364] .start_secondary_prolog+0xc/0x10 Instruction dump: 409a0030 393f0018 3880 7d6048a8 7d6b0378 7d6049ad 40a2fff4 38002000 7d2018a8 7d290378 7d2019ad 40a2fff4 e9230038 e89f0018 e969 f8410028 Kernel panic - not syncing: Fatal exception in interrupt This oops is the real bug here, but is that a machine check exception? If so, it could be a hardware failure what you saw there instead, and not really a kernel bug ... - 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/
Re: 2.6.23-rc6-mm1
On Tue, Sep 18, 2007 at 02:43:48PM +0530, Kamalesh Babulal wrote: Andrew Morton wrote: ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc6/2.6.23-rc6-mm1/ 2.6.23-rc6-mm1 is a 29MB diff against 2.6.23-rc6. snip Hi Andrew, The 2.6.23-rc6-mm1build fails at CC drivers/pci/hotplug/rpadlpar_core.o CC drivers/pci/hotplug/rpadlpar_sysfs.o drivers/pci/hotplug/rpadlpar_sysfs.c:132: error: unknown field `name' specified in initializer drivers/pci/hotplug/rpadlpar_sysfs.c: In function `dlpar_sysfs_init': drivers/pci/hotplug/rpadlpar_sysfs.c:142: error: structure has no member named `name' make[3]: *** [drivers/pci/hotplug/rpadlpar_sysfs.o] Error 1 make[2]: *** [drivers/pci/hotplug] Error 2 make[1]: *** [drivers/pci] Error 2 make: *** [drivers] Error 2 This seems to be occuring across a number of the powerpc systems we test with. That driver is a power dynamic lpar IO partitioning driver. Relevant Cc: added. -apw - 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/
2.6.23-rc6-mm1 -- powerpc link failure
I am seeing this strange link error from a PowerMac G5 (powerpc): [...] KSYM.tmp_kallsyms2.S AS .tmp_kallsyms2.o LD vmlinux.o ld: dynreloc miscount for fs/built-in.o, section .opd ld: can not edit opd Bad value make: *** [vmlinux.o] Error 1 Compiler version below. [EMAIL PROTECTED]:~# gcc -v Using built-in specs. Target: powerpc-linux-gnu Configured with: ../src/configure -v --enable-languages=c,c++,java,f95,objc,ada,treelang --prefix=/usr --enable-shared --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --program-suffix=-4.0 --enable-__cxa_atexit --enable-clocale=gnu --enable-libstdcxx-debug --enable-java-awt=gtk-default --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.4.2-gcj-4.0-1.4.2.0/jre --enable-mpfr --disable-softfloat --enable-targets=powerpc-linux,powerpc64-linux --with-cpu=default32 --disable-werror --enable-checking=release powerpc-linux-gnu Thread model: posix gcc version 4.0.3 (Ubuntu 4.0.3-1ubuntu5) -apw - 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/
2.6.23-rc6-mm1 -- powerpc pSeries_log_error panic in rtas_call/early_enable_eeh
Seeing the following panic booting an old powerpc LPAR: Unable to handle kernel paging request for data at address 0x Faulting instruction address: 0xc0047b48 cpu 0x0: Vector: 300 (Data Access) at [c06a3750] pc: c0047b48: .pSeries_log_error+0x364/0x420 lr: c0047acc: .pSeries_log_error+0x2e8/0x420 sp: c06a39d0 msr: 80001032 dar: 0 dsisr: 4200 current = 0xc05acab0 paca= 0xc05ad700 pid = 0, comm = swapper enter ? for help [c06a3af0] c0021164 .rtas_call+0x200/0x250 [c06a3ba0] c0049d50 .early_enable_eeh+0x168/0x360 [c06a3c70] c002f674 .traverse_pci_devices+0x8c/0x138 [c06a3d10] c0560ce8 .eeh_init+0x1a8/0x200 [c06a3db0] c055fb70 .pSeries_setup_arch+0x128/0x234 [c06a3e40] c054f830 .setup_arch+0x214/0x24c [c06a3ee0] c0546a38 .start_kernel+0xd4/0x3e4 [c06a3f90] c045adc4 .start_here_common+0x54/0x58 0:mon This machine is: # cat /proc/cpuinfo processor : 0 cpu : POWER4+ (gq) clock : 1703.965296MHz revision: 19.0 [...] machine : CHRP IBM,7040-681 -apw - 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/
2.6.23-rc6-mm1 -- mkfs stuck in 'D'
Seems I have a case of a largish i386 NUMA (NUMA-Q) which has a mkfs stuck in a 'D' wait: === mkfs.ext2 D c10220f4 0 6233 6222 c344fc80 0082 0286 c10220f4 c344fc90 002ed099 c2963340 c2b9f640 c142bce0 c2b9f640 c344fc90 002ed099 c344fcfc c344fcc0 c1219563 c1109bf2 c344fcc4 c186e4d4 c186e4d4 002ed099 c1022612 c2b9f640 c186e000 c104000c Call Trace: [c10220f4] lock_timer_base+0x19/0x35 [c1219563] schedule_timeout+0x70/0x8d [c1109bf2] prop_fraction_single+0x37/0x5d [c1022612] process_timeout+0x0/0x5 [c104000c] task_dirty_limit+0x3a/0xb5 [c12194da] io_schedule_timeout+0x1e/0x28 [c10454b4] congestion_wait+0x62/0x7a [c102b021] autoremove_wake_function+0x0/0x33 [c10402af] get_dirty_limits+0x16a/0x172 [c102b021] autoremove_wake_function+0x0/0x33 [c104040b] balance_dirty_pages+0x154/0x1be [c103bda3] generic_perform_write+0x168/0x18a [c103be38] generic_file_buffered_write+0x73/0x107 [c103c346] __generic_file_aio_write_nolock+0x47a/0x4a5 [c11b0fef] do_sock_write+0x92/0x99 [c11b1048] sock_aio_write+0x52/0x5e [c103c3b9] generic_file_aio_write_nolock+0x48/0x9b [c105d2d6] do_sync_write+0xbf/0xfc [c102b021] autoremove_wake_function+0x0/0x33 [c1010311] do_page_fault+0x2cc/0x739 [c105d3a0] vfs_write+0x8d/0x108 [c105d4c3] sys_write+0x41/0x67 [c100260a] syscall_call+0x7/0xb === This machine and others have run numerous test runs on this kernel and this is the first time I've see a hang like this. I wonder if this is the ultimate cause of the couple of mainline hangs which were seen, but not diagnosed. -apw - 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/
Re: 2.6.23-rc6-mm1 -- powerpc link failure
On Wed, Sep 19, 2007 at 06:36:29PM +0200, Segher Boessenkool wrote: I am seeing this strange link error from a PowerMac G5 (powerpc): [...] KSYM.tmp_kallsyms2.S AS .tmp_kallsyms2.o LD vmlinux.o ld: dynreloc miscount for fs/built-in.o, section .opd ld: can not edit opd Bad value make: *** [vmlinux.o] Error 1 Compiler version below. It's an ld error, could you show us your ld version instead? And please try with current mainline ld, too? [EMAIL PROTECTED]:~# ld -v GNU ld version 2.16.91 20060118 Debian GNU/Linux Getting the compiler suite changed on here is going to be a lot tricker. One of the reasons we keep it back there is those versions are supposed to be supported and we want to test those combinations. -apw - 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/
Re: 2.6.23-rc6: hanging ext3 dbench tests
On Fri, Sep 14, 2007 at 10:49:05AM +0100, Andy Whitcroft wrote: On Tue, Sep 11, 2007 at 06:30:49PM +0100, Andy Whitcroft wrote: Annoyingly this seems to be intermittent, and I have not managed to get a machine into this state again yet. Will keep trying. Ok, I have been completly unsuccessful in reproducing this. Dispite having two distinct machines showing this behaviour. I have neither been able to reproduce it with those machine on 2.6.23-rc6 nor has any of the testing of any of the -git releases which follow thrown this error. I have run about 10 repeats of the jobs which failed too and none of those have thrown the same error. It is pretty clear from the dbench output that the problem is/was real, that its not some artifact of the test harness. I am a loss as to how to get this to trigger again. I guess I will keep monitoring the ongoing tests for new instances. I will also look to getting the sysrq-* stuff triggered automatically on job timeout as that seems like a sane plan in all cases. Frustrated. I have since had a single occurance of a hang on 2.6.23-rc6-mm1. As the base is different I cannot for sure say its the same problem. In this new event we had a mkfs hung in a 'D' wait: === mkfs.ext2 D c10220f4 0 6233 6222 c344fc80 0082 0286 c10220f4 c344fc90 002ed099 c2963340 c2b9f640 c142bce0 c2b9f640 c344fc90 002ed099 c344fcfc c344fcc0 c1219563 c1109bf2 c344fcc4 c186e4d4 c186e4d4 002ed099 c1022612 c2b9f640 c186e000 c104000c Call Trace: [c10220f4] lock_timer_base+0x19/0x35 [c1219563] schedule_timeout+0x70/0x8d [c1109bf2] prop_fraction_single+0x37/0x5d [c1022612] process_timeout+0x0/0x5 [c104000c] task_dirty_limit+0x3a/0xb5 [c12194da] io_schedule_timeout+0x1e/0x28 [c10454b4] congestion_wait+0x62/0x7a [c102b021] autoremove_wake_function+0x0/0x33 [c10402af] get_dirty_limits+0x16a/0x172 [c102b021] autoremove_wake_function+0x0/0x33 [c104040b] balance_dirty_pages+0x154/0x1be [c103bda3] generic_perform_write+0x168/0x18a [c103be38] generic_file_buffered_write+0x73/0x107 [c103c346] __generic_file_aio_write_nolock+0x47a/0x4a5 [c11b0fef] do_sock_write+0x92/0x99 [c11b1048] sock_aio_write+0x52/0x5e [c103c3b9] generic_file_aio_write_nolock+0x48/0x9b [c105d2d6] do_sync_write+0xbf/0xfc [c102b021] autoremove_wake_function+0x0/0x33 [c1010311] do_page_fault+0x2cc/0x739 [c105d3a0] vfs_write+0x8d/0x108 [c105d4c3] sys_write+0x41/0x67 [c100260a] syscall_call+0x7/0xb === -apw - 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/
Re: [Celinux-dev] [Announce] Linux-tiny project revival
On Thu, Sep 20, 2007 at 12:38:55AM +0200, Michael Opdenacker wrote: Andrew, you're completely right... The patches should all aim at being included into mainline or die. I'm finishing a sequence of crazy weeks and I will have time to send you patches one by one next week, starting with the easiest ones. Well thats good news. In response to the comments made about testing the impact of these patches on big-iron I was going to suggest we ask Andrew to include your patch set in -mm so that it firstly gets at least compiled on big-iron, and secondly so we could think about how to test with some of the options enabled on big-iron. Knowing nothing about these options, from a test perspective it would be nice if we were able to simply enable the lot so we can do normal -mm runs and tiny -mm runs without any manual intervention? -apw - 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/
Re: Warning: commit message does not conform to UTF-8. (mmotm 10-Nov-2007 22:46)
On Sat, Nov 10, 2007 at 07:30:57PM -0500, Erez Zadok wrote: Andrew, I'm getting minor warnings when applying two patches from http://userweb.kernel.org/~akpm/mmotm/ It's probably not serious but I wonder if checkpatch catches this. Applying patch..git-net.patch Warning: commit message does not conform to UTF-8. You may want to amend it after fixing the message, or set the config variable i18n.commitencoding to the encoding your project uses. Applying patch..mnt_unbindable-fix.patch Warning: commit message does not conform to UTF-8. You may want to amend it after fixing the message, or set the config variable i18n.commitencoding to the encoding your project uses. We do have checks for UTF-8. The commit message problems are usually real names in Signed-off-by:s being complex and being in their email locale not UTF-8. It is a problem area for sure as people want their email to look right in their email to the list, and yet they need it in UTF-8 for git. Yeah it looks like we pick it up ok, the message could do with a little love: [EMAIL PROTECTED] ~/git/checkpatch/checkpatch.pl -q --no-tree ./git-net.patch ERROR: Invalid UTF-8 #229: This is based on a patch by Vicenç Beltran Querol. ERROR: Invalid UTF-8 #398: by Toralf Förster: ERROR: Invalid UTF-8 #773: Signed-off-by: Lutz Preßler [EMAIL PROTECTED] -apw - 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/
Re: checkpatch.pl and no newline handling
On Mon, Nov 12, 2007 at 02:46:30PM -0500, Mike Frysinger wrote: the current checkpatch.pl does not reject new files that lack a newline, yet rejects patches that fix newlines in files ... quite the opposite of what we actually want Nice. Just what the world needs. I wonder what the heck that format means. Obviously the corrupt check is wrong as a \ No newline line is valid not corrupt. Now when is \ No newline a good thing and when a bad thing. It _looks_ like it is 'bound' to the line before, and if so then its pretty simple. Bad: +moo \ No newline at end of file Good: -moo \ No newline at end of file /me goes read the source for diff. Sigh. Thanks for the report. -apw - 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/
Re: checkpatch.pl and no newline handling
On Mon, Nov 12, 2007 at 02:46:30PM -0500, Mike Frysinger wrote: the current checkpatch.pl does not reject new files that lack a newline, yet rejects patches that fix newlines in files ... quite the opposite of what we actually want [EMAIL PROTECTED] echo -n moo no-newline.c [EMAIL PROTECTED] diff -Nu /dev/null no-newline.c | ./checkpatch.pl -q -no-tree --no-signoff - WARNING: adding a line without newline at end of file #4: FILE: no-newline.c:1: +moo total: 0 errors, 1 warnings, 1 lines checked [EMAIL PROTECTED] echo moo newline.c [EMAIL PROTECTED] diff -Nu no-newline.c newline.c | ./checkpatch.pl -q -no-tree --no-signoff - total: 0 errors, 0 warnings, 3 lines checked [EMAIL PROTECTED] This should be fixed in the 0.12 release and is fixed in -next. -apw - 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/
[PATCH] update checkpatch.pl to version 0.12
This version brings a new terse output mode as well as many improvements to the unary detection and bare type regcognition. It also brings the usual updates for false positives, though these seem to be slowing markedly now that the unary detector is no longer just putting its finger in the air and guessing. Of note: - new --terse mode producing a single line per report - loosening of the block brace checks - new checks for enum/union/struch brace placements - hugely expanded bare type detection - checks for inline usage - better handling of already open comment blocks - handle patches which introduce or remove lines without newlines Andy Whitcroft (19): Version: 0.12 style fixes as spotted by checkpatch add a --terse options of a single line of output per report block brace checks should only apply for single line blocks all new bare type detector check spacing for open braces with enum, union and struct check for LINUX_VERSION_CODE macros definition bracketing checks need to ignore -ve context clean up the mail-back mode, -q et al expand possible type matching to declarations allow const and sparse annotations on possible types handle possible types as regular types everywhere prefer plain inline over __inline__ and __inline all new open comment detection fix up conditional extraction for if assignment checks add const to the possible type matcher unary checks: a for loop is a conditional too possible types: detect function pointer definitions handle missind newlines at end of file, report addition Signed-off-by: Andy Whitcroft [EMAIL PROTECTED] --- All versions available at: http://www.kernel.org/pub/linux/kernel/people/apw/checkpatch/ scripts/checkpatch.pl | 395 +++-- 1 files changed, 287 insertions(+), 108 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index cbb4258..579f50f 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -9,7 +9,7 @@ use strict; my $P = $0; $P =~ [EMAIL PROTECTED]/@@g; -my $V = '0.11'; +my $V = '0.12'; use Getopt::Long qw(:config no_auto_abbrev); @@ -19,8 +19,11 @@ my $chk_signoff = 1; my $chk_patch = 1; my $tst_type = 0; my $emacs = 0; +my $terse = 0; my $file = 0; my $check = 0; +my $summary = 1; +my $mailback = 0; my $root; GetOptions( 'q|quiet+' = \$quiet, @@ -29,10 +32,13 @@ GetOptions( 'patch!'= \$chk_patch, 'test-type!'= \$tst_type, 'emacs!'= \$emacs, + 'terse!'= \$terse, 'file!' = \$file, 'subjective!' = \$check, 'strict!' = \$check, 'root=s'= \$root, + 'summary!' = \$summary, + 'mailback!' = \$mailback, ) or exit; my $exit = 0; @@ -42,6 +48,7 @@ if ($#ARGV 0) { print version: $V\n; print options: -q = quiet\n; print --no-tree= run without a kernel tree\n; + print --terse = one line per report\n; print --emacs = emacs compile window format\n; print --file = check a source file\n; print --strict = enable more subjective tests\n; @@ -49,6 +56,11 @@ if ($#ARGV 0) { exit(1); } +if ($terse) { + $emacs = 1; + $quiet++; +} + if ($tree) { if (defined $root) { if (!top_of_kernel_tree($root)) { @@ -90,41 +102,6 @@ our $Attribute = qr{ __(?:mem|cpu|dev|)(?:initdata|init) }x; our $Inline= qr{inline|__always_inline|noinline}; -our $NonptrType= qr{ - \b - (?:const\s+)? - (?:unsigned\s+)? - (?: - void| - char| - short| - int| - long| - unsigned| - float| - double| - bool| - long\s+int| - long\s+long| - long\s+long\s+int| - (?:__)?(?:u|s|be|le)(?:8|16|32|64)| - struct\s+$Ident| - union\s+$Ident| - enum\s+$Ident| - ${Ident}_t| - ${Ident}_handler| - ${Ident}_handler_fn - ) - (?:\s+$Sparse)* - \b - }x; - -our $Type = qr{ - \b$NonptrType\b - (?:\s*\*+\s*const|\s*\*+|(?:\s
Re: 2.6.24-rc2-mm1 -- mkfs failing on variety of fs types
We seem to have some general problem with mkfs for all filesystems. I am seeing this across at least three test systems although most are unable to compile this kernel :(, even with the hotfix. Basically, all mkfs operations for any filsystem type are failing, ext2 reports this as short write, various others are mentioning pwrite and pwrite64 returning bad things: ext2: Could not write 8 blocks in inode table starting at 851970: Attempt to write block from filesystem resulted in short write reiserfs: bwrite: write 4096 bytes returned -1 (block=851968, dev=3): No space left on device xfs: mkfs.xfs: pwrite64 failed: No space left on device Nothing is reported in dmesg at the time as far as I can tell. From the ext2 log I would swear we get this error on a block number far below that which is reported written successfully, though I cannot say I trust mkfs. Nothing obvious has changed pwrite or block/* to my eye, so heck knows where this is coming from. 2.6.24-rc2 works on these same systems as goes the latest 2.6.24-rc2-git5. Full mkfs output below. -apw *** elm3b6, x86_64: mke2fs 1.37 (21-Mar-2005) Filesystem label= OS type: Linux Block size=4096 (log=2) Fragment size=4096 (log=2) 1465920 inodes, 2929854 blocks 146492 blocks (5.00%) reserved for the super user First data block=0 90 block groups 32768 blocks per group, 32768 fragments per group 16288 inodes per group Superblock backups stored on blocks: 32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208 mkfs.ext2: Attempt to write block from filesystem resulted in short write while zeroing block 2929824 at end of filesystem Writing inode tables: Could not write 8 blocks in inode table starting at 851970: Attempt to write block from filesystem resulted in short write === mkfs.jfs version 1.1.7, 22-Jul-2004 The specified disk did not finish formatting. === mkfs.reiserfs 3.6.19 (2003 www.namesys.com) [...] Guessing about desired format.. Kernel 2.6.24-rc2-mm1-autokern1 is running. Format 3.6 with standard journal Count of blocks on the device: 2929840 Number of blocks consumed by mkreiserfs formatting process: 8301 Blocksize: 4096 Hash function used to sort names: r5 Journal Size 8193 blocks (first block 18) Journal Max transaction length 1024 inode generation number: 0 UUID: c759e218-681b-4891-b4c4-33466d4eb4f0 Initializing journal - 0%20%40%60%80%100% bwrite: write 4096 bytes returned -1 (block=851968, dev=3): No space left on device === mkfs.xfs: pwrite64 failed: No space left on device meta-data=/dev/sdb2 isize=256agcount=16, agsize=183115 blks = sectsz=512 data = bsize=4096 blocks=2929840, imaxpct=25 = sunit=0 swidth=0 blks, unwritten=1 naming =version 2 bsize=4096 log =internal log bsize=4096 blocks=2560, version=1 = sectsz=512 sunit=0 blks realtime =none extsz=65536 blocks=0, rtextents=0 *** elm3b239, x86_64: mke2fs 1.38 (30-Jun-2005) Filesystem label= OS type: Linux Block size=4096 (log=2) Fragment size=4096 (log=2) 2410624 inodes, 4819500 blocks 240975 blocks (5.00%) reserved for the super user First data block=0 148 block groups 32768 blocks per group, 32768 fragments per group 16288 inodes per group Superblock backups stored on blocks: 32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208, 4096000 mkfs.ext2: Attempt to write block from filesystem resulted in short write while zeroing block 4819472 at end of filesystem Writing inode tables: Could not write 8 blocks in inode table starting at 655362: Attempt to write block from filesystem resulted in short write *** pSeries-101, ppc64 mke2fs 1.38 (30-Jun-2005) Filesystem label= OS type: Linux Block size=4096 (log=2) Fragment size=4096 (log=2) 1281696 inodes, 256 blocks 128000 blocks (5.00%) reserved for the super user First data block=0 79 block groups 32768 blocks per group, 32768 fragments per group 16224 inodes per group Superblock backups stored on blocks: 32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632 mkfs.ext2: Attempt to write block from filesystem resulted in short write while zeroing block 2559984 at end of filesystem Writing inode tables: Could not write 8 blocks in inode table starting at 491522: Attempt to write block from filesystem resulted in short write *** gekko-lp1, ppc64 mke2fs 1.38 (30-Jun-2005) Filesystem label= OS type: Linux Block size=4096 (log=2) Fragment size=4096 (log=2) 1224000 inodes, 2443880 blocks 122194 blocks (5.00%) reserved for the super user First data block=0 75 block groups 32768 blocks per group, 32768 fragments per group 16320 inodes per group Superblock backups stored on blocks: 32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632 mkfs.ext2: Attempt to write block from filesystem
Re: 2.6.24-rc2-mm1 (memory hotplug x86_64/vmemmap fix)
-rc2-mm1.orig/mm/sparse-vmemmap.c +++ linux-2.6.24-rc2-mm1/mm/sparse-vmemmap.c @@ -30,6 +30,17 @@ #include asm/pgtable.h /* + * wrapper for calling bootmem alloc from __meminit code. + */ +void __init_refok *__call_alloc_bootmem(int node, + int size, int align, int goal) +{ + return __alloc_bootmem_node(NODE_DATA(node), size, align, goal); +} + Same comment on naming here, static etc. + + +/* * Allocate a block of memory to be used to back the virtual memory map * or to back the page tables that are used to create the mapping. * Uses the main allocators if they are available, else bootmem. @@ -44,7 +55,7 @@ void * __meminit vmemmap_alloc_block(uns return page_address(page); return NULL; } else - return __alloc_bootmem_node(NODE_DATA(node), size, size, + return __call_alloc_bootmem(node, size, size, __pa(MAX_DMA_ADDRESS)); } Reviewed-by: Andy Whitcroft [EMAIL PROTECTED] -apw - 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/
Re: 2.6.24-rc2-mm1 -- strange apparent network failures
When testing some of the later 2.6.24-rc2-mm1+hotfix combinations on three of our test systems one job from each batch (1/4) failed. In each case the machine appears to have booted normally all the way to a login: prompt. However in the failed boots the networking though apparently initialised completely and correctly (as far as I can tell from the console output), is reported as not responding to ssh connections. The network interface seems to have been initialised on the right port, and the ssh daemons started. Two of the machines are powerpc boxes, the other an older x86_64. One machine is 4/4 in testing, just one. Most of the other machines are still not able to compile this stack so do not contribute to our knowledge. Any ideas? -apw - 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/
Re: 2.6.24-rc2-mm1 -- QLogics ISP1020 gone missing
All of our machines with QLogics ISP1020 cards seem to have lost them on boot with 2.6.24-rc1-mm1+hotfixes. # lspci :00:0a.0 SCSI storage controller: QLogic Corp. ISP1020 Fast-wide SCSI (rev 05) # lspci -n :00:0a.0 0100: 1077:1020 (rev 05) # lspci -v -v :00:0a.0 SCSI storage controller: QLogic Corp. ISP1020 Fast-wide SCSI (rev 05) Subsystem: QLogic Corp.: Unknown device Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR+ FastB2B- Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium TAbort- TAbort- MAbort- SERR- PERR- Latency: 248, Cache Line Size: 0x08 (32 bytes) Interrupt: pin A routed to IRQ 23 Region 0: I/O ports at fc00 [size=256] Region 1: Memory at fa00 (32-bit, non-prefetchable) [size=4K] These devices are normally reported as below, which I note is not as a 1020? qla1280: QLA1040 found on PCI bus 0, dev 10 scsi(0:0): Resetting SCSI BUS scsi0 : QLogic QLA1040 PCI to SCSI Host Adapter Firmware version: 7.65.06, Driver version 3.26 There is nothing major around in the area so I am somewhat bemused. -apw - 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/
Re: 2.6.24-rc2-mm1 -- strange apparent network failures
On Fri, Nov 16, 2007 at 09:16:58PM -0800, Andrew Morgan wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi, This warning is just saying that you might want to reconsider recompiling your dhclient with a newer libcap - which has native support for 64-bit capabilities. This is supposed to be informative, and not be associated with any particular error. - From your comments, you believe that this patch causes something in your boot process to fail. Can you supply some detail about the version of dhclient you are using? I'd like to understand exactly what it is doing (via libcap). Thanks The machine which show this problem for me are using static network configurations, so I don't know if libcap is still in the mix there. I've just compared the boot logs from a successful and unsuccessful boot on this kernel, and I don't see that particular message, nor do I see any significant differences overall. Perlexed. -apw - 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/
Re: [PATCH] [RFC] Xilinx SystemACE: Add media hotplug support
On Wed, Dec 19, 2007 at 11:23:48PM -0700, Grant Likely wrote: + /* Make the sysace device 'live' */ + if (ace-fsm_state != ACE_FSM_STATE_INVALIDATE_MEDIA); + add_disk(ace-gd); checkpatch.pl reports the above if as suspect due to the trailing ;. Looks wrong to me too. WARNING: Trailing semicolon indicates no statements, indent implies otherwise #296: FILE: drivers/block/xsysace.c:835: + if (ace-fsm_state != ACE_FSM_STATE_INVALIDATE_MEDIA); + add_disk(ace-gd); -apw -- 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/
Re: Trailing periods in kernel messages
On Thu, Nov 29, 2007 at 06:35:28PM -0800, Joe Perches wrote: On Fri, 2007-11-30 at 09:54 +0800, Li Zefan wrote: So it doesn't deserve the effort to eliminate these periods, isn't it? I hope these will eventually disappear. Or we can add a check to checkpatch.pl to prevent new ones. Perhaps that's a good idea. diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index cbb4258..707f84c 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1390,6 +1390,10 @@ sub process { if ($line =~ /\*\s*\)\s*k[czm]alloc\b/) { WARN(unnecessary cast may hide bugs, see http://c-faq.com/malloc/mallocnocast.html\n; . $herecurr); } + + if ($rawline =~ /(print|pr_(emerg|alert|crit|err|warning|notice|info|debug)).*\.\\n\/) { + WARN(unnecessary period before newline\n . $herecurr); + } I missed the context on this one. So this is checking for periods at the end of messages for printk's. We would need something a little cleverer to ensure we are only checking the contents of the string. But eminently doable. /me plays Ok, yes this seems ok. Have added it for 0.13. -apw -- 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/
Re: [PATCH] SC26XX: New serial driver for SC2681 uarts
On Mon, Dec 03, 2007 at 03:53:17PM -0800, Andrew Morton wrote: +#define READ_SC(p, r)readb ((p)-membase + RD_##r) +#define WRITE_SC(p, r, v)writeb ((v), (p)-membase + WR_##r) No space before the (. checkpatch misses this. Yep, over careful in the special case for the parameters for #define macros, which have different spacing rules. This will be fixed in 0.13: WARNING: no space between function name and open parenthesis '(' #1: FILE: Z45.c:1: +#define WRITE_SC(p, r, v)writeb ((v), (p)-membase + WR_##r) -apw -- 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/
Re: [PATCH] scripts/checkpatch.pl: add a check for the patch level (patch -pnum)
On Tue, Dec 18, 2007 at 06:46:41AM +0100, Borislav Petkov wrote: On Mon, Dec 17, 2007 at 08:11:05AM +0100, Borislav Petkov wrote: A slightly microoptimized version 1.1: --- From: Borislav Petkov [EMAIL PROTECTED] Check the patch level of the single hunks in a patch file, however only when checkpatch.pl is called from within the kernel tree. Signed-off-by: Borislav Petkov [EMAIL PROTECTED] -- diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 579f50f..3eda27b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -653,6 +653,18 @@ sub CHK { } } +sub check_patchlevel { + + if ($tree) { + my ($path) = @_; + $path =~ s![^/]*/!!; + + if (!stat($path)) { + WARN(Check the patchlevel (hint: patch option -p)); + } + } Hmmm that will trigger on all patches which create new files if I am grokking you correctly. I would have thought this would pretty easy to check from the form of the names. Hmmm. +} + sub process { my $filename = shift; my @lines = @_; @@ -713,10 +725,16 @@ sub process { #extract the filename as it passes if ($line=~/^\+\+\+\s+(\S+)/) { $realfile=$1; + + if ($realfile) { + check_patchlevel($realfile); + } + $realfile =~ [EMAIL PROTECTED]/]*/@@; $in_comment = 0; next; } + #extract the line range in the file after the patch is applied if ($line=~/[EMAIL PROTECTED]@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? [EMAIL PROTECTED]@/) { $is_patch = 1; -apw -- 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/
Re: bug in checkpath.pl
On Wed, Nov 28, 2007 at 12:07:38PM +0100, Holger Schurig wrote: I have a case where scripts/checkpatch.pl returns a false error. First, here is the code: static int lbs_scan_add_rates_tlv(u8 *tlv) { int i; struct mrvlietypes_ratesparamset *rate_tlv = (struct mrvlietypes_ratesparamset *) tlv; rate_tlv-header.type = cpu_to_le16(TLV_TYPE_RATES); tlv += sizeof(rate_tlv-header); for (i = 0; i MAX_RATES; i++) { *tlv = lbs_bg_rates[i]; if (*tlv == 0) break; if (*tlv == 0x02 || *tlv == 0x04 || *tlv == 0x0b || *tlv == 0x16) *tlv |= 0x80; tlv++; } rate_tlv-header.len = i; return sizeof(rate_tlv-header) + i; } And here the error from checkpatch.pl: ERROR: need consistent spacing around '*' (ctx:WxV) #553: FILE: drivers/net/wireless/libertas/scan.c:438: + *tlv |= 0x80; This error seems wrong, tlv is a pointer to some u8 value (a.k.a. unsigned char), and it is very well allowed to operate on it via *variablename |= 0x80; Yes that would be wrong. I believe that this is fixed in the latest versions. 0.12 should have the fix for this, and it seems to work in the latest development snapshot. Can you try the 0.12 and next from the URL below for me. http://www.kernel.org/pub/linux/kernel/people/apw/checkpatch/ -apw - 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/
Re: [PATCH] checkpatch: Print filenames of patches
On Sun, Nov 18, 2007 at 11:03:47AM +0100, Geert Uytterhoeven wrote: checkpatch: Print filenames of patches instead of the very uninformative `Your patch'. Well this isn't quite enough as we often use this thing checking its stdin. Which leads to an even less useful '- has no obvious ...'. I guess a hybrid would be an improvement. Signed-off-by: Geert Uytterhoeven [EMAIL PROTECTED] --- This patch is not `checkpatch' clean :-) Although I shortened 2 lines, they're still longer than 80 characters... Heh, yeah its a pig getting those RE's into 80 chars. scripts/checkpatch.pl |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1408,10 +1408,10 @@ sub process { } } if ($clean == 1 $quiet == 0) { - print Your patch has no obvious style problems and is ready for submission.\n + print $filename has no obvious style problems and is ready for submission.\n } if ($clean == 0 $quiet == 0) { - print Your patch has style problems, please review. If any of these errors\n; + print $filename has style problems, please review. If any of these errors\n; print are false positives report them to the maintainer, see\n; print CHECKPATCH in MAINTAINERS.\n; } -apw - 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/
Re: mm snapshot broken-out-2007-11-20-01-45.tar.gz -- powerpc panic
I have one powerpc machine which managed to compile this snapshot! It paniced on boot as below, might be nfs so copied them. General results are popping out on TKO. -apw Freeing initrd memory: 1224k freed Installing knfsd (copyright (C) 1996 [EMAIL PROTECTED]). Unable to handle kernel paging request for data at address 0x0050 Faulting instruction address: 0xc0113b64 Oops: Kernel access of bad area, sig: 11 [#1] SMP NR_CPUS=32 NUMA pSeries Modules linked in: NIP: c0113b64 LR: c0113b44 CTR: REGS: C0077E0679D0 TRAP: 0300 Not tainted (2.6.24-rc3-mm1-autokern1) MSR: 80009032 EE,ME,IR,DR CR: 24004044 XER: 2000 DAR: 0050, DSISR: 4000 TASK = C0077E062000[1] 'swapper' THREAD: C0077E064000 CPU: 0 GPR00: c0077e067c50 c06c5650 0001 GPR04: c0077e625bdc 0005 c0501ad4 c0731d18 GPR08: c0077e625b40 0001 GPR12: 24004044 c05fd000 GPR16: GPR20: 43a0 0430 03fb94a8 00132000 GPR24: 03fb9718 c05b6e70 GPR28: 0005 c0077e625b40 c0652e48 c0077e625c50 NIP [c0113b64] .remove_proc_entry+0xac/0x234 LR [c0113b44] .remove_proc_entry+0x8c/0x234 Call Trace: [c0077e067c50] [c0113b44] .remove_proc_entry+0x8c/0x234 (unreliable) [c0077e067d10] [c048bf28] .cache_unregister+0x108/0x1b4 [c0077e067d90] [c01ca988] .nfsd_export_shutdown+0x50/0xa4 [c0077e067e10] [c05a712c] .init_nfsd+0x108/0x13c [c0077e067ea0] [c0582438] .kernel_init+0x224/0x3fc [c0077e067f90] [c0026204] .kernel_thread+0x4c/0x68 Instruction dump: e8bf e8810090 7f83e378 4bfffd25 2f83 419e0018 ebbf e81d0050 f81f 3800 f81d0050 e93f e8090050 3be90050 2fa0 409effc4 Kernel panic - not syncing: Attempted to kill init! - 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/
Re: checkpatch false ERROR: trailing statements should be on next line
On Tue, Dec 25, 2007 at 04:50:04PM -0500, Erez Zadok wrote: Using v2.6.24-rc6-125-g5356f66, code such as this: if (is_file)/* dirs can be unlinked but chdir'ed to */ err = -ESTALE; produces this false checkpatch error: foo.c:947: ERROR: trailing statements should be on next line I think comments such as the above should be allowed (plus a comment isn't a statement). It's often very useful to put them right on the affected line. Agreed. checkpatch 0.13 already ignores these as statements. A positive side effect of other changes in this area. Thanks for the report. -apw -- 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/
Re: [PATCH] checkpatch.pl: recognize the #elif preprocessor directive
On Tue, Jan 01, 2008 at 12:12:22PM +0200, Benny Halevy wrote: checkpatch.pl does not recognize #elif as a preprocessor directive causing it to print bogus errors for, e.g.: ERROR: need consistent spacing around '' (ctx:WxV) when the operator is not recognized as unary in this context. for example: void foo(void) { int x, y, z; void *p[1] = { #if defined(X) x #elif defined(Y) y #else z #endif }; } Signed-off-by: Benny Halevy [EMAIL PROTECTED] Yes, thanks; good catch. Commited this and added tests for it. Will be in 0.13 which is imminent. -apw -- 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/
Re: WARNING: do not add new typedefs - is that for real?
On Tue, Jan 01, 2008 at 06:15:46PM +0200, Boaz Harrosh wrote: I have this code: c_code /* * osd-r10 4.12.5 Data-In and Data-Out buffer offsets * byte offset = mantissa * (2^(exponent+8)) */ typedef __be32 osd_cdb_offset; osd_cdb_offset __osd_encode_offset(u64 offset, unsigned *padding, int min_shift, int max_shift); struct osd_attributes_list_mode { __be32 get_attr_desc_bytes; osd_cdb_offset get_attr_desc_offset; __be32 get_attr_alloc_length; osd_cdb_offset get_attr_offset; __be32 set_attr_bytes; osd_cdb_offset set_attr_offset; __be32 not_used; }; /c_code the osd_cdb_offset above is this special OSD-standard floating-point-like special type. It is of size 32 bit in special network order. What should I do then: __be32 __osd_encode_offset(u64 offset, unsigned *padding, int min_shift, int max_shift); But it is not a __be32. It is this special floating-point-like thingy!!!? How was __be32 defined with a #define???!! Come on guys, it is not checkpatch.pl place to complain about good language constructs that can be misused. This is the maintainers and reviewers job to say that a: typedef struct foo Foo; is bad practice and we don't like it, but it can not be left to a script. typedefs should be used where they should be used. It is checkpatch's role to point out things which are likely to be wrong. There will always be exceptions. Lines whihc are much more readable if they spill over 80 characters, typedefs which do make sense. atomic_t's for example. This may well be a valid use of them. Note that this is mentioned as a WARNING not an ERROR. As is stated in the patch submission notes, you are meant to be comfortable with everything which checkpatch is still reporting. checkpatch is a style _guide_, not the be all and end all. It is meant to carry a preferred style to try and maintain some consistency kernel wide. -apw -- 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/
Re: [PATCH] teach checkpatch.pl about list_for_each
On Sun, Dec 02, 2007 at 01:03:35PM +0100, Christer Weinigel wrote: Hi Andy, you seem to be the last person messing around with checkpatch.pl so I'm addressing this to you. :-) checkpatch complains about the following: WARNING: no space between function name and open parenthesis '(' #520: FILE: drivers/spi/spi_s3c24xx_dma.c:478: + list_for_each_entry (transfer, message-transfers, transfer_list) { which I think is a bit bogus since it actually is a for statement in disguise. The following patch adds list_for_each to the list of things that look like functions that it shouldn't complain about. We have had some stabs at changing this, but no consensus was reached on whether it was a for or a function. My memory is of there being slightly more without a space tenders than the other and so it has not been changed. This thread also seems so far to have not really generated a concensus. So I would tend to leave things as they are. A third option might be to accept either on *for_each* constructs. That might tend to lead to divergance. Difficult. However, also see my later comments on style guide. By the way, what is the consensus on lines over 80 characters? checkpatch complains about the following: WARNING: line over 80 characters #762: FILE: drivers/spi/spi_s3c24xx_dma.c:720: + printk(KERN_INFO S3C24xx SPI DMA driver (c) 2007 Nordnav Technologies AB\n); I can of course break this into: printk(KERN_INFO S3C24xx SPI DMA driver (c) 2007 Nordnav Technologies AB\n); but in my opinion that becomes more even unreadable. Would it be possible to add a special case so that checkpatch ignores long strings that go beyond 80 characters? Do you think it is a good idea? I think you are miss-understanding the point of checkpatch here. It has flagged this line as suspect. You are happy it is better as it is. You should therefore leave it as it is as you are happy to justify that checkpatch failure. Checkpatch is a style guide, if it complains you should think about its complaint and act as you see fit in response. Sometimes you will ignore it, that is fine. -apw -- 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/
Re: Make checkpatch.pl's quiet option not print the summary on no errors
On Thu, Jan 03, 2008 at 01:54:42AM +0100, Arjan van de Ven wrote: Subject: Make checkpatch.pl's quiet option not print the summary on no errors From: Arjan van de Ven [EMAIL PROTECTED] CC: [EMAIL PROTECTED] Right now, in quiet mode, checkpatch.pl still prints a summary line even if the patch is 100% clean. IMO, quiet mode should mean no output if clean, the patch below makes that so. (This also makes the quilt integration on my system work nicer :) Signed-off-by: Arjan van de Ven [EMAIL PROTECTED] I think that sounds reasonable. Should people want the summary regardless it makes more sense to provice an option for that, perhaps a doubling of the --summary flag. Will be in 0.13. -apw -- 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/
Re: [PATCH v2] bitops kernel-doc: inline instead of macro
On Thu, Oct 25, 2007 at 01:48:14PM -0700, Andrew Morton wrote: Andy, I thought we were going to whine about __inline__ and __inline, too? Hmmm, I don't remember that coming up, but I'll add it to the todo. I am assuming plain 'inline' is preferred over both of these -- yell if you meant something else. -apw - 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/
Re: [PATCH v2] bitops kernel-doc: inline instead of macro
On Sat, Oct 27, 2007 at 11:43:28AM +0100, Andy Whitcroft wrote: On Thu, Oct 25, 2007 at 01:48:14PM -0700, Andrew Morton wrote: Andy, I thought we were going to whine about __inline__ and __inline, too? Hmmm, I don't remember that coming up, but I'll add it to the todo. I am assuming plain 'inline' is preferred over both of these -- yell if you meant something else. Ok, this will be in the next release and is in -next. WARNING: plain inline is preferred over __inline #4: FILE: tmp/testset-28768.c:1: +static __inline int foo(void) -apw - 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/
Re: [stable] 2.6.23 boot failures on x86-64.
On Mon, Oct 29, 2007 at 11:37:40AM -0700, Linus Torvalds wrote: On Mon, 29 Oct 2007, Greg KH wrote: I'll be glad to revert it in -stable, if it's also reverted in Linus's tree first :) We've had some changes since 2.6.23, and afaik, the alloc_bootmem_high_node() code is alreadt effectively dead there. It's only called if CONFIG_SPARSEMEM_VMEMMAP is *not* enabled, and I *think* we enable it by force on x86-64 these days. CONFIG_SPARSEMEM_VMEMMAP is the default when SPARSEMEM is enabled on x86_64. The overall default remains DISCONTIGMEM, mainly as a safety measure while the i386/x86_64 = x86 merge stablises. But yes this code is only used when SPARSEMEM is enabled but VMEMMAP is not. So it is effectivly redundant. More people added to Cc, just to clarify whether I'm just confused. Andy, Christoph, Mel: commit 2e1c49db4c640b35df13889b86b9d62215ade4b6 aka x86_64: allocate sparsemem memmap above 4G is the one that causes the failures, just fyi. That patch seems to have a laudable goal of trying to push the memory which backs the sparsemem memmap out to non-dma memory. I would have expected that call to actually succeed as the bootmem allocator seems to try at the goal which would likely be outside the node on a small machine, and then retry without a goal. Which is what the code without the goal does. Most illogical. Martin - it would be great if you could try out your failing machine with 2.6.24-rc1 (or a nightly snapshot or current git.. the more recent the better). But if I'm right, that commit should be reverted from 2.6.24 just because it's pointless (even if the bug itself is gone). And if I'm wrong, it should be reverted. So something like the appended would make sense regardless. Can I get a tested-by? And/or ack/nack's on my half-arsed theory above? This code is definatly only used when SPARSEMEM is enabled, and VMEMMAP is not which is not a combination we see on x86_64. Acked-by: Andy Whitcroft [EMAIL PROTECTED] Linus -- From: Linus Torvalds [EMAIL PROTECTED] Revert x86_64: allocate sparsemem memmap above 4G This reverts commit 2e1c49db4c640b35df13889b86b9d62215ade4b6, since testing in Fedora has shown it to cause boot failures, as per Dave Jones. Bisected down by Martin Ebourne. Cc: Dave Jones [EMAIL PROTECTED] Cc: Martin Ebourne [EMAIL PROTECTED] Cc: Zou Nan hai [EMAIL PROTECTED] Cc: Suresh Siddha [EMAIL PROTECTED] Cc: Andrew Morton [EMAIL PROTECTED] Signed-off-by: Linus Torvalds [EMAIL PROTECTED] diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c index 1e3862e..a7308b2 100644 --- a/arch/x86/mm/init_64.c +++ b/arch/x86/mm/init_64.c @@ -728,12 +728,6 @@ int in_gate_area_no_task(unsigned long addr) return (addr = VSYSCALL_START) (addr VSYSCALL_END); } -void * __init alloc_bootmem_high_node(pg_data_t *pgdat, unsigned long size) -{ - return __alloc_bootmem_core(pgdat-bdata, size, - SMP_CACHE_BYTES, (4UL*1024*1024*1024), 0); -} - const char *arch_vma_name(struct vm_area_struct *vma) { if (vma-vm_mm vma-vm_start == (long)vma-vm_mm-context.vdso) diff --git a/include/linux/bootmem.h b/include/linux/bootmem.h index c83534e..0365ec9 100644 --- a/include/linux/bootmem.h +++ b/include/linux/bootmem.h @@ -59,7 +59,6 @@ extern void *__alloc_bootmem_core(struct bootmem_data *bdata, unsigned long align, unsigned long goal, unsigned long limit); -extern void *alloc_bootmem_high_node(pg_data_t *pgdat, unsigned long size); #ifndef CONFIG_HAVE_ARCH_BOOTMEM_NODE extern void reserve_bootmem(unsigned long addr, unsigned long size); diff --git a/mm/sparse.c b/mm/sparse.c index 08fb14f..e06f514 100644 --- a/mm/sparse.c +++ b/mm/sparse.c @@ -220,12 +220,6 @@ static int __meminit sparse_init_one_section(struct mem_section *ms, return 1; } -__attribute__((weak)) __init -void *alloc_bootmem_high_node(pg_data_t *pgdat, unsigned long size) -{ - return NULL; -} - static unsigned long usemap_size(void) { unsigned long size_bytes; @@ -267,11 +261,6 @@ struct page __init *sparse_mem_map_populate(unsigned long pnum, int nid) if (map) return map; - map = alloc_bootmem_high_node(NODE_DATA(nid), - sizeof(struct page) * PAGES_PER_SECTION); - if (map) - return map; - map = alloc_bootmem_node(NODE_DATA(nid), sizeof(struct page) * PAGES_PER_SECTION); return map; -apw - 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/
Re: checkpatch bug: space between left parenthesis and asterisk
On Tue, Oct 30, 2007 at 02:27:13PM -0500, Timur Tabi wrote: I'm running checkpatch.pl (dated 10/17), and it complains about this line: crc = __be32_to_cpu(* ((__be32 *) ((void *) firmware + calc_size))); the message I get is: ERROR: need space before that '*' (ctx:BxW) #721: FILE: arch/powerpc/sysdev/qe_lib/qe.c:527: + crc = __be32_to_cpu(* ((__be32 *) ((void *) firmware + calc_size))); Hmmm, i've looked back a couple of releases and this seems to get reported correctly. Removing this space is then silent. ERROR: no space after that '*' (ctx:BxW) #4: FILE: Z31.c:1: + crc = __be32_to_cpu(* ((__be32 *) ((void *) firmware + calc_size))); What version is checkpatch reported as? Also can I have the entire diff hunk this is from in case its being caused by the context, though that seems unlikely. Thanks. -apw - 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/
Re: [PATCH 5/6] fix typo in SubmittingPatches
On Tue, Oct 30, 2007 at 10:11:44PM -0700, Greg Kroah-Hartman wrote: From: Keiichi Kii [EMAIL PROTECTED] Fix typo. Signed-off-by: Keiichi Kii [EMAIL PROTECTED] Cc: Andy Whitcroft [EMAIL PROTECTED] Cc: Randy Dunlap [EMAIL PROTECTED] Signed-off-by: Greg Kroah-Hartman [EMAIL PROTECTED] Acked-by: Andy Whitcroft [EMAIL PROTECTED] --- Documentation/SubmittingPatches |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index a30dd44..681e2b3 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -464,8 +464,8 @@ section Linus Computer Science 101. Nuff said. If your code deviates too much from this, it is likely to be rejected without further review, and without comment. -Once significant exception is when moving code from one file to -another in this case you should not modify the moved code at all in +One significant exception is when moving code from one file to +another -- in this case you should not modify the moved code at all in the same patch which moves it. This clearly delineates the act of moving the code and your changes. This greatly aids review of the actual differences and allows tools to better track the history of -apw - 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/
s390x: getting ipv6 bugs on mainline since 2.6.23-git3
Seems we are getting some kind of bug out of our s390x partition (lnxabat1) when booting latest mainline releases, specifically since 2.6.23-git3. Kernel BUG at 0002 Ýverbose debug info unavailable¨ illegal operation: 0001 Ý#1¨ Modules linked in: dm_mod sit tunnel4 ipv6 qeth ccwgroup qdio dasd_fba_mod dasd_ eckd_mod dasd_mod CPU:0Not tainted Process ip (pid: 2614, task: 3f42f680, ksp: 3cf2feb8) Krnl PSW : 070420018000 0002 (0x2) R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:0 CC:2 PM:0 EA:3 Krnl GPRS: 3e6d5d00 3e01f000 86dd 3cdac144 0010 3e01f000 3d97e3c0 000100eaaa00 3cdac100 3e6d5d00 0001018fb1d0 00286da8 3cf2f5e8 Krnl Code:0002: unknown 0004: unknown 0006: unknown 0008: unknown 000a: unknown 000c: unknown 000e: unknown 0010: unknown Call Trace: (Ý00286d60¨ neigh_connected_output+0x68/0xfc) Ý0001018cc7fe¨ ip6_output2+0x416/0x4ac Ýipv6¨ Ý0001018ceaa8¨ ip6_output+0xfc4/0xfd4 Ýipv6¨ Ý0001018dfe40¨ __ndisc_send+0x52c/0x6f0 Ýipv6¨ Ý0001018e0218¨ ndisc_send_rs+0x4c/0x5c Ýipv6¨ Ý0001018d40e6¨ addrconf_dad_completed+0xa2/0xf0 Ýipv6¨ Ý0001018d4880¨ addrconf_dad_start+0xa0/0x124 Ýipv6¨ Ý0001018d496a¨ addrconf_add_linklocal+0x66/0x98 Ýipv6¨ Ý0001018d7a3c¨ addrconf_notify+0x6cc/0x898 Ýipv6¨ Ý0004ad7c¨ notifier_call_chain+0x50/0x8c Ý0004b0b6¨ __raw_notifier_call_chain+0x1a/0x28 Ý0004b0e6¨ raw_notifier_call_chain+0x22/0x30 Ý0027f014¨ call_netdevice_notifiers+0x28/0x38 Ý00281732¨ dev_open+0xba/0xd0 Ý00281cce¨ dev_change_flags+0xbe/0x198 Ý002d681c¨ devinet_ioctl+0x2c8/0x6c0 Ý0027138a¨ sock_ioctl+0x26e/0x2a0 Ý000ab1ce¨ do_ioctl+0x4a/0xac Ý000ab5f2¨ vfs_ioctl+0x3c2/0x3d8 Ý000ab676¨ sys_ioctl+0x6e/0x94 Ý000215c6¨ sysc_noemu+0x10/0x16 Ý021201ca¨ 0x21201ca 0Kernel panic - not syncing: Fatal exception in interrupt 01: HCPGSP2629I The virtual machine is placed in CP mode due to a SIGP stop from CPU 00. 02: HCPGSP2629I The virtual machine is placed in CP mode due to a SIGP stop from CPU 00. 03: HCPGSP2629I The virtual machine is placed in CP mode due to a SIGP stop from CPU 00. 00: HCPGIR450W CP entered; disabled wait PSW 00020001 8000 00015FDE -apw - 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/
Re: [PATCH][NET] gianfar: fix obviously wrong #ifdef CONFIG_GFAR_NAPI placement
The check then is to see if a non {}'d block has no statements in it if the ifdef is null. Hmmm. May be possible. Will think on it. if (err) +#ifdef CONFIG_GFAR_NAPI napi_disable(priv-napi); +#endif -apw - 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/
Re: latest checkpatch
On Thu, Oct 18, 2007 at 01:13:52PM +0200, Ingo Molnar wrote: latest checkpatch.pl works really well on sched.c. there's only one problem left, this bogus false positive warning reappeared: WARNING: braces {} are not necessary for single statement blocks #5710: FILE: sched.c:5710: + if (parent-groups == parent-groups-next) { + pflags = ~(SD_LOAD_BALANCE | + SD_BALANCE_NEWIDLE | + SD_BALANCE_FORK | + SD_BALANCE_EXEC | + SD_SHARE_CPUPOWER | + SD_SHARE_PKG_RESOURCES); + } (there's another place in sched.c that trips this up too.) It actually never went away, some of the wronger reports went away such as counting a commented statement as a single statement. The check for length didn't make the cut for 0.11, as I was still thinking about whether we wanted a subjective check on statements over and above the real check for lines. i think it has been pointed out numerous times that it is perfectly fine to use curly braces for multi-line single-statement blocks. That includes simple cases like this too: if (x) { /* do y() */ y(); } Yes and the comment in there actually counts as a statement for counting statement purposes. The plan is to move to counting lines and only winge on exactly one line. I have half a mind to make a subjective check on statements and a full check on lines. But probabally it will just move to lines. it's perfectly legitimate, in fact more robust. So if checkpatch.pl wants to make any noise about such constructs it should warn about the _lack_ of curly braces in every multi-line condition block _except_ the only safe single-line statement: if (x) y(); Indeed. We should probabally do more on the indentation checks in general. The current direct check for: if (foo); bar(); Could probabally be generalised to look for this kind of error: if (foo) bar(); baz(); one(); -apw - 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/
Re: latest checkpatch
On Thu, Oct 18, 2007 at 08:25:21PM +0100, Andy Whitcroft wrote: On Thu, Oct 18, 2007 at 01:13:52PM +0200, Ingo Molnar wrote: latest checkpatch.pl works really well on sched.c. there's only one problem left, this bogus false positive warning reappeared: WARNING: braces {} are not necessary for single statement blocks #5710: FILE: sched.c:5710: + if (parent-groups == parent-groups-next) { + pflags = ~(SD_LOAD_BALANCE | + SD_BALANCE_NEWIDLE | + SD_BALANCE_FORK | + SD_BALANCE_EXEC | + SD_SHARE_CPUPOWER | + SD_SHARE_PKG_RESOURCES); + } (there's another place in sched.c that trips this up too.) Ok, checkpatch.pl-next should now no longer trip up on this statement. -apw - 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/
Re: latest checkpatch
On Thu, Oct 18, 2007 at 10:51:47PM +0200, Ingo Molnar wrote: * Avi Kivity [EMAIL PROTECTED] wrote: if (foo) bar(); baz(); one(); detecting that would be awesome - it's often the sign of a real bug because the intent is often to have bar() and baz() in the conditional block. This is more useful operating on an entire file, so the script can see all the context. there's checkpatch --file for complete files, so it can see the full context if the user passes it in. Indeed so. checkpatch uses all the context it can when making a decision. Often 3 lines carries enough history to figure this out. -apw - 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/
Re: [BUG] 2.6.23-git18 Kernel oops in sg helpers
On Tue, Oct 23, 2007 at 08:44:20PM +0200, Jens Axboe wrote: On Tue, Oct 23 2007, Kamalesh Babulal wrote: Hi, Kernel oops is triggered while running fsx-linux test, followed by cpu softlock over the AMD box Unable to handle kernel NULL pointer dereference at 0018 RIP: [8021f2f6] gart_map_sg+0x26c/0x406 PGD 10185b067 PUD 10075b067 PMD 0 Oops: 0002 [1] SMP CPU 3 Modules linked in: Pid: 18676, comm: fsx-linux Not tainted 2.6.23-git18-autokern1 #1 RIP: 0010:[8021f2f6] [8021f2f6] gart_map_sg+0x26c/0x406 RSP: :810181edf948 EFLAGS: 00010002 Can you check where gart_map_sg+0x26c is at? Make sure you have CONFIG_DEBUG_INFO defined, then do: $ gdb vmlinux $ l *gart_map_sg+0x26c Ok, this problem still seems to be about in 2.6.24-rc1. Here is the gdb output from that version, the panic (also below) seems the same: (gdb) l *gart_map_sg+0x26c 0x8022011e is in gart_map_sg (arch/x86/kernel/pci-gart_64.c:433). 428 goto error; 429 out++; 430 flush_gart(); 431 if (out nents) { 432 sgmap = sg_next(sgmap); 433 sgmap-dma_length = 0; 434 } 435 return out; 436 437 error: So it seems sg_next has returned 0. -apw elm3b6 login: -- 0:conmux-control -- time-stamp -- Oct/24/07 3:31:05 -- -- 0:conmux-control -- time-stamp -- Oct/24/07 3:46:40 -- Unable to handle kernel NULL pointer dereference at 0018 RIP: [8022011e] gart_map_sg+0x26c/0x406 PGD 101a8f067 PUD 10193c067 PMD 0 Oops: 0002 [1] SMP CPU 3 Modules linked in: Pid: 18339, comm: fsx-linux Not tainted 2.6.24-rc1-autokern1 #1 RIP: 0010:[8022011e] [8022011e] gart_map_sg+0x26c/0x406 RSP: :810181e03948 EFLAGS: 00010002 RAX: RBX: RCX: RDX: 0004 RSI: 0002 RDI: 8057918c RBP: 810181d0d820 R08: 0004 R09: 810181e038d4 R10: 00db R11: 804198f0 R12: 810181d0d840 R13: 0003 R14: 0001 R15: 0003 FS: () GS:81018071e380(0063) knlGS:f7fb9900 CS: 0010 DS: 002b ES: 002b CR0: 8005003b CR2: 0018 CR3: 000101a39000 CR4: 06e0 DR0: DR1: DR2: DR3: DR6: 0ff0 DR7: 0400 Process fsx-linux (pid: 18339, threadinfo 810181e02000, task 810181f2f560) Stack: 00030001 8101 810181d0d840 0001 00020002 810181d0d800 810100773870 810002905da0 8100022d6000 8101807082f0 810002905dd0 0200 Call Trace: [803ed20b] scsi_dma_map+0x3f/0x4e [803fda81] mptscsih_qcmd+0x1bc/0x4af [803e71ad] scsi_dispatch_cmd+0x1e7/0x277 [803ec758] scsi_request_fn+0x2df/0x369 [803514a8] cfq_insert_request+0x2a6/0x2ae [803471f5] elv_insert+0xcf/0x18a [8034aa31] __make_request+0x550/0x58b [8034ac89] generic_make_request+0x1bb/0x1f0 [8034ad92] submit_bio+0xd4/0xdf [802a15fb] dio_bio_submit+0x52/0x66 [802a230b] __blockdev_direct_IO+0x813/0xa1c [80261108] pagevec_lookup_tag+0x1a/0x21 [802df9b9] ext3_direct_IO+0x107/0x19e [802e03f0] ext3_get_block+0x0/0xe2 [8025a9ab] generic_file_direct_IO+0xcb/0x111 [8025b0af] generic_file_aio_read+0x86/0x160 [8027e9a2] do_sync_read+0xc8/0x10b [80298345] __mark_inode_dirty+0x29/0x17d [80246141] autoremove_wake_function+0x0/0x2e [80290ee7] notify_change+0x255/0x26a [802815d9] vfs_getattr+0x2b/0x2f [802816c1] vfs_fstat+0x33/0x3a [8027ea90] vfs_read+0xab/0x12e [8027ed94] sys_read+0x45/0x6e [802229d2] ia32_sysret+0x0/0xa Code: c7 41 18 00 00 00 00 8b 44 24 20 e9 7b 01 00 00 e8 27 f8 ff RIP [8022011e] gart_map_sg+0x26c/0x406 RSP 810181e03948 CR2: 0018 - 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/
2.6.23-git18: ext2_check_page: bad entry in directory
Seems that we are now getting strange errors mounting an ext2 root filesystem under 2.6.23-git18: EXT2-fs error (device sda1): ext2_check_page: bad entry in directory #2: rec_len is smaller than minimal - offset=0, inode=0, rec_len=0, name_len=0 This seems to be occuring consistently since 2.6.23-git18. I have been back and rerun a -git17 job to confirm its not visible at that level. This bug is expressing itself in 2.6.23-rc1 also. I am suspicious of this commit in the -git17 - -git18 block: commit 89910cccb8fec0c1140d33a743e72a712efd4f05 Author: Jan Kara [EMAIL PROTECTED] Date: Sun Oct 21 16:41:40 2007 -0700 ext2: avoid rec_len overflow with 64KB block size /me goes debug this a bit more. Jan? -apw - 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/
Re: 2.6.23-git18: ext2_check_page: bad entry in directory
On Wed, Oct 24, 2007 at 03:59:00PM +0200, Jan Kara wrote: Hello, On Wed 24-10-07 14:15:40, Andy Whitcroft wrote: Seems that we are now getting strange errors mounting an ext2 root filesystem under 2.6.23-git18: EXT2-fs error (device sda1): ext2_check_page: bad entry in directory #2: rec_len is smaller than minimal - offset=0, inode=0, rec_len=0, name_len=0 Hmm, zero length entry in a root directory. Not nice. This seems to be occuring consistently since 2.6.23-git18. I have been back and rerun a -git17 job to confirm its not visible at that level. This bug is expressing itself in 2.6.23-rc1 also. I am suspicious of this commit in the -git17 - -git18 block: commit 89910cccb8fec0c1140d33a743e72a712efd4f05 Author: Jan Kara [EMAIL PROTECTED] Date: Sun Oct 21 16:41:40 2007 -0700 ext2: avoid rec_len overflow with 64KB block size /me goes debug this a bit more. Hmm, I'd suspect that too ;) But I swear I actually tested the code ;) So what are you using the filesystem for (or do you get the error immediately when mounting)? How large is the filesystem? Its the root filesystem, so thats all she wrote. The kernel fails to mount it and the world ends. Its pretty small in now world terms. /dev/sda1 17G 10G 6.0G 63% / -apw - 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/
Re: 2.6.23-git18: ext2_check_page: bad entry in directory
On Wed, Oct 24, 2007 at 05:03:03PM +0200, Jan Kara wrote: e2image -r /dev/sda1 - | gzip -dc root-image.gz This thing is 27MB, I'll try and find some space to hold it and let you know where it is offlist. -apw - 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/
Re: bug in checkpatch (on pointers to typedefs?)
On Sun, Feb 10, 2008 at 03:33:02PM +0100, Marcin Slusarz wrote: Hi Checkpatch in current mainline outputs following errors: $ ./scripts/checkpatch.pl -q --file ./fs/udf/misc.c ERROR: need consistent spacing around '*' (ctx:WxV) #205: FILE: fs/udf/misc.c:205: + tag *tag_p; ^ $ ./scripts/checkpatch.pl -q --file ./fs/udf/unicode.c ERROR: need consistent spacing around '*' (ctx:WxV) #48: FILE: fs/udf/unicode.c:48: +int udf_build_ustr(struct ustr *dest, dstring *ptr, int size) ^ (...) I think the code is ok. Yep the code is clearly correct. Can I have the whole patch fragment you got these against so I can figure out why we are unable to detect these two as types, I would expect us to have done so. Also what version of checkpatch is this? There is a version string at the top. -apw -- 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/
Re: bug in checkpatch (on pointers to typedefs?)
On Mon, Feb 11, 2008 at 06:58:08PM +0200, Benny Halevy wrote: OK, but the return type doesn't have to be in the patched line, it could be in a synchronization line or even missing if the function has a long multi-line argument list. Ok, I guess thats fair criticism. Could you check out the current checkpatch-next (0.14-8-g3737366 or later -9, -10 etc), and see if that works. It seems to on the simple examples you sent me :). Thanks. -apw -- 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/
Re: bug in checkpatch (on pointers to typedefs?)
On Mon, Feb 11, 2008 at 06:05:48PM +0200, Benny Halevy wrote: I saw this too with checkpatch.pl version 0.12 It seems like checkpatch.pl knows only about types derived from @typeList by build_types. Example below... Benny $ cat EOF | scripts/checkpatch.pl - Signed-off-by: [EMAIL PROTECTED] --- diff a/f.c b/f.c --- a/f.c +++ b/f.c @@ -1,0 +1,2 @@ +foo(int a, my_uint32 *); +bar(int a, my_uint32_t *); But that isn't actually syntactically correct code is it? You have types as parameters like a function declaration, but no return type. So there is no hint to checkpatch that this is a function declaration and therefore the parameters are not expected to be types, nor are they checked as such. The following diff is clean on the latest version of checkpatch: Signed-off-by: [EMAIL PROTECTED] --- diff a/f.c b/f.c --- a/f.c +++ b/f.c @@ -1,0 +1,2 @@ +void foo(int a, my_uint32 *); +int bar(int a, my_uint32_t *); EOF Could you try out the version of checkpatch at the URL below on the real patch you are using to test, and let me know if it works. There are a number of improvements to type tracking in the face of ifdef's and the like. If it doesn't could I have the hunk which fails: http://www.kernel.org/pub/linux/kernel/people/apw/checkpatch/checkpatch.pl-next -apw -- 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/