[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC 2/2] block/file-posix: verify page cache is not us
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [RFC 2/2] block/file-posix: verify page cache is not used |
Date: |
Fri, 27 Apr 2018 11:06:08 +0100 |
User-agent: |
Mutt/1.9.2 (2017-12-15) |
On Tue, Apr 24, 2018 at 04:29:15PM +0200, Kevin Wolf wrote:
> Am 24.04.2018 um 16:04 hat Stefan Hajnoczi geschrieben:
> > On Fri, Apr 20, 2018 at 08:25:13AM +0200, Kevin Wolf wrote:
> > > Am 20.04.2018 um 05:02 hat Stefan Hajnoczi geschrieben:
> > > > On Thu, Apr 19, 2018 at 10:05:47AM +0100, Dr. David Alan Gilbert wrote:
> > > > > * Stefan Hajnoczi (address@hidden) wrote:
> > > > > > This commit is for debugging only. Do not merge it.
> > > > > >
> > > > > > mincore(2) checks whether pages are resident. Use it to verify that
> > > > > > page cache has been dropped.
> > > > > >
> > > > > > You can trigger a verification failure by mmapping the image file
> > > > > > from
> > > > > > another process and loading a byte from a page so that it becomes
> > > > > > resident. bdrv_co_invalidate_cache() will fail while the process is
> > > > > > alive.
> > > > >
> > > > > It doesn't seem a bad diagnostic to keep in (with a switch to
> > > > > activate)
> > > > > for when we're faced with some weird corruption on some weird storage
> > > > > system.
> > > >
> > > > Okay. It's very slow to mmap an entire image file and query mincore(2)
> > > > so it needs to be off by default.
> > >
> > > Also, having it enabled breaks localhost migration at least on tmpfs
> > > (which was what I tried out first).
> > >
> > > I wonder if the kernel would add some way to query whether the "advice"
> > > was actually acted upon if we asked. Either with a new function that
> > > returns an error if not everything is dropped (basically
> > > .bdrv_invalidate_cache on the kernel level), or a function that just
> > > queries if any page is allocated (or maybe the address of the first
> > > allocated page in a given range) without having to use mincore() and
> > > iterating over all the pages in userspace.
> >
> > I'm trying to figure out how to expose the optional mincore check on the
> > command-line/QMP:
> >
> > 1. Add a check_consistency bool argument to bdrv_invalidate_cache*().
> > Add command-line/QMP option to -incoming and migrate_incoming. This
> > is messy and won't be easy to access for libvirt users.
> >
> > 2. Add a BlockdevOptionsFile *check-cache-consistency bool field. This
> > is specified at .bdrv_open() time. It can be changed at runtime with
> > .bdrv_reopen*().
> >
> > 3. Add a 'blockdev-check-cache-consistency' QMP command that calls a new
> > .bdrv_check_cache_consistency() callback that is implemented by
> > file-posix.c. The problem is users might issue this command after
> > I/O has resumed and pages have become resident again. It only makes
> > sense if the guest is still paused. Probably a bad interface...
> >
> > Have I missed a good way to expose this optional check functionality?
> >
> > Which approach do you prefer? I'm leaning towards #2.
>
> Yes, I think that makes the most sense.
>
> In its current form, this can probably only be a debugging feature,
> though, so maybe x-check-cache-consistency? I also don't think libvirt
> should mess with this.
Cool, I will implement this.
Stefan
signature.asc
Description: PGP signature
- Re: [Qemu-devel] [RFC 1/2] block/file-posix: implement bdrv_co_invalidate_cache() on Linux, (continued)
[Qemu-devel] [RFC 2/2] block/file-posix: verify page cache is not used, Stefan Hajnoczi, 2018/04/19
Re: [Qemu-devel] [RFC 0/2] block/file-posix: allow -drive cache.direct=off live migration, Eric Blake, 2018/04/19