All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Liu Bo <bo.liu@linux.alibaba.com>
Cc: virtio-fs@redhat.com
Subject: Re: [Virtio-fs] [PATCH] Virtiofs: fix null pointer deference in directIO
Date: Wed, 31 Jul 2019 08:39:38 -0400	[thread overview]
Message-ID: <20190731123938.GA4405@redhat.com> (raw)
In-Reply-To: <20190730222938.ipnojus4euucg4yj@US-160370MP2.local>

On Tue, Jul 30, 2019 at 03:29:39PM -0700, Liu Bo wrote:
> On Tue, Jul 30, 2019 at 11:30:41AM -0400, Vivek Goyal wrote:
> > Hi Liu Bo,
> > 
> > I have committed your patch for now here.
> > 
> > https://github.com/rhvgoyal/linux/commits/virtio-fs-dev-5.1
> > 
> > I also added a patch to wait for page reference count to drop to 1 before
> > range is reclaimed. It slows down mmap() workload very significantly
> > though.
> >
> 
> Yes, the impact on mmap is expected because of the 'unmap' in layout checking,
> it'd be helpful to check layout on the dmap range instead of the whole inode.
> 
> However, there is a race bug in the current code between mmap workloads and dmap
> reclaim, since the reclaim does not unmap the dmap range at all, such that the
> dmap range might still stay in some process's page table, if such a dmap is
> reclaimed, the situation would be worse.  So with unmapping the whole inode in
> layout checking, at least it is correct in term of that race.

Can you elaborate more on this. I am not sure I am able to see the race
you are talking about. Assume we implemented logic to unmap only pages
which are in dmap (and not whole inode).

dmap is something internal to virtio-fs. If we unmap all pages which are
in a dmap, then dmap is free to be reclaimed. We are holding fi->i_mmap_sem,
and that should make sure that unmapped pages don't get mapped again (page
fault code will block on fi->i_mmap_sem).

Thanks
Vivek


  reply	other threads:[~2019-07-31 12:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-11 22:35 [Virtio-fs] [PATCH] Virtiofs: fix null pointer deference in directIO Liu Bo
2019-07-18  0:21 ` Liu Bo
2019-07-18 18:18 ` Vivek Goyal
2019-07-18 18:58   ` Liu Bo
2019-07-18 20:51     ` Vivek Goyal
2019-07-18 23:39       ` Liu Bo
2019-07-19 19:51         ` Vivek Goyal
2019-07-20  2:34           ` Liu Bo
2019-07-30 15:30 ` Vivek Goyal
2019-07-30 22:29   ` Liu Bo
2019-07-31 12:39     ` Vivek Goyal [this message]
2019-07-31 17:53       ` Liu Bo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190731123938.GA4405@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=bo.liu@linux.alibaba.com \
    --cc=virtio-fs@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.