Re: [PATCHv3 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
Pingfan Liu writes: > A bug is observed on pseries by taking the following steps on rhel: ^ RHEL I assume it happens on mainline too? > -1. drmgr -c mem -r -q 5 > -2. echo c > /proc/sysrq-trigger > > And then, the failure looks like: > kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/ > kdump: saving vmcore-dmesg.txt > kdump: saving vmcore-dmesg.txt complete > kdump: saving vmcore > Checking for memory holes : [ 0.0 %] / > Checking for memory holes : [100.0 %] | > Excluding unnecessary pages : [100.0 %] \ > Copying data : [ 0.3 %] - > eta: 38s[ 44.337636] hash-mmu: mm: Hashing failure ! > EA=0x7fffba40 access=0x8004 current=makedumpfile > [ 44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 > psize 2 pte=0xc0005504 > [ 44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 > access=0x8004 current=makedumpfile > [ 44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 > psize 2 pte=0xc0005504 > [ 44.337708] makedumpfile[469]: unhandled signal 7 at 7fffba40 nip > 7fffbbc4d7fc lr 00011356ca3c code 2 > [ 44.338548] Core dump to |/bin/false pipe failed > /lib/kdump-lib-initramfs.sh: line 98: 469 Bus error > $CORE_COLLECTOR /proc/vmcore > $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete > kdump: saving vmcore failed > > * Root cause * > After analyzing, it turns out that in the current implementation, > when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt updating as > the code __remove_memory() comes before drmem_update_dt(). > So in kdump kernel, when read_from_oldmem() resorts to > pSeries_lpar_hpte_insert() to install hpte, but fails with -2 due to > non-exist pfn. And finally, low_hash_fault() raise SIGBUS to process, as it > can be observed "Bus error" > > From a viewpoint of listener and publisher, the publisher notifies the > listener before data is ready. This introduces a problem where udev > launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before > updating. And in capture kernel, makedumpfile will access the memory based > on the stale dt info, and hit a SIGBUS error due to an un-existed lmb. > > * Fix * > In order to fix this issue, update dt before __remove_memory(), and > accordingly the same rule in hot-add path. > > This will introduce extra dt updating payload for each involved lmb when > hotplug. > But it should be fine since drmem_update_dt() is memory based operation and > hotplug is not a hot path. > > Signed-off-by: Pingfan Liu > Cc: Michael Ellerman > Cc: Hari Bathini > Cc: Nathan Lynch > To: linuxppc-...@lists.ozlabs.org > Cc: kexec@lists.infradead.org > --- > v2 -> v3: rebase onto ppc next-test branch > --- > arch/powerpc/platforms/pseries/hotplug-memory.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c > b/arch/powerpc/platforms/pseries/hotplug-memory.c > index 1a3ac3b..def8cb3f 100644 > --- a/arch/powerpc/platforms/pseries/hotplug-memory.c > +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c > @@ -372,6 +372,7 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb) > invalidate_lmb_associativity_index(lmb); > lmb_clear_nid(lmb); > lmb->flags &= ~DRCONF_MEM_ASSIGNED; > + drmem_update_dt(); No error checking? > __remove_memory(nid, base_addr, block_sz); > > @@ -607,6 +608,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb) > > lmb_set_nid(lmb); > lmb->flags |= DRCONF_MEM_ASSIGNED; > + drmem_update_dt(); And here .. > > block_sz = memory_block_size_bytes(); > > @@ -625,6 +627,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb) > invalidate_lmb_associativity_index(lmb); > lmb_clear_nid(lmb); > lmb->flags &= ~DRCONF_MEM_ASSIGNED; > + drmem_update_dt(); And here .. > __remove_memory(nid, base_addr, block_sz); > } > @@ -877,9 +880,6 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog) > break; > } > > - if (!rc) > - rc = drmem_update_dt(); > - > unlock_device_hotplug(); > return rc; Whereas previously we did check it. cheers ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v4 07/12] ppc64/kexec_file: add support to relocate purgatory
Hari Bathini writes: > Right now purgatory implementation is only minimal. But if purgatory > code is to be enhanced to copy memory to the backup region and verify > sha256 digest, relocations may have to be applied to the purgatory. > So, add support to relocate purgatory in kexec_file_load system call > by setting up TOC pointer and applying RELA relocations as needed. > > Reported-by: kernel test robot > [lkp: In v1, 'struct mem_sym' was declared in parameter list] > Signed-off-by: Hari Bathini > --- > > * Michael, can you share your opinion on the below: > - https://lore.kernel.org/patchwork/patch/1272027/ > - My intention in cover note. It seems like a lot of complexity for little benefit. AFAICS your final purgatory_64.c is only 36 lines, and all it does is a single (open coded) memcpy(). It seems like we could write that in not many more lines of assembler and avoid all this code. What am I missing? cheers ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCHv3 1/2] powerpc/pseries: group lmb operation and memblock's
This patch prepares for the incoming patch which swaps the order of KOBJ_
uevent and dt's updating.
It has no functional effect, just groups lmb operation and memblock's in
order to insert dt updating operation easily, and makes it easier to
review.
Signed-off-by: Pingfan Liu
Cc: Michael Ellerman
Cc: Hari Bathini
Cc: Nathan Lynch
To: linuxppc-...@lists.ozlabs.org
Cc: kexec@lists.infradead.org
---
v2 -> v3: rebase onto ppc next-test branch
---
arch/powerpc/platforms/pseries/hotplug-memory.c | 26 -
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c
b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 5d545b7..1a3ac3b 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -355,7 +355,8 @@ static int dlpar_add_lmb(struct drmem_lmb *);
static int dlpar_remove_lmb(struct drmem_lmb *lmb)
{
unsigned long block_sz;
- int rc;
+ phys_addr_t base_addr;
+ int rc, nid;
if (!lmb_is_removable(lmb))
return -EINVAL;
@@ -364,17 +365,19 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
if (rc)
return rc;
+ base_addr = lmb->base_addr;
+ nid = lmb->nid;
block_sz = pseries_memory_block_size();
- __remove_memory(lmb->nid, lmb->base_addr, block_sz);
-
- /* Update memory regions for memory remove */
- memblock_remove(lmb->base_addr, block_sz);
-
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+ __remove_memory(nid, base_addr, block_sz);
+
+ /* Update memory regions for memory remove */
+ memblock_remove(base_addr, block_sz);
+
return 0;
}
@@ -603,6 +606,8 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
}
lmb_set_nid(lmb);
+ lmb->flags |= DRCONF_MEM_ASSIGNED;
+
block_sz = memory_block_size_bytes();
/* Add the memory */
@@ -614,11 +619,14 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
rc = dlpar_online_lmb(lmb);
if (rc) {
- __remove_memory(lmb->nid, lmb->base_addr, block_sz);
+ int nid = lmb->nid;
+ phys_addr_t base_addr = lmb->base_addr;
+
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
- } else {
- lmb->flags |= DRCONF_MEM_ASSIGNED;
+ lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+
+ __remove_memory(nid, base_addr, block_sz);
}
return rc;
--
2.7.5
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
[PATCHv3 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
A bug is observed on pseries by taking the following steps on rhel: -1. drmgr -c mem -r -q 5 -2. echo c > /proc/sysrq-trigger And then, the failure looks like: kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/ kdump: saving vmcore-dmesg.txt kdump: saving vmcore-dmesg.txt complete kdump: saving vmcore Checking for memory holes : [ 0.0 %] / Checking for memory holes : [100.0 %] | Excluding unnecessary pages : [100.0 %] \ Copying data : [ 0.3 %] - eta: 38s[ 44.337636] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 access=0x8004 current=makedumpfile [ 44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc0005504 [ 44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba40 access=0x8004 current=makedumpfile [ 44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc0005504 [ 44.337708] makedumpfile[469]: unhandled signal 7 at 7fffba40 nip 7fffbbc4d7fc lr 00011356ca3c code 2 [ 44.338548] Core dump to |/bin/false pipe failed /lib/kdump-lib-initramfs.sh: line 98: 469 Bus error $CORE_COLLECTOR /proc/vmcore $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete kdump: saving vmcore failed * Root cause * After analyzing, it turns out that in the current implementation, when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt updating as the code __remove_memory() comes before drmem_update_dt(). So in kdump kernel, when read_from_oldmem() resorts to pSeries_lpar_hpte_insert() to install hpte, but fails with -2 due to non-exist pfn. And finally, low_hash_fault() raise SIGBUS to process, as it can be observed "Bus error" >From a viewpoint of listener and publisher, the publisher notifies the listener before data is ready. This introduces a problem where udev launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before updating. And in capture kernel, makedumpfile will access the memory based on the stale dt info, and hit a SIGBUS error due to an un-existed lmb. * Fix * In order to fix this issue, update dt before __remove_memory(), and accordingly the same rule in hot-add path. This will introduce extra dt updating payload for each involved lmb when hotplug. But it should be fine since drmem_update_dt() is memory based operation and hotplug is not a hot path. Signed-off-by: Pingfan Liu Cc: Michael Ellerman Cc: Hari Bathini Cc: Nathan Lynch To: linuxppc-...@lists.ozlabs.org Cc: kexec@lists.infradead.org --- v2 -> v3: rebase onto ppc next-test branch --- arch/powerpc/platforms/pseries/hotplug-memory.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c index 1a3ac3b..def8cb3f 100644 --- a/arch/powerpc/platforms/pseries/hotplug-memory.c +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -372,6 +372,7 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb) invalidate_lmb_associativity_index(lmb); lmb_clear_nid(lmb); lmb->flags &= ~DRCONF_MEM_ASSIGNED; + drmem_update_dt(); __remove_memory(nid, base_addr, block_sz); @@ -607,6 +608,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb) lmb_set_nid(lmb); lmb->flags |= DRCONF_MEM_ASSIGNED; + drmem_update_dt(); block_sz = memory_block_size_bytes(); @@ -625,6 +627,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb) invalidate_lmb_associativity_index(lmb); lmb_clear_nid(lmb); lmb->flags &= ~DRCONF_MEM_ASSIGNED; + drmem_update_dt(); __remove_memory(nid, base_addr, block_sz); } @@ -877,9 +880,6 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog) break; } - if (!rc) - rc = drmem_update_dt(); - unlock_device_hotplug(); return rc; } -- 2.7.5 ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 06/13] fs/kernel_read_file: Remove redundant size argument
On Tue, Jul 21, 2020 at 02:43:07PM -0700, Scott Branden wrote:
> On 2020-07-17 10:43 a.m., Kees Cook wrote:
> > In preparation for refactoring kernel_read_file*(), remove the redundant
> > "size" argument which is not needed: it can be included in the return
> > code, with callers adjusted. (VFS reads already cannot be larger than
> > INT_MAX.)
> >
> > Signed-off-by: Kees Cook
> > ---
> > drivers/base/firmware_loader/main.c | 8
> > fs/kernel_read_file.c | 20 +---
> > include/linux/kernel_read_file.h| 8
> > kernel/kexec_file.c | 13 ++---
> > kernel/module.c | 7 +++
> > security/integrity/digsig.c | 5 +++--
> > security/integrity/ima/ima_fs.c | 5 +++--
> > 7 files changed, 32 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/base/firmware_loader/main.c
> > b/drivers/base/firmware_loader/main.c
> > index d4a413ea48ce..ea419c7d3d34 100644
> > --- a/drivers/base/firmware_loader/main.c
> > +++ b/drivers/base/firmware_loader/main.c
> > @@ -462,7 +462,7 @@ fw_get_filesystem_firmware(struct device *device,
> > struct fw_priv *fw_priv,
> > size_t in_size,
> > const void *in_buffer))
> > {
> > - loff_t size;
> > + size_t size;
> > int i, len;
> > int rc = -ENOENT;
> > char *path;
> > @@ -494,10 +494,9 @@ fw_get_filesystem_firmware(struct device *device,
> > struct fw_priv *fw_priv,
> > fw_priv->size = 0;
> > /* load firmware files from the mount namespace of init */
> > - rc = kernel_read_file_from_path_initns(path, ,
> > - , msize,
> > + rc = kernel_read_file_from_path_initns(path, , msize,
> >READING_FIRMWARE);
> > - if (rc) {
> > + if (rc < 0) {
> > if (rc != -ENOENT)
> > dev_warn(device, "loading %s failed with error
> > %d\n",
> > path, rc);
> > @@ -506,6 +505,7 @@ fw_get_filesystem_firmware(struct device *device,
> > struct fw_priv *fw_priv,
> > path);
> > continue;
> > }
> > + size = rc;
> Change fails to return 0. Need rc = 0; here.
Oh nice; good catch! I'll fix this.
--
Kees Cook
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 06/13] fs/kernel_read_file: Remove redundant size argument
Hi Kees,
On 2020-07-17 10:43 a.m., Kees Cook wrote:
In preparation for refactoring kernel_read_file*(), remove the redundant
"size" argument which is not needed: it can be included in the return
code, with callers adjusted. (VFS reads already cannot be larger than
INT_MAX.)
Signed-off-by: Kees Cook
---
drivers/base/firmware_loader/main.c | 8
fs/kernel_read_file.c | 20 +---
include/linux/kernel_read_file.h| 8
kernel/kexec_file.c | 13 ++---
kernel/module.c | 7 +++
security/integrity/digsig.c | 5 +++--
security/integrity/ima/ima_fs.c | 5 +++--
7 files changed, 32 insertions(+), 34 deletions(-)
diff --git a/drivers/base/firmware_loader/main.c
b/drivers/base/firmware_loader/main.c
index d4a413ea48ce..ea419c7d3d34 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -462,7 +462,7 @@ fw_get_filesystem_firmware(struct device *device, struct
fw_priv *fw_priv,
size_t in_size,
const void *in_buffer))
{
- loff_t size;
+ size_t size;
int i, len;
int rc = -ENOENT;
char *path;
@@ -494,10 +494,9 @@ fw_get_filesystem_firmware(struct device *device, struct
fw_priv *fw_priv,
fw_priv->size = 0;
/* load firmware files from the mount namespace of init */
- rc = kernel_read_file_from_path_initns(path, ,
- , msize,
+ rc = kernel_read_file_from_path_initns(path, , msize,
READING_FIRMWARE);
- if (rc) {
+ if (rc < 0) {
if (rc != -ENOENT)
dev_warn(device, "loading %s failed with error
%d\n",
path, rc);
@@ -506,6 +505,7 @@ fw_get_filesystem_firmware(struct device *device, struct
fw_priv *fw_priv,
path);
continue;
}
+ size = rc;
Change fails to return 0. Need rc = 0; here.
dev_dbg(device, "Loading firmware from %s\n", path);
if (decompress) {
dev_dbg(device, "f/w decompressing %s\n",
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 2/4] printk: store instead of processing cont parts
On Tue, Jul 21, 2020 at 7:42 AM Sergey Senozhatsky
wrote:
>
> OK, so basically, extending printk_caller_id() so that for IRQ/NMI
> we will have more info than just "0x8000 + raw_smp_processor_id()".
I think it's really preempt_count() that we want to have there.
That has the softirq/hardirq/NMI information in it.
So the "context" identifier of an incomplete write would be { task,
cpu, preempt_count() } of the writer. If a new KERN_CONT writer comes
in, it would have to match in that context to actually combine.
You can probably play "hide the bits" tricks to not *ac tually* use
three words for it. The task structure in particular tends to be very
aligned, you could hide bits in the low bits there. The CPU number
would fit in there, for example.
Linus
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 2/4] printk: store instead of processing cont parts
On 2020-07-21, Sergey Senozhatsky wrote: >> That said, we have traditionally used not just "current process", but >> also "last irq-level" as the context information, so I do think it >> would be good to continue to do that. > > OK, so basically, extending printk_caller_id() so that for IRQ/NMI > we will have more info than just "0x8000 + raw_smp_processor_id()". If bit31 is set, the upper 8 bits could specify what the lower 24 bits represent. That would give some freedom for the future. For example: 0x80 = cpu id (generic context) 0x81 = interrupt number 0x82 = cpu id (nmi context) Or maybe ascii should be used instead? 0x80 | '\0' = cpu id (generic context) 0x80 | 'i' = interrupt number 0x80 | 'n' = cpu id (nmi context) Just an idea. John Ogness ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH 2/4] printk: store instead of processing cont parts
On (20/07/20 11:30), Linus Torvalds wrote: > > Do I get it right, what you are saying is - when we process a PR_CONT > > message the cont buffer should already contain previous non-LOG_NEWLINE > > and non-PR_CONT message, otherwise it's a bug? > > No. > > I'm saying that the code that does PR_CONT should have done *some* > printing before, otherwise it's at the very least questionable. > > IOW, you can't just randomly start printing with PR_CONT, without > having established _some_ context for it. OK, I see. I sort of suspect that we may actually have code that does just pr_cont() (e.g. what Joe pointed out). It doesn't seem like that "establish a context" was ever enforced, doing a bunch of pr_cont() simply works. [..] > That said, we have traditionally used not just "current process", but > also "last irq-level" as the context information, so I do think it > would be good to continue to do that. OK, so basically, extending printk_caller_id() so that for IRQ/NMI we will have more info than just "0x8000 + raw_smp_processor_id()". -ss ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
[PATCH v2][next] printk: ringbuffer: support dataless records
With commit ("printk: use the lockless ringbuffer"), printk()
started silently dropping messages without text because such
records are not supported by the new printk ringbuffer.
Add support for such records.
Currently dataless records are denoted by INVALID_LPOS in order
to recognize failed prb_reserve() calls. Change the ringbuffer
to instead use two different identifiers (FAILED_LPOS and
NO_LPOS) to distinguish between failed prb_reserve() records and
successful dataless records, respectively.
Fixes: ("printk: use the lockless ringbuffer")
Fixes: https://lkml.kernel.org/r/20200718121053.ga691...@elver.google.com
Reported-by: Marco Elver
Signed-off-by: John Ogness
---
based on next-20200721
chages since v1:
- Instead of handling empty text messages as special case errors,
allow such messages to be handled as any other valid messages.
This also allows the empty text message to be counted as a line.
kernel/printk/printk_ringbuffer.c | 72 +++
kernel/printk/printk_ringbuffer.h | 15 ---
2 files changed, 43 insertions(+), 44 deletions(-)
diff --git a/kernel/printk/printk_ringbuffer.c
b/kernel/printk/printk_ringbuffer.c
index 7355ca99e852..0659b50872b5 100644
--- a/kernel/printk/printk_ringbuffer.c
+++ b/kernel/printk/printk_ringbuffer.c
@@ -264,6 +264,9 @@
/* Determine how many times the data array has wrapped. */
#define DATA_WRAPS(data_ring, lpos)((lpos) >> (data_ring)->size_bits)
+/* Determine if a logical position refers to a data-less block. */
+#define LPOS_DATALESS(lpos)((lpos) & 1UL)
+
/* Get the logical position at index 0 of the current wrap. */
#define DATA_THIS_WRAP_START_LPOS(data_ring, lpos) \
((lpos) & ~DATA_SIZE_MASK(data_ring))
@@ -320,21 +323,13 @@ static unsigned int to_blk_size(unsigned int size)
* block does not exceed the maximum possible size that could fit within the
* ringbuffer. This function provides that basic size check so that the
* assumption is safe.
- *
- * Writers are also not allowed to write 0-sized (data-less) records. Such
- * records are used only internally by the ringbuffer.
*/
static bool data_check_size(struct prb_data_ring *data_ring, unsigned int size)
{
struct prb_data_block *db = NULL;
- /*
-* Writers are not allowed to write data-less records. Such records
-* are used only internally by the ringbuffer to denote records where
-* their data failed to allocate or have been lost.
-*/
if (size == 0)
- return false;
+ return true;
/*
* Ensure the alignment padded size could possibly fit in the data
@@ -568,8 +563,8 @@ static bool data_push_tail(struct printk_ringbuffer *rb,
unsigned long tail_lpos;
unsigned long next_lpos;
- /* If @lpos is not valid, there is nothing to do. */
- if (lpos == INVALID_LPOS)
+ /* If @lpos is from a data-less block, there is nothing to do. */
+ if (LPOS_DATALESS(lpos))
return true;
/*
@@ -962,8 +957,8 @@ static char *data_alloc(struct printk_ringbuffer *rb,
if (size == 0) {
/* Specify a data-less block. */
- blk_lpos->begin = INVALID_LPOS;
- blk_lpos->next = INVALID_LPOS;
+ blk_lpos->begin = NO_LPOS;
+ blk_lpos->next = NO_LPOS;
return NULL;
}
@@ -976,8 +971,8 @@ static char *data_alloc(struct printk_ringbuffer *rb,
if (!data_push_tail(rb, data_ring, next_lpos -
DATA_SIZE(data_ring))) {
/* Failed to allocate, specify a data-less block. */
- blk_lpos->begin = INVALID_LPOS;
- blk_lpos->next = INVALID_LPOS;
+ blk_lpos->begin = FAILED_LPOS;
+ blk_lpos->next = FAILED_LPOS;
return NULL;
}
@@ -1025,6 +1020,10 @@ static char *data_alloc(struct printk_ringbuffer *rb,
static unsigned int space_used(struct prb_data_ring *data_ring,
struct prb_data_blk_lpos *blk_lpos)
{
+ /* Data-less blocks take no space. */
+ if (LPOS_DATALESS(blk_lpos->begin))
+ return 0;
+
if (DATA_WRAPS(data_ring, blk_lpos->begin) == DATA_WRAPS(data_ring,
blk_lpos->next)) {
/* Data block does not wrap. */
return (DATA_INDEX(data_ring, blk_lpos->next) -
@@ -1080,11 +1079,8 @@ bool prb_reserve(struct prb_reserved_entry *e, struct
printk_ringbuffer *rb,
if (!data_check_size(>text_data_ring, r->text_buf_size))
goto fail;
- /* Records are allowed to not have dictionaries. */
- if (r->dict_buf_size) {
- if (!data_check_size(>dict_data_ring, r->dict_buf_size))
- goto fail;
- }
+
Re: [PATCH][next] printk: ringbuffer: support dataless records
On 2020-07-21, Sergey Senozhatsky wrote:
>> @@ -1402,7 +1396,9 @@ static int prb_read(struct printk_ringbuffer *rb, u64
>> seq,
>> /* Copy text data. If it fails, this is a data-less record. */
>> if (!copy_data(>text_data_ring, _blk_lpos,
>> desc.info.text_len,
>> r->text_buf, r->text_buf_size, line_count)) {
>> -return -ENOENT;
>> +/* Report an error if there should have been data. */
>> +if (desc.info.text_len != 0)
>> +return -ENOENT;
>> }
>
> If this is a dataless record then should copy_data() return error?
You are correct. That makes more sense. I will send a v2.
John Ogness
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
