Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Fri, Jun 27, 2014 at 04:59:14PM -0700, Andrew Morton wrote: > On Thu, 26 Jun 2014 16:32:15 -0700 "Luis R. Rodriguez" > wrote: > > > On Thu, Jun 26, 2014 at 4:20 PM, Andrew Morton > > wrote: > > > On Fri, 27 Jun 2014 01:16:30 +0200 "Luis R. Rodriguez" > > > wrote: > > > > > >> > > Another note -- since this option depends on SMP and !BASE_SMALL > > >> > > technically > > >> > > num_possible_cpus() won't ever return something smaller than or > > >> > > equal to 1 > > >> > > but because of the default values chosen the -1 on the compuation > > >> > > does affect > > >> > > whether or not this will trigger on > 64 CPUs or >= 64 CPUs, keeping > > >> > > the > > >> > > -1 means we require > 64 CPUs. > > >> > > > >> > hm, that sounds like more complexity. > > >> > > > >> > > This all can be changed however we like but the language and > > >> > > explained logic > > >> > > would just need to be changed. > > >> > > > >> > Let's start out simple. What's wrong with doing > > >> > > > >> > log buf len = max(__LOG_BUF_LEN, nr_possible_cpus * per-cpu log > > >> > buf len) > > >> > > >> Sure, you already took in the patch series though so how would you like > > >> to > > >> handle a respin, you just drop the last patch and we respin it? > > > > > > A fresh patch would suit. That's if you think it is a reasonable > > > approach - you've thought about this stuff more than I have! > > > > The way its implemented now makes more technical sense, in short it > > assumes the first boot (and CPU) gets the full default kernel ring > > buffer size, the extra size is for the gibberish that each extra CPU > > is expected to spew out in the worst case. What you propose makes the > > explanation simpler and easier to understand but sends the wrong > > message about exactly how the growth of the kernel ring buffer is > > expected scale with the addition of more CPUs. > > OK, it's finally starting to sink in. The model for the kernel-wide > printk output is "a great pile of CPU-independent stuff plus a certain > amount of per-cpu stuff". And the code at present attempts to follow > that model. Yes? Yup, exactly. > I'm rather internet-challenged at present - please let me take another look at > the patch on Monday. OK! Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Thu, 26 Jun 2014 16:32:15 -0700 "Luis R. Rodriguez" wrote: > On Thu, Jun 26, 2014 at 4:20 PM, Andrew Morton > wrote: > > On Fri, 27 Jun 2014 01:16:30 +0200 "Luis R. Rodriguez" > > wrote: > > > >> > > Another note -- since this option depends on SMP and !BASE_SMALL > >> > > technically > >> > > num_possible_cpus() won't ever return something smaller than or equal > >> > > to 1 > >> > > but because of the default values chosen the -1 on the compuation does > >> > > affect > >> > > whether or not this will trigger on > 64 CPUs or >= 64 CPUs, keeping > >> > > the > >> > > -1 means we require > 64 CPUs. > >> > > >> > hm, that sounds like more complexity. > >> > > >> > > This all can be changed however we like but the language and explained > >> > > logic > >> > > would just need to be changed. > >> > > >> > Let's start out simple. What's wrong with doing > >> > > >> > log buf len = max(__LOG_BUF_LEN, nr_possible_cpus * per-cpu log buf > >> > len) > >> > >> Sure, you already took in the patch series though so how would you like to > >> handle a respin, you just drop the last patch and we respin it? > > > > A fresh patch would suit. That's if you think it is a reasonable > > approach - you've thought about this stuff more than I have! > > The way its implemented now makes more technical sense, in short it > assumes the first boot (and CPU) gets the full default kernel ring > buffer size, the extra size is for the gibberish that each extra CPU > is expected to spew out in the worst case. What you propose makes the > explanation simpler and easier to understand but sends the wrong > message about exactly how the growth of the kernel ring buffer is > expected scale with the addition of more CPUs. OK, it's finally starting to sink in. The model for the kernel-wide printk output is "a great pile of CPU-independent stuff plus a certain amount of per-cpu stuff". And the code at present attempts to follow that model. Yes? I'm rather internet-challenged at present - please let me take another look at the patch on Monday. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Thu, 26 Jun 2014 16:32:15 -0700 Luis R. Rodriguez mcg...@suse.com wrote: On Thu, Jun 26, 2014 at 4:20 PM, Andrew Morton a...@linux-foundation.org wrote: On Fri, 27 Jun 2014 01:16:30 +0200 Luis R. Rodriguez mcg...@suse.com wrote: Another note -- since this option depends on SMP and !BASE_SMALL technically num_possible_cpus() won't ever return something smaller than or equal to 1 but because of the default values chosen the -1 on the compuation does affect whether or not this will trigger on 64 CPUs or = 64 CPUs, keeping the -1 means we require 64 CPUs. hm, that sounds like more complexity. This all can be changed however we like but the language and explained logic would just need to be changed. Let's start out simple. What's wrong with doing log buf len = max(__LOG_BUF_LEN, nr_possible_cpus * per-cpu log buf len) Sure, you already took in the patch series though so how would you like to handle a respin, you just drop the last patch and we respin it? A fresh patch would suit. That's if you think it is a reasonable approach - you've thought about this stuff more than I have! The way its implemented now makes more technical sense, in short it assumes the first boot (and CPU) gets the full default kernel ring buffer size, the extra size is for the gibberish that each extra CPU is expected to spew out in the worst case. What you propose makes the explanation simpler and easier to understand but sends the wrong message about exactly how the growth of the kernel ring buffer is expected scale with the addition of more CPUs. OK, it's finally starting to sink in. The model for the kernel-wide printk output is a great pile of CPU-independent stuff plus a certain amount of per-cpu stuff. And the code at present attempts to follow that model. Yes? I'm rather internet-challenged at present - please let me take another look at the patch on Monday. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Fri, Jun 27, 2014 at 04:59:14PM -0700, Andrew Morton wrote: On Thu, 26 Jun 2014 16:32:15 -0700 Luis R. Rodriguez mcg...@suse.com wrote: On Thu, Jun 26, 2014 at 4:20 PM, Andrew Morton a...@linux-foundation.org wrote: On Fri, 27 Jun 2014 01:16:30 +0200 Luis R. Rodriguez mcg...@suse.com wrote: Another note -- since this option depends on SMP and !BASE_SMALL technically num_possible_cpus() won't ever return something smaller than or equal to 1 but because of the default values chosen the -1 on the compuation does affect whether or not this will trigger on 64 CPUs or = 64 CPUs, keeping the -1 means we require 64 CPUs. hm, that sounds like more complexity. This all can be changed however we like but the language and explained logic would just need to be changed. Let's start out simple. What's wrong with doing log buf len = max(__LOG_BUF_LEN, nr_possible_cpus * per-cpu log buf len) Sure, you already took in the patch series though so how would you like to handle a respin, you just drop the last patch and we respin it? A fresh patch would suit. That's if you think it is a reasonable approach - you've thought about this stuff more than I have! The way its implemented now makes more technical sense, in short it assumes the first boot (and CPU) gets the full default kernel ring buffer size, the extra size is for the gibberish that each extra CPU is expected to spew out in the worst case. What you propose makes the explanation simpler and easier to understand but sends the wrong message about exactly how the growth of the kernel ring buffer is expected scale with the addition of more CPUs. OK, it's finally starting to sink in. The model for the kernel-wide printk output is a great pile of CPU-independent stuff plus a certain amount of per-cpu stuff. And the code at present attempts to follow that model. Yes? Yup, exactly. I'm rather internet-challenged at present - please let me take another look at the patch on Monday. OK! Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Thu, Jun 26, 2014 at 4:20 PM, Andrew Morton wrote: > On Fri, 27 Jun 2014 01:16:30 +0200 "Luis R. Rodriguez" > wrote: > >> > > Another note -- since this option depends on SMP and !BASE_SMALL >> > > technically >> > > num_possible_cpus() won't ever return something smaller than or equal to >> > > 1 >> > > but because of the default values chosen the -1 on the compuation does >> > > affect >> > > whether or not this will trigger on > 64 CPUs or >= 64 CPUs, keeping the >> > > -1 means we require > 64 CPUs. >> > >> > hm, that sounds like more complexity. >> > >> > > This all can be changed however we like but the language and explained >> > > logic >> > > would just need to be changed. >> > >> > Let's start out simple. What's wrong with doing >> > >> > log buf len = max(__LOG_BUF_LEN, nr_possible_cpus * per-cpu log buf >> > len) >> >> Sure, you already took in the patch series though so how would you like to >> handle a respin, you just drop the last patch and we respin it? > > A fresh patch would suit. That's if you think it is a reasonable > approach - you've thought about this stuff more than I have! The way its implemented now makes more technical sense, in short it assumes the first boot (and CPU) gets the full default kernel ring buffer size, the extra size is for the gibberish that each extra CPU is expected to spew out in the worst case. What you propose makes the explanation simpler and easier to understand but sends the wrong message about exactly how the growth of the kernel ring buffer is expected scale with the addition of more CPUs. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Fri, 27 Jun 2014 01:16:30 +0200 "Luis R. Rodriguez" wrote: > > > Another note -- since this option depends on SMP and !BASE_SMALL > > > technically > > > num_possible_cpus() won't ever return something smaller than or equal to 1 > > > but because of the default values chosen the -1 on the compuation does > > > affect > > > whether or not this will trigger on > 64 CPUs or >= 64 CPUs, keeping the > > > -1 means we require > 64 CPUs. > > > > hm, that sounds like more complexity. > > > > > This all can be changed however we like but the language and explained > > > logic > > > would just need to be changed. > > > > Let's start out simple. What's wrong with doing > > > > log buf len = max(__LOG_BUF_LEN, nr_possible_cpus * per-cpu log buf len) > > Sure, you already took in the patch series though so how would you like to > handle a respin, you just drop the last patch and we respin it? A fresh patch would suit. That's if you think it is a reasonable approach - you've thought about this stuff more than I have! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Thu, Jun 26, 2014 at 02:41:17PM -0700, Andrew Morton wrote: > On Tue, 24 Jun 2014 03:05:54 +0200 "Luis R. Rodriguez" > wrote: > > > > > Ah, because its cpu_extra, not total_cpu_space that is being > > > > computed, the goal was to see how much extra junk on the > > > > worst case a CPU might contribute. The __LOG_BUF_LEN is the > > > > default size, so we combine both. > > > > > > Well... why? Isn't it simpler and more direct to say "I want at least > > > 32k per CPU"? > > > > That's certainly another way to go about this, but the original motivation > > was trying to figure out the additional *extra* junk a CPU might spewed out, > > it set out with an assumption of a base start from the first CPU booting the > > system and that first CPU using the default kernel ring buffer size. The > > language in the patch describes the worst case extra CPU junk contributed, > > rather than a specific full split of the kernel ring buffer as that's > > typically > > how extra junk is spewered out to the ring bufer and the concern. In general > > on idle each CPU only contributes about only 1 to max 2 lines. The focus > > then > > is the worst case on contribution. > > I don't think I understood all that ;) Yeah if that made *you* squint a simpler approach would be better, regardless of how technically correct the above explanation may be. > > Another note -- since this option depends on SMP and !BASE_SMALL > > technically > > num_possible_cpus() won't ever return something smaller than or equal to 1 > > but because of the default values chosen the -1 on the compuation does > > affect > > whether or not this will trigger on > 64 CPUs or >= 64 CPUs, keeping the > > -1 means we require > 64 CPUs. > > hm, that sounds like more complexity. > > > This all can be changed however we like but the language and explained logic > > would just need to be changed. > > Let's start out simple. What's wrong with doing > > log buf len = max(__LOG_BUF_LEN, nr_possible_cpus * per-cpu log buf len) Sure, you already took in the patch series though so how would you like to handle a respin, you just drop the last patch and we respin it? Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Tue, 24 Jun 2014 03:05:54 +0200 "Luis R. Rodriguez" wrote: > > > Ah, because its cpu_extra, not total_cpu_space that is being > > > computed, the goal was to see how much extra junk on the > > > worst case a CPU might contribute. The __LOG_BUF_LEN is the > > > default size, so we combine both. > > > > Well... why? Isn't it simpler and more direct to say "I want at least > > 32k per CPU"? > > That's certainly another way to go about this, but the original motivation > was trying to figure out the additional *extra* junk a CPU might spewed out, > it set out with an assumption of a base start from the first CPU booting the > system and that first CPU using the default kernel ring buffer size. The > language in the patch describes the worst case extra CPU junk contributed, > rather than a specific full split of the kernel ring buffer as that's > typically > how extra junk is spewered out to the ring bufer and the concern. In general > on idle each CPU only contributes about only 1 to max 2 lines. The focus then > is the worst case on contribution. I don't think I understood all that ;) > Another note -- since this option depends on SMP and !BASE_SMALL technically > num_possible_cpus() won't ever return something smaller than or equal to 1 > but because of the default values chosen the -1 on the compuation does affect > whether or not this will trigger on > 64 CPUs or >= 64 CPUs, keeping the > -1 means we require > 64 CPUs. hm, that sounds like more complexity. > This all can be changed however we like but the language and explained logic > would just need to be changed. Let's start out simple. What's wrong with doing log buf len = max(__LOG_BUF_LEN, nr_possible_cpus * per-cpu log buf len) ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Tue, 24 Jun 2014 03:05:54 +0200 Luis R. Rodriguez mcg...@suse.com wrote: Ah, because its cpu_extra, not total_cpu_space that is being computed, the goal was to see how much extra junk on the worst case a CPU might contribute. The __LOG_BUF_LEN is the default size, so we combine both. Well... why? Isn't it simpler and more direct to say I want at least 32k per CPU? That's certainly another way to go about this, but the original motivation was trying to figure out the additional *extra* junk a CPU might spewed out, it set out with an assumption of a base start from the first CPU booting the system and that first CPU using the default kernel ring buffer size. The language in the patch describes the worst case extra CPU junk contributed, rather than a specific full split of the kernel ring buffer as that's typically how extra junk is spewered out to the ring bufer and the concern. In general on idle each CPU only contributes about only 1 to max 2 lines. The focus then is the worst case on contribution. I don't think I understood all that ;) Another note -- since this option depends on SMP and !BASE_SMALL technically num_possible_cpus() won't ever return something smaller than or equal to 1 but because of the default values chosen the -1 on the compuation does affect whether or not this will trigger on 64 CPUs or = 64 CPUs, keeping the -1 means we require 64 CPUs. hm, that sounds like more complexity. This all can be changed however we like but the language and explained logic would just need to be changed. Let's start out simple. What's wrong with doing log buf len = max(__LOG_BUF_LEN, nr_possible_cpus * per-cpu log buf len) ? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Thu, Jun 26, 2014 at 02:41:17PM -0700, Andrew Morton wrote: On Tue, 24 Jun 2014 03:05:54 +0200 Luis R. Rodriguez mcg...@suse.com wrote: Ah, because its cpu_extra, not total_cpu_space that is being computed, the goal was to see how much extra junk on the worst case a CPU might contribute. The __LOG_BUF_LEN is the default size, so we combine both. Well... why? Isn't it simpler and more direct to say I want at least 32k per CPU? That's certainly another way to go about this, but the original motivation was trying to figure out the additional *extra* junk a CPU might spewed out, it set out with an assumption of a base start from the first CPU booting the system and that first CPU using the default kernel ring buffer size. The language in the patch describes the worst case extra CPU junk contributed, rather than a specific full split of the kernel ring buffer as that's typically how extra junk is spewered out to the ring bufer and the concern. In general on idle each CPU only contributes about only 1 to max 2 lines. The focus then is the worst case on contribution. I don't think I understood all that ;) Yeah if that made *you* squint a simpler approach would be better, regardless of how technically correct the above explanation may be. Another note -- since this option depends on SMP and !BASE_SMALL technically num_possible_cpus() won't ever return something smaller than or equal to 1 but because of the default values chosen the -1 on the compuation does affect whether or not this will trigger on 64 CPUs or = 64 CPUs, keeping the -1 means we require 64 CPUs. hm, that sounds like more complexity. This all can be changed however we like but the language and explained logic would just need to be changed. Let's start out simple. What's wrong with doing log buf len = max(__LOG_BUF_LEN, nr_possible_cpus * per-cpu log buf len) Sure, you already took in the patch series though so how would you like to handle a respin, you just drop the last patch and we respin it? Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Fri, 27 Jun 2014 01:16:30 +0200 Luis R. Rodriguez mcg...@suse.com wrote: Another note -- since this option depends on SMP and !BASE_SMALL technically num_possible_cpus() won't ever return something smaller than or equal to 1 but because of the default values chosen the -1 on the compuation does affect whether or not this will trigger on 64 CPUs or = 64 CPUs, keeping the -1 means we require 64 CPUs. hm, that sounds like more complexity. This all can be changed however we like but the language and explained logic would just need to be changed. Let's start out simple. What's wrong with doing log buf len = max(__LOG_BUF_LEN, nr_possible_cpus * per-cpu log buf len) Sure, you already took in the patch series though so how would you like to handle a respin, you just drop the last patch and we respin it? A fresh patch would suit. That's if you think it is a reasonable approach - you've thought about this stuff more than I have! -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Thu, Jun 26, 2014 at 4:20 PM, Andrew Morton a...@linux-foundation.org wrote: On Fri, 27 Jun 2014 01:16:30 +0200 Luis R. Rodriguez mcg...@suse.com wrote: Another note -- since this option depends on SMP and !BASE_SMALL technically num_possible_cpus() won't ever return something smaller than or equal to 1 but because of the default values chosen the -1 on the compuation does affect whether or not this will trigger on 64 CPUs or = 64 CPUs, keeping the -1 means we require 64 CPUs. hm, that sounds like more complexity. This all can be changed however we like but the language and explained logic would just need to be changed. Let's start out simple. What's wrong with doing log buf len = max(__LOG_BUF_LEN, nr_possible_cpus * per-cpu log buf len) Sure, you already took in the patch series though so how would you like to handle a respin, you just drop the last patch and we respin it? A fresh patch would suit. That's if you think it is a reasonable approach - you've thought about this stuff more than I have! The way its implemented now makes more technical sense, in short it assumes the first boot (and CPU) gets the full default kernel ring buffer size, the extra size is for the gibberish that each extra CPU is expected to spew out in the worst case. What you propose makes the explanation simpler and easier to understand but sends the wrong message about exactly how the growth of the kernel ring buffer is expected scale with the addition of more CPUs. Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Mon, Jun 23, 2014 at 05:45:33PM -0700, Andrew Morton wrote: > On Tue, 24 Jun 2014 02:20:50 +0200 "Luis R. Rodriguez" > wrote: > > > On Mon, Jun 23, 2014 at 03:41:34PM -0700, Andrew Morton wrote: > > > On Wed, 18 Jun 2014 13:45:37 -0700 "Luis R. Rodriguez" > > > wrote: > > > > > > > ... > > > > > > > > If an increase is required the ring buffer is increased to > > > > + the next power of 2 that can fit both the minimum kernel ring > > > > buffer > > > > + (LOG_BUF_SHIFT) plus the additional worst case CPU > > > > contributions. > > > > > > > > ... > > > > > > > > + log_buf_len_update(cpu_extra + __LOG_BUF_LEN); > > > > +} > > > > > > I'd have expected > > > > > > total_cpu_space = minimum-per-cpu-len * nr_possible_cpus; > > > log_buf_len = max(__LOG_BUF_LEN, total_cpu_space) > > > > > > but here you added __LOG_BUF_LEN to total_cpu_space and I cannot work > > > out why. > > > . > > > > Ah, because its cpu_extra, not total_cpu_space that is being > > computed, the goal was to see how much extra junk on the > > worst case a CPU might contribute. The __LOG_BUF_LEN is the > > default size, so we combine both. > > Well... why? Isn't it simpler and more direct to say "I want at least > 32k per CPU"? That's certainly another way to go about this, but the original motivation was trying to figure out the additional *extra* junk a CPU might spewed out, it set out with an assumption of a base start from the first CPU booting the system and that first CPU using the default kernel ring buffer size. The language in the patch describes the worst case extra CPU junk contributed, rather than a specific full split of the kernel ring buffer as that's typically how extra junk is spewered out to the ring bufer and the concern. In general on idle each CPU only contributes about only 1 to max 2 lines. The focus then is the worst case on contribution. Another note -- since this option depends on SMP and !BASE_SMALL technically num_possible_cpus() won't ever return something smaller than or equal to 1 but because of the default values chosen the -1 on the compuation does affect whether or not this will trigger on > 64 CPUs or >= 64 CPUs, keeping the -1 means we require > 64 CPUs. This all can be changed however we like but the language and explained logic would just need to be changed. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Tue, 24 Jun 2014 02:20:50 +0200 "Luis R. Rodriguez" wrote: > On Mon, Jun 23, 2014 at 03:41:34PM -0700, Andrew Morton wrote: > > On Wed, 18 Jun 2014 13:45:37 -0700 "Luis R. Rodriguez" > > wrote: > > > > > ... > > > > > > If an increase is required the ring buffer is increased to > > > + the next power of 2 that can fit both the minimum kernel ring buffer > > > + (LOG_BUF_SHIFT) plus the additional worst case CPU contributions. > > > > > > ... > > > > > > + log_buf_len_update(cpu_extra + __LOG_BUF_LEN); > > > +} > > > > I'd have expected > > > > total_cpu_space = minimum-per-cpu-len * nr_possible_cpus; > > log_buf_len = max(__LOG_BUF_LEN, total_cpu_space) > > > > but here you added __LOG_BUF_LEN to total_cpu_space and I cannot work > > out why. > > . > > Ah, because its cpu_extra, not total_cpu_space that is being > computed, the goal was to see how much extra junk on the > worst case a CPU might contribute. The __LOG_BUF_LEN is the > default size, so we combine both. Well... why? Isn't it simpler and more direct to say "I want at least 32k per CPU"? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Mon, Jun 23, 2014 at 03:41:34PM -0700, Andrew Morton wrote: > On Wed, 18 Jun 2014 13:45:37 -0700 "Luis R. Rodriguez" > wrote: > > > ... > > > > If an increase is required the ring buffer is increased to > > + the next power of 2 that can fit both the minimum kernel ring buffer > > + (LOG_BUF_SHIFT) plus the additional worst case CPU contributions. > > > > ... > > > > + log_buf_len_update(cpu_extra + __LOG_BUF_LEN); > > +} > > I'd have expected > > total_cpu_space = minimum-per-cpu-len * nr_possible_cpus; > log_buf_len = max(__LOG_BUF_LEN, total_cpu_space) > > but here you added __LOG_BUF_LEN to total_cpu_space and I cannot work > out why. > . Ah, because its cpu_extra, not total_cpu_space that is being computed, the goal was to see how much extra junk on the worst case a CPU might contribute. The __LOG_BUF_LEN is the default size, so we combine both. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Wed, 18 Jun 2014 13:45:37 -0700 "Luis R. Rodriguez" wrote: > ... > > If an increase is required the ring buffer is increased to > + the next power of 2 that can fit both the minimum kernel ring buffer > + (LOG_BUF_SHIFT) plus the additional worst case CPU contributions. > > ... > > + log_buf_len_update(cpu_extra + __LOG_BUF_LEN); > +} I'd have expected total_cpu_space = minimum-per-cpu-len * nr_possible_cpus; log_buf_len = max(__LOG_BUF_LEN, total_cpu_space) but here you added __LOG_BUF_LEN to total_cpu_space and I cannot work out why. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Wed, 18 Jun 2014 13:45:37 -0700 Luis R. Rodriguez mcg...@do-not-panic.com wrote: ... If an increase is required the ring buffer is increased to + the next power of 2 that can fit both the minimum kernel ring buffer + (LOG_BUF_SHIFT) plus the additional worst case CPU contributions. ... + log_buf_len_update(cpu_extra + __LOG_BUF_LEN); +} I'd have expected total_cpu_space = minimum-per-cpu-len * nr_possible_cpus; log_buf_len = max(__LOG_BUF_LEN, total_cpu_space) but here you added __LOG_BUF_LEN to total_cpu_space and I cannot work out why. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Mon, Jun 23, 2014 at 03:41:34PM -0700, Andrew Morton wrote: On Wed, 18 Jun 2014 13:45:37 -0700 Luis R. Rodriguez mcg...@do-not-panic.com wrote: ... If an increase is required the ring buffer is increased to + the next power of 2 that can fit both the minimum kernel ring buffer + (LOG_BUF_SHIFT) plus the additional worst case CPU contributions. ... + log_buf_len_update(cpu_extra + __LOG_BUF_LEN); +} I'd have expected total_cpu_space = minimum-per-cpu-len * nr_possible_cpus; log_buf_len = max(__LOG_BUF_LEN, total_cpu_space) but here you added __LOG_BUF_LEN to total_cpu_space and I cannot work out why. . Ah, because its cpu_extra, not total_cpu_space that is being computed, the goal was to see how much extra junk on the worst case a CPU might contribute. The __LOG_BUF_LEN is the default size, so we combine both. Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Tue, 24 Jun 2014 02:20:50 +0200 Luis R. Rodriguez mcg...@suse.com wrote: On Mon, Jun 23, 2014 at 03:41:34PM -0700, Andrew Morton wrote: On Wed, 18 Jun 2014 13:45:37 -0700 Luis R. Rodriguez mcg...@do-not-panic.com wrote: ... If an increase is required the ring buffer is increased to + the next power of 2 that can fit both the minimum kernel ring buffer + (LOG_BUF_SHIFT) plus the additional worst case CPU contributions. ... + log_buf_len_update(cpu_extra + __LOG_BUF_LEN); +} I'd have expected total_cpu_space = minimum-per-cpu-len * nr_possible_cpus; log_buf_len = max(__LOG_BUF_LEN, total_cpu_space) but here you added __LOG_BUF_LEN to total_cpu_space and I cannot work out why. . Ah, because its cpu_extra, not total_cpu_space that is being computed, the goal was to see how much extra junk on the worst case a CPU might contribute. The __LOG_BUF_LEN is the default size, so we combine both. Well... why? Isn't it simpler and more direct to say I want at least 32k per CPU? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
On Mon, Jun 23, 2014 at 05:45:33PM -0700, Andrew Morton wrote: On Tue, 24 Jun 2014 02:20:50 +0200 Luis R. Rodriguez mcg...@suse.com wrote: On Mon, Jun 23, 2014 at 03:41:34PM -0700, Andrew Morton wrote: On Wed, 18 Jun 2014 13:45:37 -0700 Luis R. Rodriguez mcg...@do-not-panic.com wrote: ... If an increase is required the ring buffer is increased to + the next power of 2 that can fit both the minimum kernel ring buffer + (LOG_BUF_SHIFT) plus the additional worst case CPU contributions. ... + log_buf_len_update(cpu_extra + __LOG_BUF_LEN); +} I'd have expected total_cpu_space = minimum-per-cpu-len * nr_possible_cpus; log_buf_len = max(__LOG_BUF_LEN, total_cpu_space) but here you added __LOG_BUF_LEN to total_cpu_space and I cannot work out why. . Ah, because its cpu_extra, not total_cpu_space that is being computed, the goal was to see how much extra junk on the worst case a CPU might contribute. The __LOG_BUF_LEN is the default size, so we combine both. Well... why? Isn't it simpler and more direct to say I want at least 32k per CPU? That's certainly another way to go about this, but the original motivation was trying to figure out the additional *extra* junk a CPU might spewed out, it set out with an assumption of a base start from the first CPU booting the system and that first CPU using the default kernel ring buffer size. The language in the patch describes the worst case extra CPU junk contributed, rather than a specific full split of the kernel ring buffer as that's typically how extra junk is spewered out to the ring bufer and the concern. In general on idle each CPU only contributes about only 1 to max 2 lines. The focus then is the worst case on contribution. Another note -- since this option depends on SMP and !BASE_SMALL technically num_possible_cpus() won't ever return something smaller than or equal to 1 but because of the default values chosen the -1 on the compuation does affect whether or not this will trigger on 64 CPUs or = 64 CPUs, keeping the -1 means we require 64 CPUs. This all can be changed however we like but the language and explained logic would just need to be changed. Luis -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
From: "Luis R. Rodriguez" The default size of the ring buffer is too small for machines with a large amount of CPUs under heavy load. What ends up happening when debugging is the ring buffer overlaps and chews up old messages making debugging impossible unless the size is passed as a kernel parameter. An idle system upon boot up will on average spew out only about one or two extra lines but where this really matters is on heavy load and that will vary widely depending on the system and environment. There are mechanisms to help increase the kernel ring buffer for tracing through debugfs, and those interfaces even allow growing the kernel ring buffer per CPU. We also have a static value which can be passed upon boot. Relying on debugfs however is not ideal for production, and relying on the value passed upon bootup is can only used *after* an issue has creeped up. Instead of being reactive this adds a proactive measure which lets you scale the amount of contributions you'd expect to the kernel ring buffer under load by each CPU in the worst case scenario. We use num_possible_cpus() to avoid complexities which could be introduced by dynamically changing the ring buffer size at run time, num_possible_cpus() lets us use the upper limit on possible number of CPUs therefore avoiding having to deal with hotplugging CPUs on and off. This introduces the kernel configuration option LOG_CPU_MAX_BUF_SHIFT which is used to specify the maximum amount of contributions to the kernel ring buffer in the worst case before the kernel ring buffer flips over, the size is specified as a power of 2. The total amount of contributions made by each CPU must be greater than half of the default kernel ring buffer size (1 << LOG_BUF_SHIFT bytes) in order to trigger an increase upon bootup. The kernel ring buffer is increased to the next power of two that would fit the required minimum kernel ring buffer size plus the additional CPU contribution. For example if LOG_BUF_SHIFT is 18 (256 KB) you'd require at least 128 KB contributions by other CPUs in order to trigger an increase of the kernel ring buffer. With a LOG_CPU_BUF_SHIFT of 12 (4 KB) you'd require at least anything over > 64 possible CPUs to trigger an increase. If you had 128 possible CPUs the amount of minimum required kernel ring buffer bumps to: ((1 << 18) + ((128 - 1) * (1 << 12))) / 1024 = 764 KB Since we require the ring buffer to be a power of two the new required size would be 1024 KB. This CPU contributions are ignored when the "log_buf_len" kernel parameter is used as it forces the exact size of the ring buffer to an expected power of two value. Cc: Andrew Lunn Cc: Stephen Warren Cc: Michal Hocko Cc: Petr Mladek Cc: Andrew Morton Cc: Joe Perches Cc: Arun KS Cc: Kees Cook Cc: Davidlohr Bueso Cc: Chris Metcalf Cc: linux-kernel@vger.kernel.org Reviewed-by: Davidlohr Bueso Tested-by: Davidlohr Bueso Tested-by: Petr Mladek Signed-off-by: Petr Mladek Signed-off-by: Luis R. Rodriguez --- Documentation/kernel-parameters.txt | 8 -- init/Kconfig| 53 - kernel/printk/printk.c | 34 3 files changed, 92 insertions(+), 3 deletions(-) diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 6eaa9cd..4cf08f5 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -1685,8 +1685,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted. 7 (KERN_DEBUG) debug-level messages log_buf_len=n[KMG] Sets the size of the printk ring buffer, - in bytes. n must be a power of two. The default - size is set in the kernel config file. + in bytes. n must be a power of two and greater + than the minimal size. The minimal size is defined + by LOG_BUF_SHIFT kernel config parameter. There is + also CONFIG_LOG_CPU_MAX_BUF_SHIFT config parameter + that allows to increase the default size depending on + the number of CPUs. See init/Kconfig for more details. logo.nologo [FB] Disables display of the built-in Linux logo. This may be used to provide more screen space for diff --git a/init/Kconfig b/init/Kconfig index 9d76b99..573d3f6 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -807,7 +807,11 @@ config LOG_BUF_SHIFT range 12 21 default 17 help - Select kernel log buffer size as a power of 2. + Select the minimal kernel log buffer size as a power of 2. + The final size is affected by LOG_CPU_MAX_BUF_SHIFT config + parameter, see below. Any higher size also might be forced + by "log_buf_len" boot parameter. + Examples: 17 => 128 KB
[PATCH v8 4/4] printk: allow increasing the ring buffer depending on the number of CPUs
From: Luis R. Rodriguez mcg...@suse.com The default size of the ring buffer is too small for machines with a large amount of CPUs under heavy load. What ends up happening when debugging is the ring buffer overlaps and chews up old messages making debugging impossible unless the size is passed as a kernel parameter. An idle system upon boot up will on average spew out only about one or two extra lines but where this really matters is on heavy load and that will vary widely depending on the system and environment. There are mechanisms to help increase the kernel ring buffer for tracing through debugfs, and those interfaces even allow growing the kernel ring buffer per CPU. We also have a static value which can be passed upon boot. Relying on debugfs however is not ideal for production, and relying on the value passed upon bootup is can only used *after* an issue has creeped up. Instead of being reactive this adds a proactive measure which lets you scale the amount of contributions you'd expect to the kernel ring buffer under load by each CPU in the worst case scenario. We use num_possible_cpus() to avoid complexities which could be introduced by dynamically changing the ring buffer size at run time, num_possible_cpus() lets us use the upper limit on possible number of CPUs therefore avoiding having to deal with hotplugging CPUs on and off. This introduces the kernel configuration option LOG_CPU_MAX_BUF_SHIFT which is used to specify the maximum amount of contributions to the kernel ring buffer in the worst case before the kernel ring buffer flips over, the size is specified as a power of 2. The total amount of contributions made by each CPU must be greater than half of the default kernel ring buffer size (1 LOG_BUF_SHIFT bytes) in order to trigger an increase upon bootup. The kernel ring buffer is increased to the next power of two that would fit the required minimum kernel ring buffer size plus the additional CPU contribution. For example if LOG_BUF_SHIFT is 18 (256 KB) you'd require at least 128 KB contributions by other CPUs in order to trigger an increase of the kernel ring buffer. With a LOG_CPU_BUF_SHIFT of 12 (4 KB) you'd require at least anything over 64 possible CPUs to trigger an increase. If you had 128 possible CPUs the amount of minimum required kernel ring buffer bumps to: ((1 18) + ((128 - 1) * (1 12))) / 1024 = 764 KB Since we require the ring buffer to be a power of two the new required size would be 1024 KB. This CPU contributions are ignored when the log_buf_len kernel parameter is used as it forces the exact size of the ring buffer to an expected power of two value. Cc: Andrew Lunn and...@lunn.ch Cc: Stephen Warren swar...@wwwdotorg.org Cc: Michal Hocko mho...@suse.cz Cc: Petr Mladek pmla...@suse.cz Cc: Andrew Morton a...@linux-foundation.org Cc: Joe Perches j...@perches.com Cc: Arun KS arunks.li...@gmail.com Cc: Kees Cook keesc...@chromium.org Cc: Davidlohr Bueso davidl...@hp.com Cc: Chris Metcalf cmetc...@tilera.com Cc: linux-kernel@vger.kernel.org Reviewed-by: Davidlohr Bueso davidl...@hp.com Tested-by: Davidlohr Bueso davidl...@hp.com Tested-by: Petr Mladek pmla...@suse.cz Signed-off-by: Petr Mladek pmla...@suse.cz Signed-off-by: Luis R. Rodriguez mcg...@suse.com --- Documentation/kernel-parameters.txt | 8 -- init/Kconfig| 53 - kernel/printk/printk.c | 34 3 files changed, 92 insertions(+), 3 deletions(-) diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 6eaa9cd..4cf08f5 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -1685,8 +1685,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted. 7 (KERN_DEBUG) debug-level messages log_buf_len=n[KMG] Sets the size of the printk ring buffer, - in bytes. n must be a power of two. The default - size is set in the kernel config file. + in bytes. n must be a power of two and greater + than the minimal size. The minimal size is defined + by LOG_BUF_SHIFT kernel config parameter. There is + also CONFIG_LOG_CPU_MAX_BUF_SHIFT config parameter + that allows to increase the default size depending on + the number of CPUs. See init/Kconfig for more details. logo.nologo [FB] Disables display of the built-in Linux logo. This may be used to provide more screen space for diff --git a/init/Kconfig b/init/Kconfig index 9d76b99..573d3f6 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -807,7 +807,11 @@ config LOG_BUF_SHIFT range 12 21 default 17 help - Select kernel log buffer size as a power of 2. + Select the minimal kernel log buffer
