Message ID | 20150413012115.GB15225@corellia.local (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Sun, Apr 12, 2015 at 06:21:15PM -0700, Gregory Fong wrote: > On Sun, Apr 12, 2015 at 06:09:13PM -0700, Gregory Fong wrote: > > And now we see 83 slab_reclaimable + 846 slab_unreclaimable adds up > > correctly to the total of 929. > > > > The patch below will end up with the correct count. > > > > Sorry, messed up the patch formatting. Here it is fixed: So now the question is: do we fix this, or do we use the generic version? Given that the total number of slab pages can be easily deduced from the generic statistics, do we need to modify the generic version to print an additional line with this? It seems wasteful to do so, and just adds more noise to the kernel's debug output.
On Mon, Apr 13, 2015 at 10:56:45AM +0100, Russell King - ARM Linux wrote: > On Sun, Apr 12, 2015 at 06:21:15PM -0700, Gregory Fong wrote: > > On Sun, Apr 12, 2015 at 06:09:13PM -0700, Gregory Fong wrote: > > > And now we see 83 slab_reclaimable + 846 slab_unreclaimable adds up > > > correctly to the total of 929. > > > > > > The patch below will end up with the correct count. > > > > > > > Sorry, messed up the patch formatting. Here it is fixed: > > So now the question is: do we fix this, or do we use the generic version? > Given that the total number of slab pages can be easily deduced from the > generic statistics, do we need to modify the generic version to print an > additional line with this? Whatever ARM decides, I do not think the generic version needs to do a PFN walk to recaluate the SLAB statistics. The slab_reclaimable and slab_unreclaimable stats based on the vmstat counters is sufficient.
On Mon, Apr 13, 2015 at 11:04:26AM +0100, Mel Gorman wrote: > On Mon, Apr 13, 2015 at 10:56:45AM +0100, Russell King - ARM Linux wrote: > > On Sun, Apr 12, 2015 at 06:21:15PM -0700, Gregory Fong wrote: > > > On Sun, Apr 12, 2015 at 06:09:13PM -0700, Gregory Fong wrote: > > > > And now we see 83 slab_reclaimable + 846 slab_unreclaimable adds up > > > > correctly to the total of 929. > > > > > > > > The patch below will end up with the correct count. > > > > > > > > > > Sorry, messed up the patch formatting. Here it is fixed: > > > > So now the question is: do we fix this, or do we use the generic version? > > Given that the total number of slab pages can be easily deduced from the > > generic statistics, do we need to modify the generic version to print an > > additional line with this? > > Whatever ARM decides, I do not think the generic version needs to do > a PFN walk to recaluate the SLAB statistics. The slab_reclaimable and > slab_unreclaimable stats based on the vmstat counters is sufficient. Yes, I agree. My feeling is we just switch to the generic version and be done with it.
On Mon, Apr 13, 2015 at 3:05 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Mon, Apr 13, 2015 at 11:04:26AM +0100, Mel Gorman wrote: >> On Mon, Apr 13, 2015 at 10:56:45AM +0100, Russell King - ARM Linux wrote: >> > On Sun, Apr 12, 2015 at 06:21:15PM -0700, Gregory Fong wrote: >> > > On Sun, Apr 12, 2015 at 06:09:13PM -0700, Gregory Fong wrote: >> > > > And now we see 83 slab_reclaimable + 846 slab_unreclaimable adds up >> > > > correctly to the total of 929. >> > > > >> > > > The patch below will end up with the correct count. >> > > > >> > > >> > > Sorry, messed up the patch formatting. Here it is fixed: >> > >> > So now the question is: do we fix this, or do we use the generic version? >> > Given that the total number of slab pages can be easily deduced from the >> > generic statistics, do we need to modify the generic version to print an >> > additional line with this? >> >> Whatever ARM decides, I do not think the generic version needs to do >> a PFN walk to recaluate the SLAB statistics. The slab_reclaimable and >> slab_unreclaimable stats based on the vmstat counters is sufficient. > > Yes, I agree. My feeling is we just switch to the generic version and be > done with it. Agreed. This is really what I was hoping for in the first place, but didn't know before submitting the initial patch whether there was some arcane reason for the arm-specific show_mem. If someone like Yalin really wants the total slab pages, they can just add slab_unreclaimable and slab_reclaimable (btw, Yalin, all emails I've sent to you are bouncing, maybe you'll see this since it's going to the list). Thanks, Gregory
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c index 1609b02..8d606bb 100644 --- a/arch/arm/mm/init.c +++ b/arch/arm/mm/init.c @@ -117,7 +117,7 @@ void show_mem(unsigned int filter) else if (PageSwapCache(page)) cached++; else if (PageSlab(page)) - slab++; + slab += 1 << compound_order(page); else if (!page_count(page)) free++; else