Linux-HyperV List
 help / color / mirror / Atom feed
* Re: [PATCH V4 6/6] x86/Hyper-V: Report crash data in die() when panic_on_oops is set
From: Wei Liu @ 2020-04-09 16:40 UTC (permalink / raw)
  To: Michael Kelley
  Cc: ltykernel@gmail.com, KY Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, hpa@zytor.com, x86@kernel.org, arnd@arndb.de,
	Tianyu Lan, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	vkuznets, Wei Liu
In-Reply-To: <MW2PR2101MB10524CB8CBF3FE49C2EDAA8DD7C00@MW2PR2101MB1052.namprd21.prod.outlook.com>

On Wed, Apr 08, 2020 at 08:19:47PM +0000, Michael Kelley wrote:
> From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Monday, April 6, 2020 8:54 AM
> > 
> > When oops happens with panic_on_oops unset, the oops
> > thread is killed by die() and system continues to run.
> > In such case, guest should not report crash register
> > data to host since system still runs. Check panic_on_oops
> > and return directly in hyperv_report_panic() when the function
> > is called in the die() and panic_on_oops is unset. Fix it.
> > 
> > Fixes: 7ed4325a44ea ("Drivers: hv: vmbus: Make panic reporting to be more useful")
> > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> > ---
> > Change since v3:
> > 	- Fix compile error.
> >         - Add fix commit in the change log
> > ---
> >  arch/x86/hyperv/hv_init.c      | 6 +++++-
> >  drivers/hv/vmbus_drv.c         | 5 +++--
> >  include/asm-generic/mshyperv.h | 2 +-
> >  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> Reviewed-by: Michael Kelley <mikelley@microsoft.com>

It seems to me only the last patch is new, others are already in my
tree, so I only apply the last one.

Let me know if my understanding is wrong.

Wei.

^ permalink raw reply

* Re: [PATCH v2 0/5] Drivers: hv: cleanup VMBus messages handling
From: Wei Liu @ 2020-04-09 16:43 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: linux-hyperv, linux-kernel, Wei Liu, Tianyu Lan, Michael Kelley,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Andrea Parri (Microsoft)
In-Reply-To: <20200406104154.45010-1-vkuznets@redhat.com>

On Mon, Apr 06, 2020 at 12:41:49PM +0200, Vitaly Kuznetsov wrote:
> A small cleanup series mostly aimed at sanitizing memory we pass to
> message handlers: not passing garbage/lefrtovers from other messages
> and making sure we fail early when hypervisor misbehaves.
> 
> No (real) functional change intended.
> 
> Changes since v1:
> - Check that the payload size specified by the host is <= 240 bytes
> - Add Michael's R-b tags.
> 
> Vitaly Kuznetsov (5):
>   Drivers: hv: copy from message page only what's needed
>   Drivers: hv: allocate the exact needed memory for messages
>   Drivers: hv: avoid passing opaque pointer to vmbus_onmessage()
>   Drivers: hv: make sure that 'struct vmbus_channel_message_header'
>     compiles correctly
>   Drivers: hv: check VMBus messages lengths

Queued. Thanks.

Wei.

^ permalink raw reply

* Re: [PATCH 10/28] mm: only allow page table mappings for built-in zsmalloc
From: Peter Zijlstra @ 2020-04-09 16:50 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Christoph Hellwig, Andrew Morton, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, x86, David Airlie, Daniel Vetter,
	Laura Abbott, Sumit Semwal, Sakari Ailus, Nitin Gupta,
	Robin Murphy, Christophe Leroy, linuxppc-dev, linux-hyperv,
	dri-devel, linaro-mm-sig, linux-arch, linux-mm, iommu,
	linux-arm-kernel, linux-s390, bpf, linux-kernel,
	sergey.senozhatsky
In-Reply-To: <20200409160826.GC247701@google.com>

On Thu, Apr 09, 2020 at 09:08:26AM -0700, Minchan Kim wrote:
> On Wed, Apr 08, 2020 at 01:59:08PM +0200, Christoph Hellwig wrote:
> > This allows to unexport map_vm_area and unmap_kernel_range, which are
> > rather deep internal and should not be available to modules.
> 
> Even though I don't know how many usecase we have using zsmalloc as
> module(I heard only once by dumb reason), it could affect existing
> users. Thus, please include concrete explanation in the patch to
> justify when the complain occurs.

The justification is 'we can unexport functions that have no sane reason
of being exported in the first place'.

The Changelog pretty much says that.

^ permalink raw reply

* Re: [PATCH 10/28] mm: only allow page table mappings for built-in zsmalloc
From: Minchan Kim @ 2020-04-09 17:08 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Christoph Hellwig, Andrew Morton, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, x86, David Airlie, Daniel Vetter,
	Laura Abbott, Sumit Semwal, Sakari Ailus, Nitin Gupta,
	Robin Murphy, Christophe Leroy, linuxppc-dev, linux-hyperv,
	dri-devel, linaro-mm-sig, linux-arch, linux-mm, iommu,
	linux-arm-kernel, linux-s390, bpf, linux-kernel,
	sergey.senozhatsky
In-Reply-To: <20200409165030.GG20713@hirez.programming.kicks-ass.net>

On Thu, Apr 09, 2020 at 06:50:30PM +0200, Peter Zijlstra wrote:
> On Thu, Apr 09, 2020 at 09:08:26AM -0700, Minchan Kim wrote:
> > On Wed, Apr 08, 2020 at 01:59:08PM +0200, Christoph Hellwig wrote:
> > > This allows to unexport map_vm_area and unmap_kernel_range, which are
> > > rather deep internal and should not be available to modules.
> > 
> > Even though I don't know how many usecase we have using zsmalloc as
> > module(I heard only once by dumb reason), it could affect existing
> > users. Thus, please include concrete explanation in the patch to
> > justify when the complain occurs.
> 
> The justification is 'we can unexport functions that have no sane reason
> of being exported in the first place'.
> 
> The Changelog pretty much says that.

Okay, I hope there is no affected user since this patch.
If there are someone, they need to provide sane reason why they want
to have zsmalloc as module.

Acked-by: Minchan Kim <minchan@kernel.org>

^ permalink raw reply

* Re: [PATCH 25/28] mm: remove vmalloc_user_node_flags
From: Andrii Nakryiko @ 2020-04-09 22:25 UTC (permalink / raw)
  To: Christoph Hellwig, Johannes Weiner
  Cc: Andrew Morton, K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger,
	Wei Liu, x86, David Airlie, Daniel Vetter, Laura Abbott,
	Sumit Semwal, Sakari Ailus, Minchan Kim, Nitin Gupta,
	Robin Murphy, Christophe Leroy, Peter Zijlstra, linuxppc-dev,
	linux-hyperv, dri-devel, linaro-mm-sig, linux-arch, linux-mm,
	iommu, linux-arm-kernel, linux-s390, bpf, open list
In-Reply-To: <20200408115926.1467567-26-hch@lst.de>

cc Johannes who suggested this API call originally

On Wed, Apr 8, 2020 at 5:03 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Open code it in __bpf_map_area_alloc, which is the only caller.  Also
> clean up __bpf_map_area_alloc to have a single vmalloc call with
> slightly different flags instead of the current two different calls.
>
> For this to compile for the nommu case add a __vmalloc_node_range stub
> to nommu.c.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  include/linux/vmalloc.h |  1 -
>  kernel/bpf/syscall.c    | 23 +++++++++++++----------
>  mm/nommu.c              | 14 ++++++++------
>  mm/vmalloc.c            | 20 --------------------
>  4 files changed, 21 insertions(+), 37 deletions(-)
>
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 108f49b47756..f90f2946aac2 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -106,7 +106,6 @@ extern void *vzalloc(unsigned long size);
>  extern void *vmalloc_user(unsigned long size);
>  extern void *vmalloc_node(unsigned long size, int node);
>  extern void *vzalloc_node(unsigned long size, int node);
> -extern void *vmalloc_user_node_flags(unsigned long size, int node, gfp_t flags);
>  extern void *vmalloc_exec(unsigned long size);
>  extern void *vmalloc_32(unsigned long size);
>  extern void *vmalloc_32_user(unsigned long size);
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 48d98ea8fad6..249d9bd43321 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -281,26 +281,29 @@ static void *__bpf_map_area_alloc(u64 size, int numa_node, bool mmapable)
>          * __GFP_RETRY_MAYFAIL to avoid such situations.
>          */
>
> -       const gfp_t flags = __GFP_NOWARN | __GFP_ZERO;
> +       const gfp_t gfp = __GFP_NOWARN | __GFP_ZERO;
> +       unsigned int flags = 0;
> +       unsigned long align = 1;
>         void *area;
>
>         if (size >= SIZE_MAX)
>                 return NULL;
>
>         /* kmalloc()'ed memory can't be mmap()'ed */
> -       if (!mmapable && size <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER)) {
> -               area = kmalloc_node(size, GFP_USER | __GFP_NORETRY | flags,
> +       if (mmapable) {
> +               BUG_ON(!PAGE_ALIGNED(size));
> +               align = SHMLBA;
> +               flags = VM_USERMAP;
> +       } else if (size <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER)) {
> +               area = kmalloc_node(size, gfp | GFP_USER | __GFP_NORETRY,
>                                     numa_node);
>                 if (area != NULL)
>                         return area;
>         }
> -       if (mmapable) {
> -               BUG_ON(!PAGE_ALIGNED(size));
> -               return vmalloc_user_node_flags(size, numa_node, GFP_KERNEL |
> -                                              __GFP_RETRY_MAYFAIL | flags);
> -       }
> -       return __vmalloc_node(size, 1, GFP_KERNEL | __GFP_RETRY_MAYFAIL | flags,
> -                             numa_node, __builtin_return_address(0));
> +
> +       return __vmalloc_node_range(size, align, VMALLOC_START, VMALLOC_END,
> +                       gfp | GFP_KERNEL | __GFP_RETRY_MAYFAIL, PAGE_KERNEL,
> +                       flags, numa_node, __builtin_return_address(0));
>  }
>
>  void *bpf_map_area_alloc(u64 size, int numa_node)
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 81a86cd85893..b42cd6003d7d 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -150,6 +150,14 @@ void *__vmalloc(unsigned long size, gfp_t gfp_mask)
>  }
>  EXPORT_SYMBOL(__vmalloc);
>
> +void *__vmalloc_node_range(unsigned long size, unsigned long align,
> +               unsigned long start, unsigned long end, gfp_t gfp_mask,
> +               pgprot_t prot, unsigned long vm_flags, int node,
> +               const void *caller)
> +{
> +       return __vmalloc(size, flags);
> +}
> +
>  void *__vmalloc_node(unsigned long size, unsigned long align, gfp_t gfp_mask,
>                 int node, const void *caller)
>  {
> @@ -180,12 +188,6 @@ void *vmalloc_user(unsigned long size)
>  }
>  EXPORT_SYMBOL(vmalloc_user);
>
> -void *vmalloc_user_node_flags(unsigned long size, int node, gfp_t flags)
> -{
> -       return __vmalloc_user_flags(size, flags | __GFP_ZERO);
> -}
> -EXPORT_SYMBOL(vmalloc_user_node_flags);
> -
>  struct page *vmalloc_to_page(const void *addr)
>  {
>         return virt_to_page(addr);
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 333fbe77255a..f6f2acdaf70c 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2658,26 +2658,6 @@ void *vzalloc_node(unsigned long size, int node)
>  }
>  EXPORT_SYMBOL(vzalloc_node);
>
> -/**
> - * vmalloc_user_node_flags - allocate memory for userspace on a specific node
> - * @size: allocation size
> - * @node: numa node
> - * @flags: flags for the page level allocator
> - *
> - * The resulting memory area is zeroed so it can be mapped to userspace
> - * without leaking data.
> - *
> - * Return: pointer to the allocated memory or %NULL on error
> - */
> -void *vmalloc_user_node_flags(unsigned long size, int node, gfp_t flags)
> -{
> -       return __vmalloc_node_range(size, SHMLBA,  VMALLOC_START, VMALLOC_END,
> -                                   flags | __GFP_ZERO, PAGE_KERNEL,
> -                                   VM_USERMAP, node,
> -                                   __builtin_return_address(0));
> -}
> -EXPORT_SYMBOL(vmalloc_user_node_flags);
> -
>  /**
>   * vmalloc_exec - allocate virtually contiguous, executable memory
>   * @size:        allocation size
> --
> 2.25.1
>

^ permalink raw reply

* Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
From: Benjamin Herrenschmidt @ 2020-04-09 22:56 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Christoph Hellwig, Andrew Morton, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, X86 ML, David Airlie, Laura Abbott,
	Sumit Semwal, Sakari Ailus, Minchan Kim, Nitin Gupta,
	Robin Murphy, Christophe Leroy, Peter Zijlstra, linuxppc-dev,
	linux-hyperv, dri-devel,
	moderated list:DMA BUFFER SHARING FRAMEWORK,
	open list:GENERIC INCLUDE/A..., Linux MM,
	list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg  Roedel <joro@8bytes.org>,,
	Linux ARM, linux-s390, bpf, Linux Kernel Mailing List
In-Reply-To: <CAKMK7uHtkLvdsWFGiAtkzVa5mpnDvXkn3CHZQ6bgJ_enbyAc8A@mail.gmail.com>

On Thu, 2020-04-09 at 11:41 +0200, Daniel Vetter wrote:
> Now if these boxes didn't ever have agp then I think we can get away
> with deleting this, since we've already deleted the legacy radeon
> driver. And that one used vmalloc for everything. The new kms one does
> use the dma-api if the gpu isn't connected through agp

Definitely no AGP there.

Cheers
Ben.



^ permalink raw reply

* RE: [PATCH V4 6/6] x86/Hyper-V: Report crash data in die() when panic_on_oops is set
From: Michael Kelley @ 2020-04-09 23:06 UTC (permalink / raw)
  To: Wei Liu
  Cc: ltykernel@gmail.com, KY Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, hpa@zytor.com, x86@kernel.org, arnd@arndb.de,
	Tianyu Lan, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	vkuznets
In-Reply-To: <20200409164054.72es2ykmbef3jbui@debian>

From: Wei Liu <wei.liu@kernel.org>  Sent: Thursday, April 9, 2020 9:41 AM
> 
> On Wed, Apr 08, 2020 at 08:19:47PM +0000, Michael Kelley wrote:
> > From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Monday, April 6, 2020 8:54 AM
> > >
> > > When oops happens with panic_on_oops unset, the oops
> > > thread is killed by die() and system continues to run.
> > > In such case, guest should not report crash register
> > > data to host since system still runs. Check panic_on_oops
> > > and return directly in hyperv_report_panic() when the function
> > > is called in the die() and panic_on_oops is unset. Fix it.
> > >
> > > Fixes: 7ed4325a44ea ("Drivers: hv: vmbus: Make panic reporting to be more useful")
> > > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> > > ---
> > > Change since v3:
> > > 	- Fix compile error.
> > >         - Add fix commit in the change log
> > > ---
> > >  arch/x86/hyperv/hv_init.c      | 6 +++++-
> > >  drivers/hv/vmbus_drv.c         | 5 +++--
> > >  include/asm-generic/mshyperv.h | 2 +-
> > >  3 files changed, 9 insertions(+), 4 deletions(-)
> >
> > Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> 
> It seems to me only the last patch is new, others are already in my
> tree, so I only apply the last one.
> 
> Let me know if my understanding is wrong.
> 

Tianyu added "Fixes:" tags to some of the other patches in
the series.  It appears the version you had already queued doesn't
have those "Fixes:" tags.

Michael

^ permalink raw reply

* Re: [PATCH 10/28] mm: only allow page table mappings for built-in zsmalloc
From: Sergey Senozhatsky @ 2020-04-10  2:38 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Peter Zijlstra, Christoph Hellwig, Andrew Morton,
	K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu, x86,
	David Airlie, Daniel Vetter, Laura Abbott, Sumit Semwal,
	Sakari Ailus, Nitin Gupta, Robin Murphy, Christophe Leroy,
	linuxppc-dev, linux-hyperv, dri-devel, linaro-mm-sig, linux-arch,
	linux-mm, iommu, linux-arm-kernel, linux-s390, bpf, linux-kernel,
	sergey.senozhatsky
In-Reply-To: <20200409170813.GD247701@google.com>

On (20/04/09 10:08), Minchan Kim wrote:
> > > Even though I don't know how many usecase we have using zsmalloc as
> > > module(I heard only once by dumb reason), it could affect existing
> > > users. Thus, please include concrete explanation in the patch to
> > > justify when the complain occurs.
> > 
> > The justification is 'we can unexport functions that have no sane reason
> > of being exported in the first place'.
> > 
> > The Changelog pretty much says that.
> 
> Okay, I hope there is no affected user since this patch.
> If there are someone, they need to provide sane reason why they want
> to have zsmalloc as module.

I'm one of those who use zsmalloc as a module - mainly because I use zram
as a compressing general purpose block device, not as a swap device.
I create zram0, mkfs, mount, checkout and compile code, once done -
umount, rmmod. This reduces the number of writes to SSD. Some people use
tmpfs, but zram device(-s) can be much larger in size. That's a niche use
case and I'm not against the patch.

	-ss

^ permalink raw reply

* SCSI low level driver: how to prevent I/O upon hibernation?
From: Dexuan Cui @ 2020-04-10  5:44 UTC (permalink / raw)
  To: Martin K. Petersen, linux-scsi@vger.kernel.org, Christoph Hellwig,
	linux-hyperv@vger.kernel.org, Ming Lei, Long Li, vkuznets
  Cc: Michael Kelley, KY Srinivasan, Olaf Hering, Stephen Hemminger,
	linux-kernel@vger.kernel.org

Hi all,
Can you please recommend the standard way to prevent the upper layer SCSI
driver from submitting new I/O requests when the system is doing hibernation?

Actually I already asked the question on 5/30 last year:
https://marc.info/?l=linux-scsi&m=155918927116283&w=2
and I thought all the sdevs are suspended and resumed automatically in
drivers/scsi/scsi_pm.c, and the low level SCSI adapter driver (i.e. hv_storvsc)
only needs to suspend/resume the state of the adapter itself. However, it looks
this is not true, because today I got such a panic in a v5.6 Linux VM running on
Hyper-V: the 'suspend' part of the hibernation process finished without any
issue, but when the VM was trying to resume back from the 'new' kernel to the
'old' kernel, these events happened:

1. the new kernel loaded the saved state from disk to memory.

2. the new kernel quiesced the devices, including the SCSI DVD device
controlled by the hv_storvsc low level SCSI driver, i.e.
drivers/scsi/storvsc_drv.c: storvsc_suspend() was called and the related vmbus
ringbuffer was freed.

3. However, disk_events_workfn() -> ... -> cdrom_check_events() -> ...
   -> scsi_queue_rq() -> ... -> storvsc_queuecommand() was still trying to
submit I/O commands to the freed vmbus ringbuffer, and as a result, a NULL
pointer dereference panic happened.

Can anyone please explain the symptom?

I made the below patch, with which it seems I can no longer reproduce the panic.

But I don't know how scsi_block_requests() can reliably prevent new I/O requests
from being issued concurrently on a different CPU -- the function only sets a
flag?

Looking forward to your insights!

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index fb41636519ee..dcfb0a820977 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1949,6 +1949,8 @@ static int storvsc_suspend(struct hv_device *hv_dev)
        struct Scsi_Host *host = stor_device->host;
        struct hv_host_device *host_dev = shost_priv(host);

+       scsi_block_requests(host);
+
        storvsc_wait_to_drain(stor_device);

        drain_workqueue(host_dev->handle_error_wq);
@@ -1968,10 +1970,14 @@ static int storvsc_suspend(struct hv_device *hv_dev)

 static int storvsc_resume(struct hv_device *hv_dev)
 {
+       struct storvsc_device *stor_device = hv_get_drvdata(hv_dev);
+       struct Scsi_Host *host = stor_device->host;
        int ret;

        ret = storvsc_connect_to_vsp(hv_dev, storvsc_ringbuffer_size,
                                     hv_dev_is_fc(hv_dev));
+       if (!ret)
+               scsi_unblock_requests(host);
        return ret;
 }


This is the log of the panic:

[    8.565615] PM: Adding info for No Bus:vcsa63
[    8.590118] Freezing user space processes ... (elapsed 0.020 seconds) done.
[    8.619143] OOM killer disabled.
[    8.645914] PM: Using 3 thread(s) for decompression
[    8.650805] PM: Loading and decompressing image data (307144 pages)...
[    8.693765] PM: Image loading progress:   0%
[    9.286720] PM: Image loading progress:  10%
[    9.541665] PM: Image loading progress:  20%
[    9.777528] PM: Image loading progress:  30%
[   10.062504] PM: Image loading progress:  40%
[   10.317178] PM: Image loading progress:  50%
[   10.588564] PM: Image loading progress:  60%
[   10.796801] PM: Image loading progress:  70%
[   11.029323] PM: Image loading progress:  80%
[   11.327868] PM: Image loading progress:  90%
[   11.650745] PM: Image loading progress: 100%
[   11.655851] PM: Image loading done
[   11.659596] PM: hibernation: Read 1228576 kbytes in 2.99 seconds (410.89 MB/s)
[   11.668702] input input1: type quiesce
[   11.668741] sr 0:0:0:1: bus quiesce
[   11.668804] sd 0:0:0:0: bus quiesce
[   11.672970] input input0: type quiesce
[   11.698082] scsi target0:0:0: bus quiesce
[   11.703296] scsi host0: bus quiesce
[   11.707448] alarmtimer alarmtimer.0.auto: bus quiesce
[   11.712782] rtc rtc0: class quiesce
[   11.716560] platform Fixed MDIO bus.0: bus quiesce
[   11.721911] serial8250 serial8250: bus quiesce
[   11.727220] simple-framebuffer simple-framebuffer.0: bus quiesce
[   11.734066] platform pcspkr: bus quiesce
[   11.738726] rtc_cmos 00:02: bus quiesce
[   11.743353] serial 00:01: bus quiesce
[   11.747433] serial 00:00: bus quiesce
[   11.751654] platform efivars.0: bus quiesce
[   11.756316] platform rtc-efi.0: bus quiesce
[   11.760883] platform HYPER_V_GEN_COUN:00: bus quiesce
[   11.766255] platform VMBUS:00: bus quiesce
[   11.770668] platform PNP0003:00: bus quiesce
[   11.781730] hv_storvsc bf78936f-7d8f-45ce-ab03-6c341452e55d: noirq bus quiesce
[   11.796479] hv_netvsc dda5a2be-b8b8-4237-b330-be8a516a72c0: noirq bus quiesce
[   11.804042] BUG: kernel NULL pointer dereference, address: 0000000000000090
[   11.804996] #PF: supervisor read access in kernel mode
[   11.804996] #PF: error_code(0x0000) - not-present page
[   11.804996] PGD 0 P4D 0
[   11.804996] Oops: 0000 [#1] SMP PTI
[   11.804996] CPU: 18 PID: 353 Comm: kworker/18:1 Not tainted 5.6.0+ #1
[   11.804996] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v4.0 05/16/2019
[   11.804996] Workqueue: events_freezable_power_ disk_events_workfn
[   11.804996] RIP: 0010:storvsc_queuecommand+0x261/0x714 [hv_storvsc]
[   11.804996] Code: ...
[   11.804996] RSP: 0018:ffffa331c2347af0 EFLAGS: 00010246
[   11.804996] RAX: 0000000000000000 RBX: ffff8e6a32cec5a0 RCX: 0000000000000000
[   11.804996] RDX: 0000000000000012 RSI: ffff8e6a32cec3e0 RDI: ffff8e6a32cec710
[   11.804996] RBP: ffff8e6b6d58c800 R08: 0000000000000010 R09: ffff8e6a32aa6060
[   11.804996] R10: ffffc8a8c4c81980 R11: 0000000000000000 R12: 0000000000000012
[   11.804996] R13: 0000000000000012 R14: ffff8e6a32cec710 R15: ffff8e6a32cec3d8
[   11.804996] FS:  0000000000000000(0000) GS:ffff8e6a42c80000(0000) knlGS:0000000000000000
[   11.804996] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   11.804996] CR2: 0000000000000090 CR3: 000000013a428006 CR4: 00000000003606e0
[   11.804996] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   11.804996] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   11.804996] Call Trace:
[   11.804996]  scsi_queue_rq+0x593/0xa10
[   11.804996]  blk_mq_dispatch_rq_list+0x8d/0x510
[   11.804996]  blk_mq_sched_dispatch_requests+0xed/0x170
[   11.804996]  __blk_mq_run_hw_queue+0x55/0x110
[   11.804996]  __blk_mq_delay_run_hw_queue+0x141/0x160
[   11.804996]  blk_mq_sched_insert_request+0xc3/0x170
[   11.804996]  blk_execute_rq+0x4b/0xa0
[   11.804996]  __scsi_execute+0xeb/0x250
[   11.804996]  sr_check_events+0x9f/0x270 [sr_mod]
[   11.804996]  cdrom_check_events+0x1a/0x30 [cdrom]
[   11.804996]  sr_block_check_events+0xcc/0x110 [sr_mod]
[   11.804996]  disk_check_events+0x68/0x160
[   11.804996]  process_one_work+0x20c/0x3d0
[   11.804996]  worker_thread+0x2d/0x3e0
[   11.804996]  kthread+0x10c/0x130
[   11.804996]  ret_from_fork+0x35/0x40

Thanks,
-- Dexuan

^ permalink raw reply related

* Re: SCSI low level driver: how to prevent I/O upon hibernation?
From: Ming Lei @ 2020-04-10  7:42 UTC (permalink / raw)
  To: Dexuan Cui
  Cc: Martin K. Petersen, linux-scsi@vger.kernel.org, Christoph Hellwig,
	linux-hyperv@vger.kernel.org, Long Li, vkuznets, Michael Kelley,
	KY Srinivasan, Olaf Hering, Stephen Hemminger,
	linux-kernel@vger.kernel.org
In-Reply-To: <HK0P153MB0273A1B109CE3A33F0984D34BFDE0@HK0P153MB0273.APCP153.PROD.OUTLOOK.COM>

Hello Dexuan,

On Fri, Apr 10, 2020 at 1:44 PM Dexuan Cui <decui@microsoft.com> wrote:
>
> Hi all,
> Can you please recommend the standard way to prevent the upper layer SCSI
> driver from submitting new I/O requests when the system is doing hibernation?
>
> Actually I already asked the question on 5/30 last year:
> https://marc.info/?l=linux-scsi&m=155918927116283&w=2
> and I thought all the sdevs are suspended and resumed automatically in
> drivers/scsi/scsi_pm.c, and the low level SCSI adapter driver (i.e. hv_storvsc)
> only needs to suspend/resume the state of the adapter itself. However, it looks
> this is not true, because today I got such a panic in a v5.6 Linux VM running on
> Hyper-V: the 'suspend' part of the hibernation process finished without any
> issue, but when the VM was trying to resume back from the 'new' kernel to the
> 'old' kernel, these events happened:
>
> 1. the new kernel loaded the saved state from disk to memory.
>
> 2. the new kernel quiesced the devices, including the SCSI DVD device
> controlled by the hv_storvsc low level SCSI driver, i.e.
> drivers/scsi/storvsc_drv.c: storvsc_suspend() was called and the related vmbus
> ringbuffer was freed.
>
> 3. However, disk_events_workfn() -> ... -> cdrom_check_events() -> ...
>    -> scsi_queue_rq() -> ... -> storvsc_queuecommand() was still trying to
> submit I/O commands to the freed vmbus ringbuffer, and as a result, a NULL
> pointer dereference panic happened.

Last time I replied to you in above link:

"scsi_device_quiesce() has been called by scsi_dev_type_suspend() to prevent
any non-pm request from entering queue."

That meant no any normal FS request can enter scsi queue after suspend, however
request with BLK_MQ_REQ_PREEMPT is still allowed to be queued to LLD
after suspend.

So you can't free related vmbus ringbuffer cause  BLK_MQ_REQ_PREEMPT request
is still to be handled.

Thanks,
Ming Lei

^ permalink raw reply

* Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
From: Daniel Vetter @ 2020-04-10  8:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Christoph Hellwig, Andrew Morton, K. Y. Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, X86 ML, David Airlie, Laura Abbott,
	Sumit Semwal, Sakari Ailus, Minchan Kim, Nitin Gupta,
	Robin Murphy, Christophe Leroy, Peter Zijlstra, linuxppc-dev,
	linux-hyperv, dri-devel,
	moderated list:DMA BUFFER SHARING FRAMEWORK,
	open list:GENERIC INCLUDE/A..., Linux MM,
	list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>,,
	Linux ARM, linux-s390, bpf, Linux Kernel Mailing List
In-Reply-To: <0f360b9cb72b80bae0d0db8150f65598c2776268.camel@kernel.crashing.org>

On Fri, Apr 10, 2020 at 12:57 AM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> On Thu, 2020-04-09 at 11:41 +0200, Daniel Vetter wrote:
> > Now if these boxes didn't ever have agp then I think we can get away
> > with deleting this, since we've already deleted the legacy radeon
> > driver. And that one used vmalloc for everything. The new kms one does
> > use the dma-api if the gpu isn't connected through agp
>
> Definitely no AGP there.

Ah in that case I think we can be sure that this code is dead.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply

* Re: [PATCH V4 6/6] x86/Hyper-V: Report crash data in die() when panic_on_oops is set
From: Wei Liu @ 2020-04-10 14:27 UTC (permalink / raw)
  To: Michael Kelley
  Cc: Wei Liu, ltykernel@gmail.com, KY Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, hpa@zytor.com, x86@kernel.org, arnd@arndb.de,
	Tianyu Lan, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	vkuznets
In-Reply-To: <MW2PR2101MB10525BA673D7FA81073B9F65D7C10@MW2PR2101MB1052.namprd21.prod.outlook.com>

On Thu, Apr 09, 2020 at 11:06:13PM +0000, Michael Kelley wrote:
> From: Wei Liu <wei.liu@kernel.org>  Sent: Thursday, April 9, 2020 9:41 AM
> > 
> > On Wed, Apr 08, 2020 at 08:19:47PM +0000, Michael Kelley wrote:
> > > From: Tianyu Lan <Tianyu.Lan@microsoft.com> Sent: Monday, April 6, 2020 8:54 AM
> > > >
> > > > When oops happens with panic_on_oops unset, the oops
> > > > thread is killed by die() and system continues to run.
> > > > In such case, guest should not report crash register
> > > > data to host since system still runs. Check panic_on_oops
> > > > and return directly in hyperv_report_panic() when the function
> > > > is called in the die() and panic_on_oops is unset. Fix it.
> > > >
> > > > Fixes: 7ed4325a44ea ("Drivers: hv: vmbus: Make panic reporting to be more useful")
> > > > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
> > > > ---
> > > > Change since v3:
> > > > 	- Fix compile error.
> > > >         - Add fix commit in the change log
> > > > ---
> > > >  arch/x86/hyperv/hv_init.c      | 6 +++++-
> > > >  drivers/hv/vmbus_drv.c         | 5 +++--
> > > >  include/asm-generic/mshyperv.h | 2 +-
> > > >  3 files changed, 9 insertions(+), 4 deletions(-)
> > >
> > > Reviewed-by: Michael Kelley <mikelley@microsoft.com>
> > 
> > It seems to me only the last patch is new, others are already in my
> > tree, so I only apply the last one.
> > 
> > Let me know if my understanding is wrong.
> > 
> 
> Tianyu added "Fixes:" tags to some of the other patches in
> the series.  It appears the version you had already queued doesn't
> have those "Fixes:" tags.

Oh, I only read the subject lines and checked the Reviewed-by tags. I
will queue the new series instead.

Wei.

> 
> Michael

^ permalink raw reply

* RE: [PATCH 01/11] Drivers: hv: vmbus: Always handle the VMBus messages on CPU0
From: Michael Kelley @ 2020-04-10 17:18 UTC (permalink / raw)
  To: Andrea Parri (Microsoft), linux-kernel@vger.kernel.org
  Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
	linux-hyperv@vger.kernel.org, Dexuan Cui, Boqun Feng, vkuznets
In-Reply-To: <20200406001514.19876-2-parri.andrea@gmail.com>

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Sunday, April 5, 2020 5:15 PM
> 
> A Linux guest have to pick a "connect CPU" to communicate with the
> Hyper-V host.  This CPU can not be taken offline because Hyper-V does
> not provide a way to change that CPU assignment.
> 
> Current code sets the connect CPU to whatever CPU ends up running the
> function vmbus_negotiate_version(), and this will generate problems if
> that CPU is taken offine.
> 
> Establish CPU0 as the connect CPU, and add logics to prevents the
> connect CPU from being taken offline.   We could pick some other CPU,
> and we could pick that "other CPU" dynamically if there was a reason to
> do so at some point in the future.  But for now, #defining the connect
> CPU to 0 is the most straightforward and least complex solution.
> 
> While on this, add inline comments explaining "why" offer and rescind
> messages should not be handled by a same serialized work queue.
> 
> Suggested-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  drivers/hv/connection.c   | 20 +-------------------
>  drivers/hv/hv.c           |  7 +++++++
>  drivers/hv/hyperv_vmbus.h | 11 ++++++-----
>  drivers/hv/vmbus_drv.c    | 20 +++++++++++++++++---
>  4 files changed, 31 insertions(+), 27 deletions(-)

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply

* RE: [PATCH 02/11] Drivers: hv: vmbus: Don't bind the offer&rescind works to a specific CPU
From: Michael Kelley @ 2020-04-10 17:23 UTC (permalink / raw)
  To: Andrea Parri (Microsoft), linux-kernel@vger.kernel.org
  Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
	linux-hyperv@vger.kernel.org, Dexuan Cui, Boqun Feng, vkuznets
In-Reply-To: <20200406001514.19876-3-parri.andrea@gmail.com>

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Sunday, April 5, 2020 5:15 PM
> 
> The offer and rescind works are currently scheduled on the so called
> "connect CPU".  However, this is not really needed: we can synchronize
> the works by relying on the usage of the offer_in_progress counter and
> of the channel_mutex mutex.  This synchronization is already in place.
> So, remove this unnecessary "bind to the connect CPU" constraint and
> update the inline comments accordingly.
> 
> Suggested-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
>  drivers/hv/channel_mgmt.c | 21 ++++++++++++++++-----
>  drivers/hv/vmbus_drv.c    | 39 ++++++++++++++++++++++++++++-----------
>  2 files changed, 44 insertions(+), 16 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply

* RE: [PATCH 03/11] Drivers: hv: vmbus: Replace the per-CPU channel lists with a global array of channels
From: Michael Kelley @ 2020-04-10 17:29 UTC (permalink / raw)
  To: Andrea Parri (Microsoft), linux-kernel@vger.kernel.org
  Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
	linux-hyperv@vger.kernel.org, Dexuan Cui, Boqun Feng, vkuznets
In-Reply-To: <20200406001514.19876-4-parri.andrea@gmail.com>

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Sunday, April 5, 2020 5:15 PM
> 
> When Hyper-V sends an interrupt to the guest, the guest has to figure
> out which channel the interrupt is associated with.  Hyper-V sets a bit
> in a memory page that is shared with the guest, indicating a particular
> "relid" that the interrupt is associated with.  The current Linux code
> then uses a set of per-CPU linked lists to map a given "relid" to a
> pointer to a channel structure.
> 
> This design introduces a synchronization problem if the CPU that Hyper-V
> will interrupt for a certain channel is changed.  If the interrupt comes
> on the "old CPU" and the channel was already moved to the per-CPU list
> of the "new CPU", then the relid -> channel mapping will fail and the
> interrupt is dropped.  Similarly, if the interrupt comes on the new CPU
> but the channel was not moved to the per-CPU list of the new CPU, then
> the mapping will fail and the interrupt is dropped.
> 
> Relids are integers ranging from 0 to 2047.  The mapping from relids to
> channel structures can be done by setting up an array with 2048 entries,
> each entry being a pointer to a channel structure (hence total size ~16K
> bytes, which is not a problem).  The array is global, so there are no
> per-CPU linked lists to update.  The array can be searched and updated
> by loading from/storing to the array at the specified index.  With no
> per-CPU data structures, the above mentioned synchronization problem is
> avoided and the relid2channel() function gets simpler.
> 
> Suggested-by: Michael Kelley <mikelley@microsoft.com>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
> NOTE.  The inline comment in vmbus_channel_map_relid() follows the argument
> discussed in the RFC [1].  An attempt has been made to make this argument
> more 'explicit' (and, hopefully, more 'robust' against future changes) by
> adding a full memory barrier in vmbus_channel_map_relid(): the barrier takes
> the role of the 'implicit' full memory barriers from queue_work() and from
> check_ready_for_resume_event() referred to in the RFC.
> 
> [1] https://lore.kernel.org/linux-hyperv/20200403133826.GA25401@andrea/
> 
>  drivers/hv/channel_mgmt.c | 179 ++++++++++++++++++++++++--------------
>  drivers/hv/connection.c   |  38 +++-----
>  drivers/hv/hv.c           |   2 -
>  drivers/hv/hyperv_vmbus.h |  14 +--
>  drivers/hv/vmbus_drv.c    |  48 ++++++----
>  include/linux/hyperv.h    |   5 --
>  6 files changed, 160 insertions(+), 126 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply

* RE: [PATCH 04/11] hv_netvsc: Disable NAPI before closing the VMBus channel
From: Michael Kelley @ 2020-04-10 19:20 UTC (permalink / raw)
  To: Andrea Parri (Microsoft), linux-kernel@vger.kernel.org
  Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
	linux-hyperv@vger.kernel.org, Dexuan Cui, Boqun Feng, vkuznets,
	Stephen Hemminger, David S. Miller, netdev@vger.kernel.org
In-Reply-To: <20200406001514.19876-5-parri.andrea@gmail.com>

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Sunday, April 5, 2020 5:15 PM
> 
> vmbus_chan_sched() might call the netvsc driver callback function that
> ends up scheduling NAPI work.  This "work" can access the channel ring
> buffer, so we must ensure that any such work is completed and that the
> ring buffer is no longer being accessed before freeing the ring buffer
> data structure in the channel closure path.  To this end, disable NAPI
> before calling vmbus_close() in netvsc_device_remove().
> 
> Suggested-by: Michael Kelley <mikelley@microsoft.com>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: <netdev@vger.kernel.org>
> ---
>  drivers/hv/channel.c        | 6 ++++++
>  drivers/net/hyperv/netvsc.c | 7 +++++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply

* RE: [PATCH 05/11] hv_utils: Always execute the fcopy and vss callbacks in a tasklet
From: Michael Kelley @ 2020-04-10 19:26 UTC (permalink / raw)
  To: Andrea Parri (Microsoft), linux-kernel@vger.kernel.org
  Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
	linux-hyperv@vger.kernel.org, Dexuan Cui, Boqun Feng, vkuznets
In-Reply-To: <20200406001514.19876-6-parri.andrea@gmail.com>

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Sunday, April 5, 2020 5:15 PM
> 
> The fcopy and vss callback functions could be running in a tasklet
> at the same time they are called in hv_poll_channel().  Current code
> serializes the invocations of these functions, and their accesses to
> the channel ring buffer, by sending an IPI to the CPU that is allowed
> to access the ring buffer, cf. hv_poll_channel().  This IPI mechanism
> becomes infeasible if we allow changing the CPU that a channel will
> interrupt.  Instead modify the callback wrappers to always execute
> the fcopy and vss callbacks in a tasklet, thus mirroring the solution
> for the kvp callback functions adopted since commit a3ade8cc474d8
> ("HV: properly delay KVP packets when negotiation is in progress").
> This will ensure that the callback function can't run on two CPUs at
> the same time.
> 
> Suggested-by: Michael Kelley <mikelley@microsoft.com>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
>  drivers/hv/hv_fcopy.c     | 2 +-
>  drivers/hv/hv_snapshot.c  | 2 +-
>  drivers/hv/hyperv_vmbus.h | 7 +------
>  3 files changed, 3 insertions(+), 8 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply

* RE: [PATCH 06/11] Drivers: hv: vmbus: Use a spin lock for synchronizing channel scheduling vs. channel removal
From: Michael Kelley @ 2020-04-10 19:28 UTC (permalink / raw)
  To: Andrea Parri (Microsoft), linux-kernel@vger.kernel.org
  Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
	linux-hyperv@vger.kernel.org, Dexuan Cui, Boqun Feng, vkuznets
In-Reply-To: <20200406001514.19876-7-parri.andrea@gmail.com>

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Sunday, April 5, 2020 5:15 PM
> 
> Since vmbus_chan_sched() dereferences the ring buffer pointer, we have
> to make sure that the ring buffer data structures don't get freed while
> such dereferencing is happening.  Current code does this by sending an
> IPI to the CPU that is allowed to access that ring buffer from interrupt
> level, cf., vmbus_reset_channel_cb().  But with the new functionality
> to allow changing the CPU that a channel will interrupt, we can't be
> sure what CPU will be running the vmbus_chan_sched() function for a
> particular channel, so the current IPI mechanism is infeasible.
> 
> Instead synchronize vmbus_chan_sched() and vmbus_reset_channel_cb() by
> using the (newly introduced) per-channel spin lock "sched_lock".  Move
> the test for onchannel_callback being NULL before the "switch" control
> statement in vmbus_chan_sched(), in order to not access the ring buffer
> if the vmbus_reset_channel_cb() has been completed on the channel.
> 
> Suggested-by: Michael Kelley <mikelley@microsoft.com>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
>  drivers/hv/channel.c      | 24 +++++++-----------------
>  drivers/hv/channel_mgmt.c |  1 +
>  drivers/hv/vmbus_drv.c    | 30 +++++++++++++++++-------------
>  include/linux/hyperv.h    |  6 ++++++
>  4 files changed, 31 insertions(+), 30 deletions(-)
> 

Reviewed-by:  Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply

* RE: [PATCH 07/11] PCI: hv: Prepare hv_compose_msi_msg() for the VMBus-channel-interrupt-to-vCPU reassignment functionality
From: Michael Kelley @ 2020-04-10 19:30 UTC (permalink / raw)
  To: Andrea Parri (Microsoft), linux-kernel@vger.kernel.org
  Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
	linux-hyperv@vger.kernel.org, Dexuan Cui, Boqun Feng, vkuznets,
	Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas,
	linux-pci@vger.kernel.org
In-Reply-To: <20200406001514.19876-8-parri.andrea@gmail.com>

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Sunday, April 5, 2020 5:15 PM
> 
> The current implementation of hv_compose_msi_msg() is incompatible with
> the new functionality that allows changing the vCPU a VMBus channel will
> interrupt: if this function always calls hv_pci_onchannelcallback() in
> the polling loop, the interrupt going to a different CPU could cause
> hv_pci_onchannelcallback() to be running simultaneously in a tasklet,
> which will break.  The current code also has a problem in that it is not
> synchronized with vmbus_reset_channel_cb(): hv_compose_msi_msg() could
> be accessing the ring buffer via the call of hv_pci_onchannelcallback()
> well after the time that vmbus_reset_channel_cb() has finished.
> 
> Fix these issues as follows.  Disable the channel tasklet before
> entering the polling loop in hv_compose_msi_msg() and re-enable it when
> done.  This will prevent hv_pci_onchannelcallback() from running in a
> tasklet on a different CPU.  Moreover, poll by always calling
> hv_pci_onchannelcallback(), but check the channel callback function for
> NULL and invoke the callback within a sched_lock critical section.  This
> will prevent hv_compose_msi_msg() from accessing the ring buffer after
> vmbus_reset_channel_cb() has acquired the sched_lock spinlock.
> 
> Suggested-by: Michael Kelley <mikelley@microsoft.com>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Andrew Murray <amurray@thegoodpenguin.co.uk>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: <linux-pci@vger.kernel.org>
> ---
>  drivers/pci/controller/pci-hyperv.c | 44 ++++++++++++++++++-----------
>  1 file changed, 28 insertions(+), 16 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply

* RE: [PATCH 08/11] Drivers: hv: vmbus: Remove the unused HV_LOCALIZED channel affinity logic
From: Michael Kelley @ 2020-04-10 19:32 UTC (permalink / raw)
  To: Andrea Parri (Microsoft), linux-kernel@vger.kernel.org
  Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
	linux-hyperv@vger.kernel.org, Dexuan Cui, Boqun Feng, vkuznets
In-Reply-To: <20200406001514.19876-9-parri.andrea@gmail.com>

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Sunday, April 5, 2020 5:15 PM
> 
> The logic is unused since commit 509879bdb30b8 ("Drivers: hv: Introduce
> a policy for controlling channel affinity").
> 
> This logic assumes that a channel target_cpu doesn't change during the
> lifetime of a channel, but this assumption is incompatible with the new
> functionality that allows changing the vCPU a channel will interrupt.
> 
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
>  drivers/hv/channel_mgmt.c | 105 +++++++++-----------------------------
>  include/linux/hyperv.h    |  27 ----------
>  2 files changed, 25 insertions(+), 107 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply

* RE: [PATCH 09/11] Drivers: hv: vmbus: Synchronize init_vp_index() vs. CPU hotplug
From: Michael Kelley @ 2020-04-10 19:33 UTC (permalink / raw)
  To: Andrea Parri (Microsoft), linux-kernel@vger.kernel.org
  Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
	linux-hyperv@vger.kernel.org, Dexuan Cui, Boqun Feng, vkuznets
In-Reply-To: <20200406001514.19876-10-parri.andrea@gmail.com>

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Sunday, April 5, 2020 5:15 PM
> 
> init_vp_index() may access the cpu_online_mask mask via its calls of
> cpumask_of_node().  Make sure to protect these accesses with a
> cpus_read_lock() critical section.
> 
> Also, remove some (hardcoded) instances of CPU(0) from init_vp_index()
> and replace them with VMBUS_CONNECT_CPU.  The connect CPU can not go
> offline, since Hyper-V does not provide a way to change it.
> 
> Finally, order the accesses of target_cpu from init_vp_index() and
> hv_synic_cleanup() by relying on the channel_mutex; this is achieved
> by moving the call of init_vp_index() into vmbus_process_offer().
> 
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
>  drivers/hv/channel_mgmt.c | 47 ++++++++++++++++++++++++++++-----------
>  drivers/hv/hv.c           |  7 +++---
>  2 files changed, 38 insertions(+), 16 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply

* RE: [PATCH 10/11] Drivers: hv: vmbus: Introduce the CHANNELMSG_MODIFYCHANNEL message type
From: Michael Kelley @ 2020-04-10 19:34 UTC (permalink / raw)
  To: Andrea Parri (Microsoft), linux-kernel@vger.kernel.org
  Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
	linux-hyperv@vger.kernel.org, Dexuan Cui, Boqun Feng, vkuznets
In-Reply-To: <20200406001514.19876-11-parri.andrea@gmail.com>

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Sunday, April 5, 2020 5:15 PM
> 
> VMBus version 4.1 and later support the CHANNELMSG_MODIFYCHANNEL(22)
> message type which can be used to request Hyper-V to change the vCPU
> that a channel will interrupt.
> 
> Introduce the CHANNELMSG_MODIFYCHANNEL message type, and define the
> vmbus_send_modifychannel() function to send CHANNELMSG_MODIFYCHANNEL
> requests to the host via a hypercall.  The function is then used to
> define a sysfs "store" operation, which allows to change the (v)CPU
> the channel will interrupt by using the sysfs interface.  The feature
> can be used for load balancing or other purposes.
> 
> One interesting catch here is that Hyper-V can *not* currently ACK
> CHANNELMSG_MODIFYCHANNEL messages with the promise that (after the ACK
> is sent) the channel won't send any more interrupts to the "old" CPU.
> 
> The peculiarity of the CHANNELMSG_MODIFYCHANNEL messages is problematic
> if the user want to take a CPU offline, since we don't want to take a
> CPU offline (and, potentially, "lose" channel interrupts on such CPU)
> if the host is still processing a CHANNELMSG_MODIFYCHANNEL message
> associated to that CPU.
> 
> It is worth mentioning, however, that we have been unable to observe
> the above mentioned "race": in all our tests, CHANNELMSG_MODIFYCHANNEL
> requests appeared *as if* they were processed synchronously by the host.
> 
> Suggested-by: Michael Kelley <mikelley@microsoft.com>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> ---
>  drivers/hv/channel.c      |  28 ++++++++++
>  drivers/hv/channel_mgmt.c |   2 +-
>  drivers/hv/hv_trace.h     |  19 +++++++
>  drivers/hv/vmbus_drv.c    | 108 +++++++++++++++++++++++++++++++++++++-
>  include/linux/hyperv.h    |  10 +++-
>  5 files changed, 163 insertions(+), 4 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply

* RE: [PATCH 11/11] scsi: storvsc: Re-init stor_chns when a channel interrupt is re-assigned
From: Michael Kelley @ 2020-04-10 19:35 UTC (permalink / raw)
  To: Andrea Parri (Microsoft), linux-kernel@vger.kernel.org
  Cc: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Wei Liu,
	linux-hyperv@vger.kernel.org, Dexuan Cui, Boqun Feng, vkuznets,
	James E.J. Bottomley, Martin K. Petersen,
	linux-scsi@vger.kernel.org
In-Reply-To: <20200406001514.19876-12-parri.andrea@gmail.com>

From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Sunday, April 5, 2020 5:15 PM
> 
> For each storvsc_device, storvsc keeps track of the channel target CPUs
> associated to the device (alloced_cpus) and it uses this information to
> fill a "cache" (stor_chns) mapping CPU->channel according to a certain
> heuristic.  Update the alloced_cpus mask and the stor_chns array when a
> channel of the storvsc device is re-assigned to a different CPU.
> 
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com>
> Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
> Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
> Cc: <linux-scsi@vger.kernel.org>
> ---
>  drivers/hv/vmbus_drv.c     |  4 ++
>  drivers/scsi/storvsc_drv.c | 95 ++++++++++++++++++++++++++++++++++----
>  include/linux/hyperv.h     |  3 ++
>  3 files changed, 94 insertions(+), 8 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply

* RE: [PATCH 20/28] mm: remove the pgprot argument to __vmalloc
From: Michael Kelley @ 2020-04-10 20:39 UTC (permalink / raw)
  To: Christoph Hellwig, Andrew Morton, KY Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, x86@kernel.org, David Airlie,
	Daniel Vetter, Laura Abbott, Sumit Semwal, Sakari Ailus,
	Minchan Kim, Nitin Gupta
  Cc: Robin Murphy, Christophe Leroy, Peter Zijlstra,
	linuxppc-dev@lists.ozlabs.org, linux-hyperv@vger.kernel.org,
	dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org,
	linux-arch@vger.kernel.org, linux-mm@kvack.org,
	iommu@lists.linux-foundation.org,
	linux-arm-kernel@lists.infradead.org, linux-s390@vger.kernel.org,
	bpf@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20200408115926.1467567-21-hch@lst.de>

From: Christoph Hellwig <hch@lst.de> Sent: Wednesday, April 8, 2020 4:59 AM
> 
> The pgprot argument to __vmalloc is always PROT_KERNEL now, so remove
> it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/x86/hyperv/hv_init.c              |  3 +--
>  arch/x86/include/asm/kvm_host.h        |  3 +--
>  arch/x86/kvm/svm.c                     |  3 +--
>  drivers/block/drbd/drbd_bitmap.c       |  4 +---
>  drivers/gpu/drm/etnaviv/etnaviv_dump.c |  4 ++--
>  drivers/lightnvm/pblk-init.c           |  5 ++---
>  drivers/md/dm-bufio.c                  |  4 ++--
>  drivers/mtd/ubi/io.c                   |  4 ++--
>  drivers/scsi/sd_zbc.c                  |  3 +--
>  fs/gfs2/dir.c                          |  9 ++++-----
>  fs/gfs2/quota.c                        |  2 +-
>  fs/nfs/blocklayout/extent_tree.c       |  2 +-
>  fs/ntfs/malloc.h                       |  2 +-
>  fs/ubifs/debug.c                       |  2 +-
>  fs/ubifs/lprops.c                      |  2 +-
>  fs/ubifs/lpt_commit.c                  |  4 ++--
>  fs/ubifs/orphan.c                      |  2 +-
>  fs/xfs/kmem.c                          |  2 +-
>  include/linux/vmalloc.h                |  2 +-
>  kernel/bpf/core.c                      |  6 +++---
>  kernel/groups.c                        |  2 +-
>  kernel/module.c                        |  3 +--
>  mm/nommu.c                             | 15 +++++++--------
>  mm/page_alloc.c                        |  2 +-
>  mm/percpu.c                            |  2 +-
>  mm/vmalloc.c                           |  4 ++--
>  net/bridge/netfilter/ebtables.c        |  6 ++----
>  sound/core/memalloc.c                  |  2 +-
>  sound/core/pcm_memory.c                |  2 +-
>  29 files changed, 47 insertions(+), 59 deletions(-)
> 

For Hyper-V changes,

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply

* RE: [PATCH 01/28] x86/hyperv: use vmalloc_exec for the hypercall page
From: Michael Kelley @ 2020-04-10 20:40 UTC (permalink / raw)
  To: Christoph Hellwig, Andrew Morton, KY Srinivasan, Haiyang Zhang,
	Stephen Hemminger, Wei Liu, x86@kernel.org, David Airlie,
	Daniel Vetter, Laura Abbott, Sumit Semwal, Sakari Ailus,
	Minchan Kim, Nitin Gupta
  Cc: Robin Murphy, Christophe Leroy, Peter Zijlstra,
	linuxppc-dev@lists.ozlabs.org, linux-hyperv@vger.kernel.org,
	dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org,
	linux-arch@vger.kernel.org, linux-mm@kvack.org,
	iommu@lists.linux-foundation.org,
	linux-arm-kernel@lists.infradead.org, linux-s390@vger.kernel.org,
	bpf@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20200408115926.1467567-2-hch@lst.de>

From: Christoph Hellwig <hch@lst.de> Sent: Wednesday, April 8, 2020 4:59 AM
> 
> Use the designated helper for allocating executable kernel memory, and
> remove the now unused PAGE_KERNEL_RX define.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/x86/hyperv/hv_init.c            | 2 +-
>  arch/x86/include/asm/pgtable_types.h | 2 --
>  2 files changed, 1 insertion(+), 3 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox