)
[4] https://lkml.kernel.org/r/87zh89kkek@jogness.linutronix.de
[5] https://lkml.kernel.org/r/87r1tmcfhf@jogness.linutronix.de
John Ogness (4):
crash: add VMCOREINFO macro to define offset in a struct declared by
typedef
printk: add lockless ringbuffer
Revert "printk: lock/u
-by: John Ogness
Acked-by: Sergey Senozhatsky
Reviewed-by: Petr Mladek
---
kernel/printk/printk.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index b71eaf5f5a86..1b41e1b98221 100644
--- a/kernel/printk/printk.c
+++ b
The existing macro VMCOREINFO_OFFSET() can't be used for structures
declared via typedef because "struct" is not part of type definition.
Create another macro for this purpose.
Signed-off-by: John Ogness
Acked-by: Baoquan He
Acked-by: Sergey Senozhatsky
Reviewed-by: Petr Mladek
--
function and made prb_first_seq() static. (The ringbuffer still needs
prb_first_seq() for itself.)
John Ogness
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 0b1337f4188c..fec71229169e 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -775,9 +775,9 @@ stati
ndle this. Let me try some
alternatives and post a proposed solution.
John Ogness
[0] https://lkml.kernel.org/r/20200213090757.ga36...@google.com
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
ment write_atomic() for 8250 UART
Some of the steps may be combined into a single series if the changes
are not too dramatic.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Fix prepared for for v5.
On 2020-07-08, kernel test robot wrote:
>>> kernel/printk/printk.c:1146:10: warning: format specifies type 'unsigned
>>> long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
> new_descs_size);
>
-by: John Ogness
---
kernel/printk/printk.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index b71eaf5f5a86..1b41e1b98221 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1981,9 +1981,8 @@ asmlinkage
://lkml.kernel.org/r/87k0ztbea9@jogness.linutronix.de
[11] https://lkml.kernel.org/r/20200625152523.GJ8444@alley
[12] https://lkml.kernel.org/r/87o8oznh2c@jogness.linutronix.de
John Ogness (4):
crash: add VMCOREINFO macro to define offset in a struct declared by
typedef
printk: add
The existing macro VMCOREINFO_OFFSET() can't be used for structures
declared via typedef because "struct" is not part of type definition.
Create another macro for this purpose.
Signed-off-by: John Ogness
Acked-by: Baoquan He
Acked-by: Sergey Senozhatsky
Reviewed-by: Petr Mladek
--
st using 32-bit dumps on 64-bit machines and vice versa. But
it should work.
John Ogness
[0] https://github.com/Linutronix/kexec-tools.git (printk branch)
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
ash, makedumpfile, vmcore-dmesg), others should be able to
implement the changes to their software without needing my help.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
og2() here.
new_log_buf_len can only be set within log_buf_len_update(), and it
is already doing exactly what you want:
if (size)
size = roundup_pow_of_two(size);
if (size > log_buf_len)
new_log_buf_len = (unsigned long)size;
I can switch to ilog2()
On 2020-06-25, Petr Mladek wrote:
> On Thu 2020-06-18 16:55:19, John Ogness wrote:
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -1609,11 +1633,15 @@ int do_syslog(int type, char __user *buf, int len,
>> int source)
>>
that
> might be checked easily. But it has many steps and it might hide some
> subtle catches like the one commented right above.
Your suggestions to simplify are good.
>> +static size_t get_record_text_size(struct printk_info *info,
>> + unsigned int l
bles.
>> log_buf_len = new_log_buf_len;
>> log_buf = new_log_buf;
>> new_log_buf_len = 0;
>> -free = __LOG_BUF_LEN - log_next_idx;
>> -memcpy(log_buf, __log_buf, __LOG_BUF_LEN);
>> +
>> +free = __LOG_BUF_LEN;
>> +prb_for_
*trunc_msg_len = 0;
>> +
>> /* disable the "dict" completely */
>> *dict_len = 0;
>
> The dictionary does not longer need to be removed at this point.
> It is stored in a separate buffer. It is ignored by prb_reserve()
> when there is not enough space
Hi Dave,
On 2020-06-25, Dave Young wrote:
> On 06/18/20 at 04:55pm, John Ogness wrote:
>> Here is a v3 for the first series to rework the printk
>> subsystem. The v2 and history are here [0]. This first series
>> only replaces the existing ringbuffer implementation. No
for the static array and 72 * (log_buf_len >> 5) bytes for the
dynamic array.
Signed-off-by: John Ogness
---
include/linux/kmsg_dump.h | 2 -
kernel/printk/printk.c| 944 --
2 files changed, 497 insertions(+), 449 deletions(-)
diff --git a/include
element to
allow readers and writers to locklessly synchronize access to the data.
Co-developed-by: Petr Mladek
Signed-off-by: John Ogness
---
kernel/printk/Makefile|1 +
kernel/printk/printk_ringbuffer.c | 1674 +
kernel/printk/printk_ringbuffer.h
to just have _DECLARE_PRINTKRB (and DECLARE_PRINTKRB) declare all
the variables as static.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
ed int"
- general: allow some lines to go beyond 80 characters
John Ogness
[0] https://lkml.kernel.org/r/20200501094010.17694-1-john.ogn...@linutronix.de
[1] https://lkml.kernel.org/r/87ftcd86d2@vostro.fn.ogness.net
[2] https://lkml.kernel.org/r/87v9ktcs3q@vostro.fn.ogness.net
The existing macro VMCOREINFO_OFFSET() can't be used for structures
declared via typedef because "struct" is not part of type definition.
Create another macro for this purpose.
Signed-off-by: John Ogness
---
include/linux/crash_core.h | 3 +++
1 file changed, 3 insertions(+)
d
desc_make_reusable
smp_mb
data_push_tail
read new data tail
data_push_head
smp_mb
write new data
read garbage new da
# dr->tail_lpos
> % prb_commit()
> %
> % id = blk->id
> % # read id for the freshly written data on CPU1
> % # and happily make them reusable
> % data_make_reusable()
>
> Sigh, sigh, sigh, there is a hugely misl
On 2020-06-10, John Ogness wrote:
>>> +static bool data_make_reusable(struct printk_ringbuffer *rb,
>>> + struct prb_data_ring *data_ring,
>>> + unsigned long lpos_begin,
>>> +
return false;
>> +desc_make_reusable(desc_ring, id);
>> +break;
>> +case desc_reusable:
>> +/*
>> + * This data block is invalid if the descriptor
>> + * does not point back to it.
>> + */
>> +if (blk_lpos->begin != lpos_begin)
>> +return false;
>> +break;
>> +}
>> +
>> +/* Advance @lpos_begin to the next data block. */
>> +lpos_begin = blk_lpos->next;
>> +}
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
priate anyway.
Also, this comment only talks about when a new value is seen, but not
about when the old value is seen. IMO it is seeing the old value that is
worthy of a comment since that is the only case with a data race.
In preparation for my next version I have added the following comment:
>> */
>> if (blk_lpos->begin != lpos_begin)
>> return false;
>> break;
No. Your example showed that it is not caught here.
> Here again the comments describe what the check does but not why.
> I would write something like:
>
> /*
>* The block might have already been
>* reused. Make sure that the descriptor really
>* points back to the checked lpos. It covers
>* both situations. Random data might point to
>* a valid descriptor just by chance. Or the block
>* has been already reused by another descriptor.
>*/
OK. I will expand the comments to something similar to this.
> So, I think that the lpos range check is still redundant. We might
> describe it as an extra paranoid check but I am not sure if it is
> worth it.
>
> But I would remove it and keep the code "simple" and design "clear".
> Well, I do not resist on it.
If we remove it, we end up back at your bug report. ;-)
John Ogness
[0] https://lkml.kernel.org/r/87ftecy343@linutronix.de
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
* Guarantee any data ring tail changes are stored before
* recycling the descriptor. Data ring tail changes can happen
* via desc_push_tail()->data_push_tail(). A full memory
* barrier is needed since another task may
l() might
> be done on different CPUs.
There needs to be a reason why the ordering of data tail pushing and
descriptor tail pushing is important.
> It is similar like the full barrier in data_push_tail() before changing
> data_ring->tail_lpos.
How so? That memory barrier exists t
. Tools could
greatly assist with this. It is my hope that my memory barrier
documentation can spark some ideas about how we could do this. (Doing
all this manually really sucks!)
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
ver, the desc_reserve:G
>> + * CPU (which performs the full memory barrier)
>> + * must have previously seen data_push_tail:D.
>> + */
>> + smp_rmb(); /* LMM(data_push_tail:A) */
&
d blk->id before CPU1 wrote new value
> because there is no read barrier betwen reading tail_lpos
> and blk->id here.
In your example, CPU1 is pushing the tail and then setting the block ID
for the _newly_ allocated block, that is located is _before_ the new
tail. If CPU0 sees the new tail already, it is still reading a valid
block ID, which is _not_ from the block that CPU1 is in the process of
writing.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
probably some other architectures), since the cmpxchg is always
> ordered on x86 and there exists no "relaxed" form of it.
ACK.
All three smp_mb()'s and both smp_wmb()'s sit directly next to
cmpxchg_relaxed() calls. Having explicit memory barriers was helpful for
identifying, proving, and test
+ */
> + smp_mb(); /* LMM(data_push_tail:C) */
> +
> + } while (!atomic_long_try_cmpxchg_relaxed(_ring->tail_lpos,
> + _lpos, next_lpos)); /* LMM(data_push_tail:D) */
> +
> + return true;
> +}
John Ogness
diff --git a/ker
Hi,
I discovered a bug while preparing my next series, which also made me
realize I had never tested the extended mode feature of netconsole. :-/
The only other user of extended output is /dev/kmsg, and it is doing it
correctly.
Explanation and patch below.
On 2020-05-01, John Ogness wrote
element to
allow readers and writers to locklessly synchronize access to the data.
Co-developed-by: Petr Mladek
Signed-off-by: John Ogness
---
kernel/printk/Makefile|1 +
kernel/printk/printk_ringbuffer.c | 1626 +
kernel/printk/printk_ringbuffer.h
for the static array and 72 * (log_buf_len >> 5) bytes for the
dynamic array.
Signed-off-by: John Ogness
---
include/linux/kmsg_dump.h | 2 -
kernel/printk/printk.c| 938 --
2 files changed, 492 insertions(+), 448 deletions(-)
diff --git a/include
() for this purpose)
- syslog_print_all(): fix missing break if copy_to_user() failed
- moved "messages dropped" printing to call_console_drivers()
John Ogness
[0] https://lkml.kernel.org/r/20200128161948.8524-1-john.ogn...@linutronix.de
[1] https://www.redhat.com/archives/crash-utility/2020-Apri
Some structs are not named and are only available via their typedef.
Add a VMCOREINFO macro to export field offsets for such structs.
Signed-off-by: John Ogness
---
include/linux/crash_core.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/crash_core.h b/include/linux
ot; symbol.
Signed-off-by: John Ogness
---
Makefile | 2 +-
dwarf_info.c | 36 +-
makedumpfile.c | 101 ++--
makedumpfile.h | 25 +++
printk.c | 177 +
5 files changed, 333 insertions(+), 8
expect you to take the patch as-is, but I hope it can
provide some positive ground work for moving forward.
John Ogness (1):
printk: add support for lockless ringbuffer
Makefile | 2 +-
dwarf_info.c | 36 +-
makedumpfile.c | 101 ++--
makedumpfi
a lot to export everything, but I don't have a problem
with it. If we decide to export everything (which I expect we will need
to do), then I would change my crash(8) implementation to also rely only
on the VMCOREINFO. I see no point in having some implementations using
debug data and other implementations using VMCOREINFO data, if
VMCOREINFO has everything that is needed.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
ing->head_lpos,
>>>> +_lpos, next_lpos));
>>>> +
>>>
>>> IMHO, we need smp_wmb() here to guarantee that others see the updated
>>> data_ring->head_lpos before we write anything into the data buffer.
>>>
>>> It would pair with a read barrier in data_make_reusable
>>> between reading tail_lpos and blk->id in data_make_reusable().
>>
>> Please explain why this pair is necessary. What is the scenario that
>> needs to be avoided?
>
> This code looks very similar to desc_reserve(). We are pushing
> tail/head and writing into to allocated space. Why do we need less
> barriers here?
The memory barrier pairing in desc_reserve() is necessary to order
descriptor reading with descriptor tail changes. For data we do not need
such a synchronization because data validity is guaranteed by the
descriptor states, not the data tail.
Note that above I talked about changing the cmpxchg_relaxed() in
data_push_tail() to cmpxchg() to deal with a data validity issue that
you discovered. That probably covers your gut feeling that we need
something here.
>>>> + blk = to_block(data_ring, begin_lpos);
>>>> + blk->id = id;
>>>> +
>>>> + if (DATA_WRAPS(data_ring, begin_lpos) !=
>>>> + DATA_WRAPS(data_ring, next_lpos)) {
>>>> + /* Wrapping data blocks store their data at the beginning. */
>>>> + blk = to_block(data_ring, 0);
>>>> + blk->id = id;
>>>> + }
>>>> +
>>>> + blk_lpos->begin = begin_lpos;
>>>> + blk_lpos->next = next_lpos;
>>>> +
>>>> + return >data[0];
>>>> +}
>
> Anyway, I am not sure how responsible I would be during
> the following days. My both hands are aching (Carpal tunnel
> syndrome or so) and it is getting worse. I have to visit
> a doctor. I hope that I will be able to work with some
> bandage but...
Please take care of yourself!
Here is the litmust test I talked about above, showing that smp_rb()
together with the smp_mb() before the head ID update does indeed avoid
the fail case.
-- begin desc-reserve.litmus --
C desc-reserve
(*
* Result: Never
*
* Make sure the head ID can never be pushed past the tail ID.
*)
{
dr_head_id = 0;
dr_tail_id = 1;
}
P0(int *dr_head_id, int *dr_tail_id)
{
int tail_id_next;
int id_prev_wrap;
int head_id;
int tail_id;
int r0;
int r1;
head_id = READ_ONCE(*dr_head_id);
id_prev_wrap = head_id + 1;
// Guarantee the head ID is read before reading the tail ID.
smp_rmb();
tail_id = READ_ONCE(*dr_tail_id);
if (id_prev_wrap == tail_id) {
// Make space for the new descriptor by advancing the tail.
tail_id_next = tail_id + 1;
r0 = cmpxchg_relaxed(dr_tail_id, tail_id, tail_id_next);
}
// Guarantee a new tail ID is stored before recycling the descriptor.
smp_mb();
r1 = cmpxchg_relaxed(dr_head_id, head_id, id_prev_wrap);
}
// identical to P0
P1(int *dr_head_id, int *dr_tail_id)
{
int tail_id_next;
int id_prev_wrap;
int head_id;
int tail_id;
int r0;
int r1;
head_id = READ_ONCE(*dr_head_id);
id_prev_wrap = head_id + 1;
// Guarantee the head ID is read before reading the tail ID.
smp_rmb();
tail_id = READ_ONCE(*dr_tail_id);
if (id_prev_wrap == tail_id) {
// Make space for the new descriptor by advancing the tail.
tail_id_next = tail_id + 1;
r0 = cmpxchg_relaxed(dr_tail_id, tail_id, tail_id_next);
}
// Guarantee a new tail ID is stored before recycling the descriptor.
smp_mb();
r1 = cmpxchg_relaxed(dr_head_id, head_id, id_prev_wrap);
}
// identical to P0
P2(int *dr_head_id, int *dr_tail_id)
{
int tail_id_next;
int id_prev_wrap;
int head_id;
int tail_id;
int r0;
int r1;
head_id = READ_ONCE(*dr_head_id);
id_prev_wrap = head_id + 1;
// Guarantee the head ID is read before reading the tail ID.
smp_rmb();
tail_id = READ_ONCE(*dr_tail_id);
if (id_prev_wrap == tail_id) {
// Make space for the new descriptor by advancing the tail.
tail_id_next = tail_id + 1;
r0 = cmpxchg_relaxed(dr_tail_id, tail_id, tail_id_next);
}
// Guarantee a new tail ID is stored before recycling the descriptor.
smp_mb();
r1 = cmpxchg_relaxed(dr_head_id, head_id, id_prev_wrap);
}
exists (dr_head_id=2 /\ dr_tail_id=2)
-- end desc-reserve.litmus --
$ herd7 -conf linux-kernel.cfg desc-reserve.litmus
Test desc-reserve Allowed
States 3
dr_head_id=1; dr_tail_id=2;
dr_head_id=2; dr_tail_id=3;
dr_head_id=3; dr_tail_id=4;
No
Witnesses
Positive: 0 Negative: 138
Condition exists (dr_head_id=2 /\ dr_tail_id=2)
Observation desc-reserve Never 0 138
Time desc-reserve 490.62
Hash=4198247b011ab3db1ac8ff48152bbb18
Note that if the smp_mb() is _not_ moved up (even with an added
smp_rmb() in desc_reserve()), the fail case will happen:
$ herd7 -conf linux-kernel.cfg desc-reserve-bad.litmus
Test desc-reserve Allowed
States 4
dr_head_id=1; dr_tail_id=2;
dr_head_id=2; dr_tail_id=2; < head overtakes tail!
dr_head_id=2; dr_tail_id=3;
dr_head_id=3; dr_tail_id=4;
Ok
Witnesses
Positive: 24 Negative: 162
Condition exists (dr_head_id=2 /\ dr_tail_id=2)
Observation desc-reserve Sometimes 24 162
Time desc-reserve 515.87
Hash=1e80a5d56c53a87355d8a34a850cb7f5
The smp_mb() happens too late. Moving it before pushing the head ID
fixes the problem.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
>> shared data. desc_read_committed() is called twice in prb_read() and
>> it is expected that both calls are using the same @id.
>
> It is not error prone. If "id" changes then "seq" will not match.
@id is
INVAL;
>
> if (d_state == desc_reusable)
> return -ENOENT;
I can use this refactoring.
>
> if (d_state != desc_committed)
> return -EINVAL;
I suppose you meant to remove this check and leave in the @blk_lpos
check instead. If we're trying to minimize lines of code, the @blk_lpos
check could be combined with the "== desc_reusable" check as well.
>
> return 0;
> }
Thanks.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
lease, rename it to make it clear that does only a check.
> For example, check_state_commited().
This function _does_ read. It is a helper function of prb_read() to
_read_ the descriptor. It is an extended version of desc_read() that
also performs various checks that the descriptor is committed.
I will update the function description to be more similar to desc_read()
so that it is obvious that it is "getting a copy of a specified
descriptor".
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
patterns like:
do {
} while (!try_cmpxchg_relaxed());
smp_mb();
or possibly:
smp_mb();
cmpxchg_relaxed(); /* no return value check */
> On Tue 2020-01-28 17:25:47, John Ogness wrote:
>> diff --git a/kernel/printk/printk_ringbuffer.c
>> b/kernel/pr
we remove the locks (and it
will disappear once we go to kthreads). But we aren't that far at this
stage and I'd like to keep the general logic somewhat close to the
current mainline implementation for now.
> I prefer to call wake_up_klogd() directly from log_output() or
> log_store() instead. It might lat
ers of record_print_text() for counting will now
call the new counting functions. IMHO it is a nice cleanup and also
removes the static printk_record structs for console and syslog.
Thanks.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lis
missing). I will get this
sorted out for v2. And I will provide some heavily hacked code for crash
and makedumpfile to show that the necessary symbols are there and it
works.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
On 2020-02-14, Petr Mladek wrote:
>> I oversaw that devkmsg_open() setup a printk_record and so I did not
>> see to add the extra NULL initialization of text_line_count. There
>> should be be an initializer function/macro to avoid this danger.
>>
>> John
uman-readable
text.) I am currently hacking the crash tool to see exactly what needs
to be made available in order to access all the data of the ringbuffer.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
e reader can either:
1. continue reading and see the jump
2. reopen the file descriptor, possibly having missed a ton more
messages due to reopening, and then start from the oldest available
message
With my series, #2 is no longer an option because the lost messages
could exist in a part of the ring
data-less records): I hacked the
ringbuffer code to inject data-less records at various times in order to
verify your report. And I stumbled upon a bug in the ringbuffer, which
can lead to an infinite loop in console_unlock(). The problem occurs at:
retry = prb_read_valid(prb, console_se
parately, it is enough just to check
for the 2nd case. For the 1st case, prb_first_seq() could be less than
r->info->seq if all the preceeding records have no data. But this just
means the whole set of records with missing data are skipped, which
matches existing behavior. (For example
rs, writers request buffers. This eliminates the
manual twiddling with the record struct and ensures that the struct is
always properly initialized.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
dictionaries, we might see their usage increase and start
to be as large as the messages themselves.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
cro to avoid this danger.
John Ogness
The quick fixup:
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index d0d24ee1d1f4..5ad67ff60cd9 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -883,6 +883,7 @@ static int devkmsg_open(struct inode *inode, struct file
*file)
On 2020-02-05, Sergey Senozhatsky wrote:
So there is a General protection fault. That's the type of a
problem that kills the boot for me as well (different backtrace,
tho).
>>>
>>> Do you have CONFIG_RELOCATABLE and CONFIG_RANDOMIZE_BASE (KASLR)
>>> enabled?
>>
>> Yes. These two
) that maps to a desc_ring
index followed by the dictionary string of the record.
Descriptor state information is the key element to allow readers and
writers to locklessly synchronize access to the data.
Co-developed-by: Petr Mladek
Signed-off-by: John Ogness
---
kernel/printk
^ ((log_buf_len - 6))) bytes for
the dynamic array.
Signed-off-by: John Ogness
---
include/linux/kmsg_dump.h | 2 -
kernel/printk/Makefile| 1 +
kernel/printk/printk.c| 836 +++---
3 files changed, 416 insertions(+), 423 deletions(-)
diff --git
are stored in a separate
buffer so that they cannot interfere with the human-readable buffer.
John Ogness
[0] https://lkml.kernel.org/r/20190212143003.48446-1-john.ogn...@linutronix.de
[1] https://lkml.kernel.org/r/20190607162349.18199-1-john.ogn...@linutronix.de
[2] https://lkml.kernel.org/r
umber of CMPXCHG: similar questions would
> apply to those other instances...)
LMM_TAG(data_push_tail:A) is the only CMPXCHG that requires its full
barrier. All others can be relaxed. I will make this change for the next
series.
Thanks for taking time for this.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
+ 1;
>> + * }
>> + */
>
> Will this loop ever end? :)
>
> pr_info() adds data to ringbuffer, which prb_read_valid() reads, so
> pr_info() can add more data, which prb_read_valid() will read, so
> pr_info()...
The sample code is assuming that @rb is not the same ring
committed()->desc_read()->memcpy(). The context which
> interrupts the reader recycles the descriptor and pushes new
> data. Suppose that reader was interrupted right after it copied
> ->info.seq and ->info.text_len. So the first desc_read_committed()
> will pass - we h
the state
variable. That's why the code is using state_var/state_val (SV) for the
actual data values, keeping it separate from desc_state/d_state for the
the state queries.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
will probably be the most interesting, due to memory barrier
testing.
Otherwise I will definitely be reaching out to you when we are ready to
perform actual printk testing with the newly agreed up semantics
(lockless, per-console printing threads, synchronous panic
consoles). Thanks for your hel
will change the data block ID size to "unsigned long". Then it is
really only an issue for 32-bit systems.
AFAICT, the only real issue is that the head can be pushed to the
descriptor index that the tail is pointing to. From there, the head can
be advanced beyond and the tail
On 2019-12-04, Petr Mladek wrote:
>> +} else if ((DATA_WRAPS(data_ring, blk_lpos->begin) + 1 ==
>> +DATA_WRAPS(data_ring, blk_lpos->next)) ||
>> + ((DATA_WRAPS(data_ring, blk_lpos->begin) ==
>> + DATA_WRAPS(data_ring, -1UL)) &&
>> +
er and cannot be explicitly created by
writers.
> Also reader API users might not expect to get a "valid" data-less
> record.
Readers will never see them. The reader API implementation skips over
data-less records.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
On 2019-12-03, Petr Mladek wrote:
>> Add the reader implementation for the new ringbuffer.
>>
>> Signed-off-by: John Ogness
>> ---
>> kernel/printk/printk_ringbuffer.c | 234 ++
>> kernel/printk/printk_ringbuffer.h | 12 +-
>&
case of error.
I need to go through the code again in detail and see how many locations
are affected by ABA. All the code was written with the assumption that
this type of ABA will not happen.
As you've stated, we could add minimal handling so that the ringbuffer
at least does not break or get stuck.
BTW: The same assumption is made for logical positions. There are 4
times as many of these (on 32-bit systems) but logical positions advance
much faster. I will review these as well.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Add the reader implementation for the new ringbuffer.
Signed-off-by: John Ogness
---
kernel/printk/printk_ringbuffer.c | 234 ++
kernel/printk/printk_ringbuffer.h | 12 +-
2 files changed, 245 insertions(+), 1 deletion(-)
diff --git a/kernel/printk
Add a new lockless ringbuffer implementation to be used for printk.
First only add support for writers. Reader support will be added in
a follow-up commit.
Signed-off-by: John Ogness
---
kernel/printk/printk_ringbuffer.c | 676 ++
kernel/printk/printk_ringbuffer.h
reading core misses only about 15% of the
total records.
John Ogness
[0] https://lkml.kernel.org/r/20190212143003.48446-1-john.ogn...@linutronix.de
[1] https://lkml.kernel.org/r/20190607162349.18199-1-john.ogn...@linutronix.de
[2] https://lkml.kernel.org/r/2019072701.11260-1-john.ogn
This module does some heavy write stress testing on the ringbuffer
with a reader that is checking for integrity.
Signed-off-by: John Ogness
---
kernel/printk/Makefile | 3 +
kernel/printk/test_prb.c | 347 +++
2 files changed, 350 insertions(+)
create
On 2019-08-16, Dave Young wrote:
> John, can you cc kexec list for your later series?
Sure.
> On 08/08/19 at 12:32am, John Ogness wrote:
>> This is a major change because the API (and underlying workings) of
>> the new ringbuffer are completely different than the previous
&g
101 - 179 of 179 matches
Mail list logo
