All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
To: Miklos Szeredi <miklos-sUDqSbJrdHQHWmgEVkV9KA@public.gmane.org>
Cc: Linux NFS list
	<linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>,
	Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Chris Wilson
	<chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	v9fs-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	ceph-devel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linus Torvalds
	<torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	"Kirill A. Shutemov"
	<kirill.shutemov-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
	lustre-devel-aLEFhgZF4x6X6Mz3xDxJMA@public.gmane.org
Subject: Re: [PATCH v3 0/2] iov_iter: allow iov_iter_get_pages_alloc to allocate more pages per call
Date: Tue, 7 Feb 2017 07:19:09 +0000	[thread overview]
Message-ID: <20170207071909.GI13195@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CAJfpegv-ePQE9pNwZe6O+0LjJdq2aVk3bnhxeZ=y7P+iFq72XA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Mon, Feb 06, 2017 at 03:18:42PM +0100, Miklos Szeredi wrote:

> But I think it's breakable in the same way: if the deadlocked request
> is aborted, the fault will release the page lock as well as mmap_sem,
> and from there things will resolve themselves.

Right you are - original holder of ->mmap_sem is waiting for request and
abort will wake it up...

> But you are definitely right about needing to clean up that mess in
> fuse/dev.c and doing so by fixing up the arg refcounting for just the
> read and write requests is going to be a lot simpler than having to do
> that for all of them (which was my original plan).

Speaking of refcounting - what's going on with fuse_file one?  My reading
of that code is that you have 4 states of that thing:
	* new (just created, fallback request allocated, use fuse_file_free()
to kill).  Refcount is 0.
	* intermediate - in fact it's already opened, but still not
put into ->private_data.  Refcount is still 0.  Use fuse_sync_release() to kill.
	* live - normal refcounting (fuse_file_get()/fuse_file_put()).
	* shutdown - refcount has reached 0.  Can't happen until ->release()
(obviously - ->private_data holds a counting reference), some pieces of
fuse_sync_release() correspond to some stuff in fuse_release_common(), some -
to final fuse_file_put().

To make it even more convoluted, cuse is using fuse_sync_release() and
apparently relies upon no references being grabbed after fuse_do_open(),
so that thing can be called with refcount 0 *or* refcount 1.

Another thing: what guarantees that places in writepages-related paths
where we store a reference into req->ff won't hit a request with already
non-NULL ->ff?

WARNING: multiple messages have this Message-ID (diff)
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Linux NFS list <linux-nfs@vger.kernel.org>,
	ceph-devel@vger.kernel.org, lustre-devel@lists.lustre.org,
	v9fs-developer@lists.sourceforge.net,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Jan Kara <jack@suse.cz>, Chris Wilson <chris@chris-wilson.co.uk>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Jeff Layton <jlayton@redhat.com>
Subject: Re: [PATCH v3 0/2] iov_iter: allow iov_iter_get_pages_alloc to allocate more pages per call
Date: Tue, 7 Feb 2017 07:19:09 +0000	[thread overview]
Message-ID: <20170207071909.GI13195@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CAJfpegv-ePQE9pNwZe6O+0LjJdq2aVk3bnhxeZ=y7P+iFq72XA@mail.gmail.com>

On Mon, Feb 06, 2017 at 03:18:42PM +0100, Miklos Szeredi wrote:

> But I think it's breakable in the same way: if the deadlocked request
> is aborted, the fault will release the page lock as well as mmap_sem,
> and from there things will resolve themselves.

Right you are - original holder of ->mmap_sem is waiting for request and
abort will wake it up...

> But you are definitely right about needing to clean up that mess in
> fuse/dev.c and doing so by fixing up the arg refcounting for just the
> read and write requests is going to be a lot simpler than having to do
> that for all of them (which was my original plan).

Speaking of refcounting - what's going on with fuse_file one?  My reading
of that code is that you have 4 states of that thing:
	* new (just created, fallback request allocated, use fuse_file_free()
to kill).  Refcount is 0.
	* intermediate - in fact it's already opened, but still not
put into ->private_data.  Refcount is still 0.  Use fuse_sync_release() to kill.
	* live - normal refcounting (fuse_file_get()/fuse_file_put()).
	* shutdown - refcount has reached 0.  Can't happen until ->release()
(obviously - ->private_data holds a counting reference), some pieces of
fuse_sync_release() correspond to some stuff in fuse_release_common(), some -
to final fuse_file_put().

To make it even more convoluted, cuse is using fuse_sync_release() and
apparently relies upon no references being grabbed after fuse_do_open(),
so that thing can be called with refcount 0 *or* refcount 1.

Another thing: what guarantees that places in writepages-related paths
where we store a reference into req->ff won't hit a request with already
non-NULL ->ff?

WARNING: multiple messages have this Message-ID (diff)
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Miklos Szeredi <miklos-sUDqSbJrdHQHWmgEVkV9KA@public.gmane.org>
Cc: Linux NFS list
	<linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>,
	Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Chris Wilson
	<chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	v9fs-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	ceph-devel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linus Torvalds
	<torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	"Kirill A. Shutemov"
	<kirill.shutemov-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
	lustre-devel-aLEFhgZF4x6X6Mz3xDxJMA@public.gmane.org
Subject: [lustre-devel] [PATCH v3 0/2] iov_iter: allow iov_iter_get_pages_alloc to allocate more pages per call
Date: Tue, 7 Feb 2017 07:19:09 +0000	[thread overview]
Message-ID: <20170207071909.GI13195@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CAJfpegv-ePQE9pNwZe6O+0LjJdq2aVk3bnhxeZ=y7P+iFq72XA@mail.gmail.com>

On Mon, Feb 06, 2017 at 03:18:42PM +0100, Miklos Szeredi wrote:

> But I think it's breakable in the same way: if the deadlocked request
> is aborted, the fault will release the page lock as well as mmap_sem,
> and from there things will resolve themselves.

Right you are - original holder of ->mmap_sem is waiting for request and
abort will wake it up...

> But you are definitely right about needing to clean up that mess in
> fuse/dev.c and doing so by fixing up the arg refcounting for just the
> read and write requests is going to be a lot simpler than having to do
> that for all of them (which was my original plan).

Speaking of refcounting - what's going on with fuse_file one?  My reading
of that code is that you have 4 states of that thing:
	* new (just created, fallback request allocated, use fuse_file_free()
to kill).  Refcount is 0.
	* intermediate - in fact it's already opened, but still not
put into ->private_data.  Refcount is still 0.  Use fuse_sync_release() to kill.
	* live - normal refcounting (fuse_file_get()/fuse_file_put()).
	* shutdown - refcount has reached 0.  Can't happen until ->release()
(obviously - ->private_data holds a counting reference), some pieces of
fuse_sync_release() correspond to some stuff in fuse_release_common(), some -
to final fuse_file_put().

To make it even more convoluted, cuse is using fuse_sync_release() and
apparently relies upon no references being grabbed after fuse_do_open(),
so that thing can be called with refcount 0 *or* refcount 1.

Another thing: what guarantees that places in writepages-related paths
where we store a reference into req->ff won't hit a request with already
non-NULL ->ff?

  parent reply	other threads:[~2017-02-07  7:19 UTC|newest]

Thread overview: 101+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-24 21:23 [PATCH] iov_iter: allow iov_iter_get_pages_alloc to allocate more pages per call Jeff Layton
2017-01-25 13:32 ` [PATCH v3 0/2] " Jeff Layton
2017-01-25 13:32   ` [PATCH v3 1/2] " Jeff Layton
2017-01-26 12:35     ` Jeff Layton
2017-01-27 13:24       ` [PATCH v4 0/2] " Jeff Layton
2017-01-27 13:24         ` [PATCH v4 1/2] " Jeff Layton
2017-01-27 13:24         ` [PATCH v4 2/2] ceph: switch DIO code to use iov_iter_get_pages_alloc Jeff Layton
     [not found]           ` <20170127132451.6601-3-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2017-01-30 15:40             ` Jeff Layton
2017-01-30 15:40               ` Jeff Layton
2017-01-25 13:32   ` [PATCH v3 " Jeff Layton
     [not found]   ` <20170125133205.21704-1-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2017-02-02  9:51     ` [PATCH v3 0/2] iov_iter: allow iov_iter_get_pages_alloc to allocate more pages per call Al Viro
2017-02-02  9:51       ` [lustre-devel] " Al Viro
2017-02-02  9:51       ` Al Viro
     [not found]       ` <20170202095125.GF27291-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2017-02-02 10:56         ` Christoph Hellwig
2017-02-02 10:56           ` [lustre-devel] " Christoph Hellwig
2017-02-02 10:56           ` Christoph Hellwig
     [not found]           ` <20170202105651.GA32111-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2017-02-02 11:16             ` Al Viro
2017-02-02 11:16               ` [lustre-devel] " Al Viro
2017-02-02 11:16               ` Al Viro
     [not found]               ` <20170202111625.GG27291-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2017-02-02 13:00                 ` Jeff Layton
2017-02-02 13:00                   ` Jeff Layton
     [not found]                   ` <1486040452.2812.6.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2017-02-03  7:29                     ` Al Viro
2017-02-03  7:29                       ` [lustre-devel] " Al Viro
2017-02-03  7:29                       ` Al Viro
     [not found]                       ` <20170203072952.GI27291-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2017-02-03 18:29                         ` Linus Torvalds
2017-02-03 18:29                           ` [lustre-devel] " Linus Torvalds
2017-02-03 18:29                           ` Linus Torvalds
     [not found]                           ` <CA+55aFx=NPESJv9RjCNRKFH_rk9uVMov0UtFbpZH-xBsgK2h-w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-03 19:08                             ` Al Viro
2017-02-03 19:08                               ` [lustre-devel] " Al Viro
2017-02-03 19:08                               ` Al Viro
     [not found]                               ` <20170203190816.GK27291-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2017-02-03 19:28                                 ` Linus Torvalds
2017-02-03 19:28                                   ` [lustre-devel] " Linus Torvalds
2017-02-03 19:28                                   ` Linus Torvalds
2017-02-13  9:56                                   ` Steve Capper
     [not found]                                     ` <20170213095616.GA18053-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-02-13 21:40                                       ` Linus Torvalds
2017-02-13 21:40                                         ` [lustre-devel] " Linus Torvalds
2017-02-13 21:40                                         ` Linus Torvalds
2017-02-03  7:49                     ` Christoph Hellwig
2017-02-03  7:49                       ` [lustre-devel] " Christoph Hellwig
2017-02-03  7:49                       ` Christoph Hellwig
     [not found]                       ` <20170203074901.GA19808-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2017-02-03  8:54                         ` Al Viro
2017-02-03  8:54                           ` [lustre-devel] " Al Viro
2017-02-03  8:54                           ` Al Viro
     [not found]                           ` <20170203085415.GJ27291-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2017-02-03 11:09                             ` Christoph Hellwig
2017-02-03 11:09                               ` [lustre-devel] " Christoph Hellwig
2017-02-03 11:09                               ` Christoph Hellwig
2017-02-02 14:48         ` Jan Kara
2017-02-02 14:48           ` [lustre-devel] " Jan Kara
2017-02-02 14:48           ` Jan Kara
     [not found]           ` <20170202144817.GB15545-4I4JzKEfoa/jFM9bn6wA6Q@public.gmane.org>
2017-02-02 18:28             ` Al Viro
2017-02-02 18:28               ` [lustre-devel] " Al Viro
2017-02-02 18:28               ` Al Viro
     [not found]               ` <20170202182802.GH27291-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2017-02-03 14:47                 ` Jan Kara
2017-02-03 14:47                   ` [lustre-devel] " Jan Kara
2017-02-03 14:47                   ` Jan Kara
2017-02-04  3:08         ` Al Viro
2017-02-04  3:08           ` [lustre-devel] " Al Viro
2017-02-04  3:08           ` Al Viro
     [not found]           ` <20170204030842.GL27291-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2017-02-04 19:26             ` Al Viro
2017-02-04 19:26               ` [lustre-devel] " Al Viro
2017-02-04 19:26               ` Al Viro
     [not found]               ` <20170204192655.GA13195-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2017-02-04 22:12                 ` Miklos Szeredi
2017-02-04 22:12                   ` Miklos Szeredi
2017-02-04 22:11             ` Miklos Szeredi
2017-02-04 22:11               ` Miklos Szeredi
     [not found]               ` <CAJfpegtVb8PKNnKe5wGMd0u0WzgLpjpVtVpqDScbrBJShLAfGw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-05  1:51                 ` Al Viro
2017-02-05  1:51                   ` [lustre-devel] " Al Viro
2017-02-05  1:51                   ` Al Viro
     [not found]                   ` <20170205015145.GB13195-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2017-02-05 20:15                     ` Miklos Szeredi
2017-02-05 20:15                       ` Miklos Szeredi
     [not found]                       ` <CAJfpegv=r9J8Mqax_ZAB2h5QbRgJMHwyVMENTpYZ8u3_pqNfJw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-05 21:01                         ` Al Viro
2017-02-05 21:01                           ` [lustre-devel] " Al Viro
2017-02-05 21:01                           ` Al Viro
     [not found]                           ` <20170205210151.GD13195-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2017-02-05 21:19                             ` Miklos Szeredi
2017-02-05 21:19                               ` Miklos Szeredi
     [not found]                               ` <CAJfpeguzOgX9d+5XCNJcmXW5KkrGbnWB5aZSP1-0q3a6i6uk2w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-05 22:04                                 ` Al Viro
2017-02-05 22:04                                   ` Al Viro
2017-02-05 22:04                                   ` [lustre-devel] " Al Viro
2017-02-05 22:04                                   ` Al Viro
     [not found]                                   ` <20170205220445.GE13195-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2017-02-06  3:05                                     ` Al Viro
2017-02-06  3:05                                       ` [lustre-devel] " Al Viro
2017-02-06  3:05                                       ` Al Viro
2017-02-06  9:08                                       ` Miklos Szeredi
     [not found]                                         ` <CAJfpegv5ZGd2gzSbQvgk4uX5q06AijY+TNg2jdrPBSjbFoXMfg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-06  9:57                                           ` Al Viro
2017-02-06  9:57                                             ` [lustre-devel] " Al Viro
2017-02-06  9:57                                             ` Al Viro
2017-02-06 14:18                                             ` Miklos Szeredi
     [not found]                                               ` <CAJfpegv-ePQE9pNwZe6O+0LjJdq2aVk3bnhxeZ=y7P+iFq72XA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-07  7:19                                                 ` Al Viro [this message]
2017-02-07  7:19                                                   ` [lustre-devel] " Al Viro
2017-02-07  7:19                                                   ` Al Viro
     [not found]                                                   ` <20170207071909.GI13195-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
2017-02-07 11:35                                                     ` Miklos Szeredi
2017-02-07 11:35                                                       ` Miklos Szeredi
     [not found]                                                       ` <20170207113554.GA30656-6fGP+1Jn+PcQFMt52x3flrFA6kmya3R0B1poBFZFfSg@public.gmane.org>
2017-02-08  5:54                                                         ` Al Viro
2017-02-08  5:54                                                           ` [lustre-devel] " Al Viro
2017-02-08  5:54                                                           ` Al Viro
2017-02-08  9:53                                                           ` Miklos Szeredi
2017-02-06  8:37                                   ` Miklos Szeredi
2017-02-05 20:56                     ` Al Viro
2017-02-05 20:56                       ` [lustre-devel] " Al Viro
2017-02-05 20:56                       ` Al Viro
2017-02-16 13:10       ` Jeff Layton

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=20170207071909.GI13195@ZenIV.linux.org.uk \
    --to=viro-3bdd1+5odreifsdqtta3olvcufugdwfn@public.gmane.org \
    --cc=ceph-devel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org \
    --cc=jack-AlSwsSmVLrQ@public.gmane.org \
    --cc=jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=kirill.shutemov-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
    --cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=lustre-devel-aLEFhgZF4x6X6Mz3xDxJMA@public.gmane.org \
    --cc=miklos-sUDqSbJrdHQHWmgEVkV9KA@public.gmane.org \
    --cc=torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org \
    --cc=v9fs-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
    /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.