[PATCH] kexec-tools: Remove duplicated variable declarations

2020-01-28 Thread Kairui Song
When building kexec-tools for Fedora 32, following error is observed:

/usr/bin/ld: kexec/arch/x86_64/kexec-bzImage64.o:(.bss+0x0): multiple 
definition of `bzImage_support_efi_boot';
kexec/arch/i386/kexec-bzImage.o:(.bss+0x0): first defined here

/builddir/build/BUILD/kexec-tools-2.0.20/kexec/arch/arm/../../fs2dt.h:33: 
multiple definition of `my_debug';
kexec/fs2dt.o:/builddir/build/BUILD/kexec-tools-2.0.20/kexec/fs2dt.h:33: first 
defined here

/builddir/build/BUILD/kexec-tools-2.0.20/kexec/arch/arm64/kexec-arm64.h:68: 
multiple definition of `arm64_mem';
kexec/fs2dt.o:/builddir/build/BUILD/kexec-tools-2.0.20/././kexec/arch/arm64/kexec-arm64.h:68:
 first defined here

/builddir/build/BUILD/kexec-tools-2.0.20/kexec/arch/arm64/kexec-arm64.h:54: 
multiple definition of `initrd_size';
kexec/fs2dt.o:/builddir/build/BUILD/kexec-tools-2.0.20/././kexec/arch/arm64/kexec-arm64.h:54:
 first defined here

/builddir/build/BUILD/kexec-tools-2.0.20/kexec/arch/arm64/kexec-arm64.h:53: 
multiple definition of `initrd_base';
kexec/fs2dt.o:/builddir/build/BUILD/kexec-tools-2.0.20/././kexec/arch/arm64/kexec-arm64.h:53:
 first defined here

And apparently, these variables are wrongly declared multiple times. So
remove duplicated declaration.

Signed-off-by: Kairui Song 
---
 kexec/arch/arm64/kexec-arm64.h  | 6 +++---
 kexec/arch/ppc64/kexec-elf-ppc64.c  | 2 --
 kexec/arch/x86_64/kexec-bzImage64.c | 1 -
 kexec/fs2dt.h   | 2 +-
 4 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/kexec/arch/arm64/kexec-arm64.h b/kexec/arch/arm64/kexec-arm64.h
index 628de79..ed447ac 100644
--- a/kexec/arch/arm64/kexec-arm64.h
+++ b/kexec/arch/arm64/kexec-arm64.h
@@ -50,8 +50,8 @@ int zImage_arm64_load(int argc, char **argv, const char 
*kernel_buf,
 void zImage_arm64_usage(void);
 
 
-off_t initrd_base;
-off_t initrd_size;
+extern off_t initrd_base;
+extern off_t initrd_size;
 
 /**
  * struct arm64_mem - Memory layout info.
@@ -65,7 +65,7 @@ struct arm64_mem {
 };
 
 #define arm64_mem_ngv UINT64_MAX
-struct arm64_mem arm64_mem;
+extern struct arm64_mem arm64_mem;
 
 uint64_t get_phys_offset(void);
 uint64_t get_vp_offset(void);
diff --git a/kexec/arch/ppc64/kexec-elf-ppc64.c 
b/kexec/arch/ppc64/kexec-elf-ppc64.c
index 3510b70..695b8b0 100644
--- a/kexec/arch/ppc64/kexec-elf-ppc64.c
+++ b/kexec/arch/ppc64/kexec-elf-ppc64.c
@@ -44,8 +44,6 @@
 uint64_t initrd_base, initrd_size;
 unsigned char reuse_initrd = 0;
 const char *ramdisk;
-/* Used for enabling printing message from purgatory code */
-int my_debug = 0;
 
 int elf_ppc64_probe(const char *buf, off_t len)
 {
diff --git a/kexec/arch/x86_64/kexec-bzImage64.c 
b/kexec/arch/x86_64/kexec-bzImage64.c
index 8edb3e4..ba8dc48 100644
--- a/kexec/arch/x86_64/kexec-bzImage64.c
+++ b/kexec/arch/x86_64/kexec-bzImage64.c
@@ -42,7 +42,6 @@
 #include 
 
 static const int probe_debug = 0;
-int bzImage_support_efi_boot;
 
 int bzImage64_probe(const char *buf, off_t len)
 {
diff --git a/kexec/fs2dt.h b/kexec/fs2dt.h
index 7633273..fe24931 100644
--- a/kexec/fs2dt.h
+++ b/kexec/fs2dt.h
@@ -30,7 +30,7 @@ extern struct bootblock bb[1];
 
 /* Used for enabling printing message from purgatory code
  * Only has implemented for PPC64 */
-int my_debug;
+extern int my_debug;
 extern int dt_no_old_root;
 
 void reserve(unsigned long long where, unsigned long long length);
-- 
2.24.1


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


[PATCH] makedumpfile: Remove duplicated variable declarations

2020-01-28 Thread Kairui Song
When building on Fedora 32, following error is observed:

/usr/bin/ld: 
erase_info.o:/builddir/build/BUILD/kexec-tools-2.0.20/makedumpfile-1.6.7/makedumpfile.h:2010:
multiple definition of `crash_reserved_mem_nr'; 
elf_info.o:/builddir/build/BUILD/kexec-tools-2.0.20/makedumpfile-1.6.7/makedumpfile.h:2010:
 first defined here
/usr/bin/ld: 
erase_info.o:/builddir/build/BUILD/kexec-tools-2.0.20/makedumpfile-1.6.7/makedumpfile.h:2009:
multiple definition of `crash_reserved_mem'; 
elf_info.o:/builddir/build/BUILD/kexec-tools-2.0.20/makedumpfile-1.6.7/makedumpfile.h:2009:
 first defined here
/usr/bin/ld: 
erase_info.o:/builddir/build/BUILD/kexec-tools-2.0.20/makedumpfile-1.6.7/makedumpfile.h:1278:
multiple definition of `parallel_info_t'; 
elf_info.o:/builddir/build/BUILD/kexec-tools-2.0.20/makedumpfile-1.6.7/makedumpfile.h:1278:
 first defined here
/usr/bin/ld: 
erase_info.o:/builddir/build/BUILD/kexec-tools-2.0.20/makedumpfile-1.6.7/makedumpfile.h:1265:
multiple definition of `splitting_info_t'; 
elf_info.o:/builddir/build/BUILD/kexec-tools-2.0.20/makedumpfile-1.6.7/makedumpfile.h:1265:
 first defined here

And apparently, these variables are wrongly declared multiple times. So
remove duplicated declaration.

Signed-off-by: Kairui Song 
---
 makedumpfile.c |  2 ++
 makedumpfile.h | 10 ++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index e290fbd..9aad77b 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -34,6 +34,8 @@ struct array_tablearray_table;
 struct number_tablenumber_table;
 struct srcfile_table   srcfile_table;
 struct save_controlsc;
+struct parallel_info   parallel_info_t;
+struct splitting_info  splitting_info_t;
 
 struct vm_tablevt = { 0 };
 struct DumpInfo*info = NULL;
diff --git a/makedumpfile.h b/makedumpfile.h
index 68d9691..614764c 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1262,7 +1262,8 @@ struct splitting_info {
mdf_pfn_t   end_pfn;
off_t   offset_eraseinfo;
unsigned long   size_eraseinfo;
-} splitting_info_t;
+};
+extern struct splitting_info splitting_info_t;
 
 struct parallel_info {
int fd_memory;
@@ -1275,7 +1276,8 @@ struct parallel_info {
 #ifdef USELZO
lzo_bytep   wrkmem;
 #endif
-} parallel_info_t;
+};
+extern struct parallel_info parallel_info_t;
 
 struct ppc64_vmemmap {
unsigned long   phys;
@@ -2006,8 +2008,8 @@ struct memory_range {
 };
 
 #define CRASH_RESERVED_MEM_NR   8
-struct memory_range crash_reserved_mem[CRASH_RESERVED_MEM_NR];
-int crash_reserved_mem_nr;
+extern struct memory_range crash_reserved_mem[CRASH_RESERVED_MEM_NR];
+extern int crash_reserved_mem_nr;
 
 unsigned long read_vmcoreinfo_symbol(char *str_symbol);
 int readmem(int type_addr, unsigned long long addr, void *bufptr, size_t size);
-- 
2.24.1


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH 1/2] printk: add lockless buffer

2020-01-28 Thread Steven Rostedt
On Tue, 28 Jan 2020 17:25:47 +0106
John Ogness  wrote:

> diff --git a/kernel/printk/printk_ringbuffer.c 
> b/kernel/printk/printk_ringbuffer.c
> new file mode 100644
> index ..796257f226ee
> --- /dev/null
> +++ b/kernel/printk/printk_ringbuffer.c
> @@ -0,0 +1,1370 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "printk_ringbuffer.h"
> +
> +/**
> + * DOC: printk_ringbuffer overview
> + *
> + * Data Structure
> + * --
> + * The printk_ringbuffer is made up of 3 internal ringbuffers::
> + *
> + *   * desc_ring:  A ring of descriptors. A descriptor contains all 
> record
> + * meta data (sequence number, timestamp, loglevel, etc.)
> + * as well as internal state information about the record
> + * and logical positions specifying where in the other
> + * ringbuffers the text and dictionary strings are
> + * located.
> + *
> + *   * text_data_ring: A ring of data blocks. A data block consists of an
> + * unsigned long integer (ID) that maps to a desc_ring
> + * index followed by the text string of the record.
> + *
> + *   * dict_data_ring: A ring of data blocks. A data block consists of an
> + * unsigned long integer (ID) that maps to a desc_ring
> + * index followed by the dictionary string of the record.
> + *
> + * Implementation
> + * --
> + *
> + * ABA Issues
> + * ~~
> + * To help avoid ABA issues, descriptors are referenced by IDs (index values
> + * with tagged states) and data blocks are referenced by logical positions
> + * (index values with tagged states). However, on 32-bit systems the number
> + * of tagged states is relatively small such that an ABA incident is (at
> + * least theoretically) possible. For example, if 4 million maximally sized

4 million? I'm guessing that maximally sized printk messages are 1k?

Perhaps say that, otherwise one might think this is a mistake. "4
million maximally sized (1k) printk messages"

> + * printk messages were to occur in NMI context on a 32-bit system, the
> + * interrupted task would not be able to recognize that the 32-bit integer
> + * wrapped and thus represents a different data block than the one the
> + * interrupted task expects.
> + *
> + * To help combat this possibility, additional state checking is performed
> + * (such as using cmpxchg() even though set() would suffice). These extra
> + * checks will hopefully catch any ABA issue that a 32-bit system might
> + * experience.
> + *
[..]

> + * Usage
> + * -
> + * Here are some simple examples demonstrating writers and readers. For the
> + * examples a global ringbuffer (test_rb) is available (which is not the
> + * actual ringbuffer used by printk)::
> + *
> + *   DECLARE_PRINTKRB(test_rb, 15, 5, 3);
> + *
> + * This ringbuffer allows up to 32768 records (2 ^ 15) and has a size of
> + * 1 MiB (2 ^ 20) for text data and 256 KiB (2 ^ 18) for dictionary data.

 (2 ^ (15 + 5)) ... (2 ^ (15 + 3)) ?

I'll play around more with this this week. But so far it looks good.

-- Steve

> + *
> + * Sample writer code::
> + *
> + *   struct prb_reserved_entry e;
> + *   struct printk_record r;
> + *
> + *   // specify how much to allocate
> + *   r.text_buf_size = strlen(textstr) + 1;
> + *   r.dict_buf_size = strlen(dictstr) + 1;
> + *
> + *   if (prb_reserve(, _rb, )) {
> + *   snprintf(r.text_buf, r.text_buf_size, "%s", textstr);
> + *
> + *   // dictionary allocation may have failed
> + *   if (r.dict_buf)
> + *   snprintf(r.dict_buf, r.dict_buf_size, "%s", dictstr);
> + *
> + *   r.info->ts_nsec = local_clock();
> + *
> + *   prb_commit();
> + *   }
> + *

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH] makedumpfile: cope with not-present mem section

2020-01-28 Thread Thadeu Lima de Souza Cascardo
On Tue, Jan 28, 2020 at 05:03:12PM +, HAGIO KAZUHITO(萩尾 一仁) wrote:
> Hi Cascardo,
> 
> > -Original Message-
> > On Mon, Jan 27, 2020 at 02:04:54PM -0300, Thadeu Lima de Souza Cascardo 
> > wrote:
> > > Sorry for taking too long to respond, as I was on vacation.
> > >
> > > The kernels that had commit 83e3c48729d9, but not commit a0b1280368d1, are
> > > not supported anymore. In a way that it's even hard for me to test them.
> > >
> > > However, I managed to test it, and those two lines are definitively needed
> > > to dump a VM running such a kernel. Is removing them really needed to fix
> > > this issue?
> > >
> > > Otherwise, I would rather keep them.
> > >
> > > Thanks.
> > > Cascardo.
> > 
> > By the way, I was too fast in sending this. We really need to keep those 
> > lines
> > as makedumpfile will fail to dump a 4.4 kernel with this patch as is.
> 
> Is that Ubuntu 4.4 kernel which has 83e3c48729d9 and not a0b1280368d1?
> Could you elaborate on how it fails?

No, it doesn't have either, so my guess is it would fail on upstream 4.4 as
well, so anything that doesn't have 83e3c48729d9.

That's what I get on that 4.4 kernel (4.4.0-171-generic):

# ./makedumpfile /proc/vmcore ../dump
get_mem_section: Could not validate mem_section.
get_mm_sparsemem: Can't get the address of mem_section.

makedumpfile Failed.
#

So, now, I have a better grasp of the whole logic, and understand why it fails
with this patch.

So, we need to either interpret the mem_section as a pointer to the array of a
pointer to the pointer to the array. The only case the second option is valid
is when sparse_extreme is on, so we don't even need to check the second case
when it's off.

Then, we check that interpreting it either way is valid. If it's valid in both
interpretations, we can't decide which to use, and will fail. So far, we
haven't seen any case in the field where that would accidentally happen. But in
case it does, we should rather fail to dump and fallback to copying, than
creating a bogus compressed dump.

When this patch is applied, a kernel which does not have 83e3c48729d9, and
thus, has mem_section as a direct pointer to the array, it so happens that we
don't detect the pointer to pointer to the array case as invalid, thus failing
to dump.

The way we validate is that the mem_maps should either have the PRESENT bit or
be NULL. Now, that assumption may be invalid, and we would need to do the
validation some other way. I can test the cases where that assumption is
invalid in a 4.4 kernel and see how to fix this in a satisfactory way.

Going through the code once again, I don't see how the second section of the
patch would be correct by itself too. I will think it through a little more and
see if I can come up with a solution.

Regards.
Cascardo.

> 
> I'm thinking that Pingfan's thought may help:
> >> I think it could be if/else, no need to call twice.
> 
> Thanks,
> Kazu
> 
> > 
> > Cascardo.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


RE: [PATCH] makedumpfile: cope with not-present mem section

2020-01-28 Thread 萩尾 一仁
Hi Cascardo,

> -Original Message-
> On Mon, Jan 27, 2020 at 02:04:54PM -0300, Thadeu Lima de Souza Cascardo wrote:
> > Sorry for taking too long to respond, as I was on vacation.
> >
> > The kernels that had commit 83e3c48729d9, but not commit a0b1280368d1, are
> > not supported anymore. In a way that it's even hard for me to test them.
> >
> > However, I managed to test it, and those two lines are definitively needed
> > to dump a VM running such a kernel. Is removing them really needed to fix
> > this issue?
> >
> > Otherwise, I would rather keep them.
> >
> > Thanks.
> > Cascardo.
> 
> By the way, I was too fast in sending this. We really need to keep those lines
> as makedumpfile will fail to dump a 4.4 kernel with this patch as is.

Is that Ubuntu 4.4 kernel which has 83e3c48729d9 and not a0b1280368d1?
Could you elaborate on how it fails?

I'm thinking that Pingfan's thought may help:
>> I think it could be if/else, no need to call twice.

Thanks,
Kazu

> 
> Cascardo.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


[PATCH 0/2] printk: replace ringbuffer

2020-01-28 Thread John Ogness
Hello,

After several RFC series [0][1][2][3][4], here is the first set of
patches to rework the printk subsystem. This first set of patches
only replace the existing ringbuffer implementation. No locking is
removed. No semantics/behavior of printk are changed.

The VMCOREINFO is updated, which will require changes to the
external crash [5] tool. I will be preparing a patch to add support
for the new VMCOREINFO.

This series is in line with the agreements [6] made at the meeting
during LPC2019 in Lisbon, with 1 exception: support for dictionaries
will _not_ be discontinued [7]. Dictionaries 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/2019072701.11260-1-john.ogn...@linutronix.de
[3] https://lkml.kernel.org/r/20190807222634.1723-1-john.ogn...@linutronix.de
[4] https://lkml.kernel.org/r/20191128015235.12940-1-john.ogn...@linutronix.de
[5] https://github.com/crash-utility/crash
[6] https://lkml.kernel.org/r/87k1acz5rx@linutronix.de
[7] https://lkml.kernel.org/r/20191007120134.ciywr3wale4gx...@pathway.suse.cz

John Ogness (2):
  printk: add lockless buffer
  printk: use the lockless ringbuffer

 include/linux/kmsg_dump.h |2 -
 kernel/printk/Makefile|1 +
 kernel/printk/printk.c|  836 +-
 kernel/printk/printk_ringbuffer.c | 1370 +
 kernel/printk/printk_ringbuffer.h |  328 +++
 5 files changed, 2114 insertions(+), 423 deletions(-)
 create mode 100644 kernel/printk/printk_ringbuffer.c
 create mode 100644 kernel/printk/printk_ringbuffer.h

-- 
2.20.1


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


[RESEND][PATCH][v2] ata: ahci: Add shutdown to freeze hardware resources of ahci

2020-01-28 Thread Prabhakar Kushwaha
device_shutdown() called from reboot or power_shutdown expect
all devices to be shutdown. Same is true for even ahci pci driver.
As no ahci shutdown function is implemented, the ata subsystem
always remains alive with DMA & interrupt support. File system 
related calls should not be honored after device_shutdown().

So defining ahci pci driver shutdown to freeze hardware (mask
interrupt, stop DMA engine and free DMA resources).

Signed-off-by: Prabhakar Kushwaha 
---
- Resending with linux-ker...@vger.kernel.org in CC
- Changes for v2: Incorporated Bjorn Helgaas's comments
   - Updated description
   - Removed Spurious whitespace change

Adding more details:
We are seeing an issue during kexec -e where SATA hard-disk is not
getting detected in second kernel. Lots of filesystem sync calls
can be seen even after device_shutdown(). 
Further invastigation revealed that pci_clear_master() causing this issue.
During device_shutdown() --> pci_device_shutdown() calls 
  --> ahci shutdown (not defined as of now, so device is still alive)
  --> pci_clear_master() 
pci_clear_master() disable DMA of device. Here filesystem calls after
device_shutdown() causing the issue. 

A device must implement shutdown() to stop/freeze before pcie_clear_master()
called from pci_device_shutdown(). Otherwise It can have weird effect.  
Refer  https://bugzilla.kernel.org/show_bug.cgi?id=63861#c24


 drivers/ata/ahci.c|  7 +++
 drivers/ata/libata-core.c | 21 +
 include/linux/libata.h|  1 +
 3 files changed, 29 insertions(+)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 4bfd1b14b390..11ea1aff40db 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -81,6 +81,7 @@ enum board_ids {
 
 static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id 
*ent);
 static void ahci_remove_one(struct pci_dev *dev);
+static void ahci_shutdown_one(struct pci_dev *dev);
 static int ahci_vt8251_hardreset(struct ata_link *link, unsigned int *class,
 unsigned long deadline);
 static int ahci_avn_hardreset(struct ata_link *link, unsigned int *class,
@@ -606,6 +607,7 @@ static struct pci_driver ahci_pci_driver = {
.id_table   = ahci_pci_tbl,
.probe  = ahci_init_one,
.remove = ahci_remove_one,
+   .shutdown   = ahci_shutdown_one,
.driver = {
.pm = _pci_pm_ops,
},
@@ -1877,6 +1879,11 @@ static int ahci_init_one(struct pci_dev *pdev, const 
struct pci_device_id *ent)
return 0;
 }
 
+static void ahci_shutdown_one(struct pci_dev *pdev)
+{
+   ata_pci_shutdown_one(pdev);
+}
+
 static void ahci_remove_one(struct pci_dev *pdev)
 {
pm_runtime_get_noresume(>dev);
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 6f4ab5c5b52d..42c8728f6117 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6767,6 +6767,26 @@ void ata_pci_remove_one(struct pci_dev *pdev)
ata_host_detach(host);
 }
 
+void ata_pci_shutdown_one(struct pci_dev *pdev)
+{
+   struct ata_host *host = pci_get_drvdata(pdev);
+   int i;
+
+   for (i = 0; i < host->n_ports; i++) {
+   struct ata_port *ap = host->ports[i];
+
+   ap->pflags |= ATA_PFLAG_FROZEN;
+
+   /* Disable port interrupts */
+   if (ap->ops->freeze)
+   ap->ops->freeze(ap);
+
+   /* Stop the port DMA engines */
+   if (ap->ops->port_stop)
+   ap->ops->port_stop(ap);
+   }
+}
+
 /* move to PCI subsystem */
 int pci_test_config_bits(struct pci_dev *pdev, const struct pci_bits *bits)
 {
@@ -7387,6 +7407,7 @@ EXPORT_SYMBOL_GPL(ata_timing_cycle2mode);
 
 #ifdef CONFIG_PCI
 EXPORT_SYMBOL_GPL(pci_test_config_bits);
+EXPORT_SYMBOL_GPL(ata_pci_shutdown_one);
 EXPORT_SYMBOL_GPL(ata_pci_remove_one);
 #ifdef CONFIG_PM
 EXPORT_SYMBOL_GPL(ata_pci_device_do_suspend);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 2dbde119721d..bff539918d82 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1221,6 +1221,7 @@ struct pci_bits {
 };
 
 extern int pci_test_config_bits(struct pci_dev *pdev, const struct pci_bits 
*bits);
+extern void ata_pci_shutdown_one(struct pci_dev *pdev);
 extern void ata_pci_remove_one(struct pci_dev *pdev);
 
 #ifdef CONFIG_PM
-- 
2.17.1


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec