Message-ID: <ZWcXyeDiI2v251F_@dwarf.suse.cz>
Date:   Wed, 29 Nov 2023 11:51:53 +0100
From:   Jiri Bohac <jbohac@...e.cz>
To:     Baoquan He <bhe@...hat.com>
Cc:     Michal Hocko <mhocko@...e.com>, Pingfan Liu <piliu@...hat.com>,
        Tao Liu <ltao@...hat.com>, Vivek Goyal <vgoyal@...hat.com>,
        Dave Young <dyoung@...hat.com>, kexec@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/4] kdump: crashkernel reservation from CMA

Hi Baoquan,

thanks for your interest...

On Wed, Nov 29, 2023 at 03:57:59PM +0800, Baoquan He wrote:
> On 11/28/23 at 10:08am, Michal Hocko wrote:
> > On Tue 28-11-23 10:11:31, Baoquan He wrote:
> > > On 11/28/23 at 09:12am, Tao Liu wrote:
> > [...]
> > > Thanks for the effort to bring this up, Jiri.
> > > 
> > > I am wondering how you will use this crashkernel=,cma parameter. I mean
> > > the scenario of crashkernel=,cma. Asking this because I don't know how
> > > SUSE deploy kdump in SUSE distros. In SUSE distros, kdump kernel's
> > > driver will be filter out? If latter case, It's possibly having the
> > > on-flight DMA issue, e.g NIC has DMA buffer in the CMA area, but not
> > > reset during kdump bootup because the NIC driver is not loaded in to
> > > initialize. Not sure if this is 100%, possible in theory?

yes, we also only add the necessary drivers to the kdump initrd (using
dracut --hostonly).

The plan was to use this feature by default only on systems where
we are reasonably sure it is safe and let the user experiment
with it when we're not sure.

I grepped a list of all calls to pin_user_pages*. From the 55,
about one half uses FOLL_LONGTERM, so these should be migrated
away from the CMA area. In the rest there are four cases that
don't use the pages to set up DMA:
	mm/process_vm_access.c:		pinned_pages = pin_user_pages_remote(mm, pa, pinned_pages,
	net/rds/info.c:	ret = pin_user_pages_fast(start, nr_pages, FOLL_WRITE, pages);
	drivers/vhost/vhost.c:	r = pin_user_pages_fast(log, 1, FOLL_WRITE, &page);
	kernel/trace/trace_events_user.c:	ret = pin_user_pages_remote(mm->mm, uaddr, 1, FOLL_WRITE | FOLL_NOFAULT,

The remaining cases are potentially problematic:
	drivers/gpu/drm/i915/gem/i915_gem_userptr.c:		ret = pin_user_pages_fast(obj->userptr.ptr + pinned * PAGE_SIZE,
	drivers/iommu/iommufd/iova_bitmap.c:	ret = pin_user_pages_fast((unsigned long)addr, npages,
	drivers/iommu/iommufd/pages.c:	rc = pin_user_pages_remote(
	drivers/media/pci/ivtv/ivtv-udma.c:	err = pin_user_pages_unlocked(user_dma.uaddr, user_dma.page_count,
	drivers/media/pci/ivtv/ivtv-yuv.c:		uv_pages = pin_user_pages_unlocked(uv_dma.uaddr,
	drivers/media/pci/ivtv/ivtv-yuv.c:	y_pages = pin_user_pages_unlocked(y_dma.uaddr,
	drivers/misc/genwqe/card_utils.c:	rc = pin_user_pages_fast(data & PAGE_MASK, /* page aligned addr */
	drivers/misc/xilinx_sdfec.c:	res = pin_user_pages_fast((unsigned long)src_ptr, nr_pages, 0, pages);
	drivers/platform/goldfish/goldfish_pipe.c:	ret = pin_user_pages_fast(first_page, requested_pages,
	drivers/rapidio/devices/rio_mport_cdev.c:		pinned = pin_user_pages_fast(
	drivers/sbus/char/oradax.c:	ret = pin_user_pages_fast((unsigned long)va, 1, FOLL_WRITE, p);
	drivers/scsi/st.c:	res = pin_user_pages_fast(uaddr, nr_pages, rw == READ ? FOLL_WRITE : 0,
	drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:		actual_pages = pin_user_pages_fast((unsigned long)ubuf & PAGE_MASK, num_pages,
	drivers/tee/tee_shm.c:		rc = pin_user_pages_fast(start, num_pages, FOLL_WRITE,
	drivers/vfio/vfio_iommu_spapr_tce.c:	if (pin_user_pages_fast(tce & PAGE_MASK, 1,
	drivers/video/fbdev/pvr2fb.c:	ret = pin_user_pages_fast((unsigned long)buf, nr_pages, FOLL_WRITE, pages);
	drivers/xen/gntdev.c:	ret = pin_user_pages_fast(addr, 1, batch->writeable ? FOLL_WRITE : 0, &page);
	drivers/xen/privcmd.c:		page_count = pin_user_pages_fast(
	fs/orangefs/orangefs-bufmap.c:	ret = pin_user_pages_fast((unsigned long)user_desc->ptr,
	arch/x86/kvm/svm/sev.c:	npinned = pin_user_pages_fast(uaddr, npages, write ? FOLL_WRITE : 0, pages);
	drivers/fpga/dfl-afu-dma-region.c:	pinned = pin_user_pages_fast(region->user_addr, npages, FOLL_WRITE,
	lib/iov_iter.c:	res = pin_user_pages_fast(addr, maxpages, gup_flags, *pages);

We can easily check if some of these drivers (of which some we don't
even ship/support) are loaded and decide this system is not safe
for CMA crashkernel. Maybe looking at the list more thoroughly
will show that even some of the above calls are acually safe,
e.g. because the DMA is set up for reading only.
lib/iov_iter.c seem like it could be the real
problem since it's used by generic block layer...

> > > The crashkernel=,cma requires no userspace data dumping, from our
> > > support engineers' feedback, customer never express they don't need to
> > > dump user space data. Assume a server with huge databse deployed, and
> > > the database often collapsed recently and database provider claimed that
> > > it's not database's fault, OS need prove their innocence. What will you
> > > do?
> > 
> > Don't use CMA backed crash memory then? This is an optional feature.

Right. Our kdump does not dump userspace by default and we would
of course make sure ,cma is not used when the user wanted to turn
on userspace dumping.

> > Jiri will know better than me but for us a proper crash memory
> > configuration has become a real nut. You do not want to reserve too much
> > because it is effectively cutting of the usable memory and we regularly
> > hit into "not enough memory" if we tried to be savvy. The more tight you
> > try to configure the easier to fail that is. Even worse any in kernel
> > memory consumer can increase its memory demand and get the overall
> > consumption off the cliff. So this is not an easy to maintain solution.
> > CMA backed crash memory can be much more generous while still usable.
> 
> Hmm, Redhat could go in a different way. We have been trying to:
> 1) customize initrd for kdump kernel specifically, e.g exclude unneeded
> devices's driver to save memory;

ditto

> 2) monitor device and kenrel memory usage if they begin to consume much
> more memory than before. We have CI testing cases to watch this. We ever
> found one NIC even eat up GB level memory, then this need be
> investigated and fixed.
> With these effort, our default crashkernel values satisfy most of cases,
> surely not call cases. Only rare cases need be handled manually,
> increasing crashkernel.

We get a lot of problems reported by partners testing kdump on
their setups prior to release. But even if we tune the reserved
size up, OOM is still the most common reason for kdump to fail
when the product starts getting used in real life. It's been
pretty frustrating for a long time.

> Wondering how you will use this crashkernel=,cma syntax. On normal
> machines and virt guests, not much meomry is needed, usually 256M or a
> little more is enough. On those high end systems with hundreds of Giga
> bytes, even Tera bytes of memory, I don't think the saved memory with
> crashkernel=,cma make much sense.

I feel the exact opposite about VMs. Reserving hundreds of MB for
crash kernel on _every_ VM on a busy VM host wastes the most
memory. VMs are often tuned to well defined task and can be set
up with very little memory, so the ~256 MB can be a huge part of
that. And while it's theoretically better to dump from the
hypervisor, users still often prefer kdump because the hypervisor
may not be under their control. Also, in a VM it should be much
easier to be sure the machine is safe WRT the potential DMA
corruption as it has less HW drivers. So I actually thought the
CMA reservation could be most useful on VMs.

Thanks,

-- 
Jiri Bohac <jbohac@...e.cz>
SUSE Labs, Prague, Czechia

Powered by blists - more mailing lists