All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Jonathan Corbet <corbet@lwn.net>
Cc: Jens Axboe <axboe@kernel.dk>, Jann Horn <jannh@google.com>,
	linux-aio@kvack.org, linux-block@vger.kernel.org,
	Linux API <linux-api@vger.kernel.org>,
	hch@lst.de, jmoyer@redhat.com, avi@scylladb.com,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 13/18] io_uring: add file set registration
Date: Mon, 11 Feb 2019 17:35:21 +0000	[thread overview]
Message-ID: <20190211173521.GI2217@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20190211085533.35190404@lwn.net>

On Mon, Feb 11, 2019 at 08:55:33AM -0700, Jonathan Corbet wrote:
> On Thu, 7 Feb 2019 11:45:40 -0700
> Jens Axboe <axboe@kernel.dk> wrote:
> 
> > > OK, braindump time:  
> > 
> > [snip]
> > 
> > This is great info, and I think it belongs in Documentation/ somewhere.
> > Not sure I've ever seen such a good and detailed dump of this before.
> 
> I suspect I might be able to make something like that happen :)  Stay
> tuned.

There are several typos I see in there (e.g. in

        * struct file instances A and B being AF_UNIX sockets.
        * A is a listener
        * B is an established connection, with the other end
yet to be accepted on A
        * the only references to A and B are in an SCM_RIGHTS
datagram sent over by A.

the last line should be "datagram sent over by B", of course - you can't
send anything over a listener socket, to start with).

Another thing is this:

        * references cannot be extracted from SCM_RIGHTS datagrams
while the garbage collector is running (achieved by having
unix_notinflight() done before references out of SCM_RIGHTS)
        * removal of SCM_RIGHTS associated with a socket can't
be done without a reference to that socket _outside_ of any
SCM_RIGHTS (automatically true).

That's worse than a typo - that's an actual bug (see the subthread
with Miklos).  Correct version would be
	* any references extracted from SCM_RIGHTS during the
garbage collector run will not be actually used until the end
of garbage collection.  For normal recvmsg() it is guaranteed
by having unix_notinflight() called between the extraction of
scm_fp_list from the packet and doing anything else with the
references extracted.  For MSG_PEEK recvmsg() it's actually
broken and lacks synchronization; Miklos has proposed to grab
and release unix_gc_lock in those, between scm_fp_dup() and
doing anything else with the references copied.

If you turn that thing into a coherent text, I'd appreciate a chance
to take a look at the result and see if anything else needs to be
corrected...

WARNING: multiple messages have this Message-ID (diff)
From: Al Viro <viro@zeniv.linux.org.uk>
To: Jonathan Corbet <corbet@lwn.net>
Cc: Jens Axboe <axboe@kernel.dk>, Jann Horn <jannh@google.com>,
	linux-aio@kvack.org, linux-block@vger.kernel.org,
	Linux API <linux-api@vger.kernel.org>,
	hch@lst.de, jmoyer@redhat.com, avi@scylladb.com,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 13/18] io_uring: add file set registration
Date: Mon, 11 Feb 2019 17:35:21 +0000	[thread overview]
Message-ID: <20190211173521.GI2217@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20190211085533.35190404@lwn.net>

On Mon, Feb 11, 2019 at 08:55:33AM -0700, Jonathan Corbet wrote:
> On Thu, 7 Feb 2019 11:45:40 -0700
> Jens Axboe <axboe@kernel.dk> wrote:
> 
> > > OK, braindump time:  
> > 
> > [snip]
> > 
> > This is great info, and I think it belongs in Documentation/ somewhere.
> > Not sure I've ever seen such a good and detailed dump of this before.
> 
> I suspect I might be able to make something like that happen :)  Stay
> tuned.

There are several typos I see in there (e.g. in

        * struct file instances A and B being AF_UNIX sockets.
        * A is a listener
        * B is an established connection, with the other end
yet to be accepted on A
        * the only references to A and B are in an SCM_RIGHTS
datagram sent over by A.

the last line should be "datagram sent over by B", of course - you can't
send anything over a listener socket, to start with).

Another thing is this:

        * references cannot be extracted from SCM_RIGHTS datagrams
while the garbage collector is running (achieved by having
unix_notinflight() done before references out of SCM_RIGHTS)
        * removal of SCM_RIGHTS associated with a socket can't
be done without a reference to that socket _outside_ of any
SCM_RIGHTS (automatically true).

That's worse than a typo - that's an actual bug (see the subthread
with Miklos).  Correct version would be
	* any references extracted from SCM_RIGHTS during the
garbage collector run will not be actually used until the end
of garbage collection.  For normal recvmsg() it is guaranteed
by having unix_notinflight() called between the extraction of
scm_fp_list from the packet and doing anything else with the
references extracted.  For MSG_PEEK recvmsg() it's actually
broken and lacks synchronization; Miklos has proposed to grab
and release unix_gc_lock in those, between scm_fp_dup() and
doing anything else with the references copied.

If you turn that thing into a coherent text, I'd appreciate a chance
to take a look at the result and see if anything else needs to be
corrected...

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

  reply	other threads:[~2019-02-11 17:35 UTC|newest]

Thread overview: 158+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-29 19:26 [PATCHSET v9] io_uring IO interface Jens Axboe
2019-01-29 19:26 ` Jens Axboe
2019-01-29 19:26 ` [PATCH 01/18] fs: add an iopoll method to struct file_operations Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 19:26 ` [PATCH 02/18] block: wire up block device iopoll method Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 19:26 ` [PATCH 03/18] block: add bio_set_polled() helper Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 19:26 ` [PATCH 04/18] iomap: wire up the iopoll method Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 19:26 ` [PATCH 05/18] Add io_uring IO interface Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 19:26 ` [PATCH 06/18] io_uring: add fsync support Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 19:26 ` [PATCH 07/18] io_uring: support for IO polling Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 20:47   ` Jann Horn
2019-01-29 20:47     ` Jann Horn
2019-01-29 20:56     ` Jens Axboe
2019-01-29 20:56       ` Jens Axboe
2019-01-29 21:10       ` Jann Horn
2019-01-29 21:10         ` Jann Horn
2019-01-29 21:33         ` Jens Axboe
2019-01-29 21:33           ` Jens Axboe
2019-01-29 19:26 ` [PATCH 08/18] fs: add fget_many() and fput_many() Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 19:26 ` [PATCH 09/18] io_uring: use fget/fput_many() for file references Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 23:31   ` Jann Horn
2019-01-29 23:31     ` Jann Horn
2019-01-29 23:44     ` Jens Axboe
2019-01-29 23:44       ` Jens Axboe
2019-01-30 15:33       ` Jens Axboe
2019-01-30 15:33         ` Jens Axboe
2019-01-29 19:26 ` [PATCH 10/18] io_uring: batch io_kiocb allocation Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 19:26 ` [PATCH 11/18] block: implement bio helper to add iter bvec pages to bio Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 19:26 ` [PATCH 12/18] io_uring: add support for pre-mapped user IO buffers Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 22:44   ` Jann Horn
2019-01-29 22:44     ` Jann Horn
2019-01-29 22:56     ` Jens Axboe
2019-01-29 22:56       ` Jens Axboe
2019-01-29 23:03       ` Jann Horn
2019-01-29 23:03         ` Jann Horn
2019-01-29 23:06         ` Jens Axboe
2019-01-29 23:06           ` Jens Axboe
2019-01-29 23:08           ` Jann Horn
2019-01-29 23:08             ` Jann Horn
2019-01-29 23:14             ` Jens Axboe
2019-01-29 23:14               ` Jens Axboe
2019-01-29 23:42               ` Jann Horn
2019-01-29 23:42                 ` Jann Horn
2019-01-29 23:51                 ` Jens Axboe
2019-01-29 23:51                   ` Jens Axboe
2019-01-29 19:26 ` [PATCH 13/18] io_uring: add file set registration Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-30  1:29   ` Jann Horn
2019-01-30  1:29     ` Jann Horn
2019-01-30 15:35     ` Jens Axboe
2019-01-30 15:35       ` Jens Axboe
2019-02-04  2:56     ` Al Viro
2019-02-04  2:56       ` Al Viro
2019-02-05  2:19       ` Jens Axboe
2019-02-05  2:19         ` Jens Axboe
2019-02-05 17:57         ` Jens Axboe
2019-02-05 17:57           ` Jens Axboe
2019-02-05 19:08           ` Jens Axboe
2019-02-05 19:08             ` Jens Axboe
2019-02-06  0:27             ` Jens Axboe
2019-02-06  0:27               ` Jens Axboe
2019-02-06  1:01               ` Al Viro
2019-02-06  1:01                 ` Al Viro
2019-02-06 17:56                 ` Jens Axboe
2019-02-06 17:56                   ` Jens Axboe
2019-02-07  4:05                   ` Al Viro
2019-02-07  4:05                     ` Al Viro
2019-02-07 16:14                     ` Jens Axboe
2019-02-07 16:30                       ` Al Viro
2019-02-07 16:30                         ` Al Viro
2019-02-07 16:35                         ` Jens Axboe
2019-02-07 16:35                           ` Jens Axboe
2019-02-07 16:51                         ` Al Viro
2019-02-07 16:51                           ` Al Viro
2019-02-06  0:56             ` Al Viro
2019-02-06  0:56               ` Al Viro
2019-02-06 13:41               ` Jens Axboe
2019-02-06 13:41                 ` Jens Axboe
2019-02-07  4:00                 ` Al Viro
2019-02-07  4:00                   ` Al Viro
2019-02-07  9:22                   ` Miklos Szeredi
2019-02-07  9:22                     ` Miklos Szeredi
2019-02-07 13:31                     ` Al Viro
2019-02-07 13:31                       ` Al Viro
2019-02-07 14:20                       ` Miklos Szeredi
2019-02-07 14:20                         ` Miklos Szeredi
2019-02-07 15:20                         ` Al Viro
2019-02-07 15:20                           ` Al Viro
2019-02-07 15:27                           ` Miklos Szeredi
2019-02-07 15:27                             ` Miklos Szeredi
2019-02-07 16:26                             ` Al Viro
2019-02-07 16:26                               ` Al Viro
2019-02-07 19:08                               ` Miklos Szeredi
2019-02-07 19:08                                 ` Miklos Szeredi
2019-02-07 18:45                   ` Jens Axboe
2019-02-07 18:45                     ` Jens Axboe
2019-02-07 18:58                     ` Jens Axboe
2019-02-07 18:58                       ` Jens Axboe
2019-02-11 15:55                     ` Jonathan Corbet
2019-02-11 15:55                       ` Jonathan Corbet
2019-02-11 17:35                       ` Al Viro [this message]
2019-02-11 17:35                         ` Al Viro
2019-02-11 20:33                         ` Jonathan Corbet
2019-02-11 20:33                           ` Jonathan Corbet
2019-01-29 19:26 ` [PATCH 14/18] io_uring: add submission polling Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 19:26 ` [PATCH 15/18] io_uring: add io_kiocb ref count Jens Axboe
2019-01-29 19:26   ` Jens Axboe
2019-01-29 19:27 ` [PATCH 16/18] io_uring: add support for IORING_OP_POLL Jens Axboe
2019-01-29 19:27   ` Jens Axboe
2019-01-29 19:27 ` [PATCH 17/18] io_uring: allow workqueue item to handle multiple buffered requests Jens Axboe
2019-01-29 19:27   ` Jens Axboe
2019-01-29 19:27 ` [PATCH 18/18] io_uring: add io_uring_event cache hit information Jens Axboe
2019-01-29 19:27   ` Jens Axboe
  -- strict thread matches above, loose matches on Subject: below --
2019-02-07 19:55 [PATCHSET v12] io_uring IO interface Jens Axboe
2019-02-07 19:55 ` [PATCH 13/18] io_uring: add file set registration Jens Axboe
2019-02-07 19:55   ` Jens Axboe
2019-02-08 12:17   ` Alan Jenkins
2019-02-08 12:17     ` Alan Jenkins
2019-02-08 12:57     ` Jens Axboe
2019-02-08 12:57       ` Jens Axboe
2019-02-08 14:02       ` Alan Jenkins
2019-02-08 14:02         ` Alan Jenkins
2019-02-08 15:13         ` Jens Axboe
2019-02-08 15:13           ` Jens Axboe
2019-02-12 12:29           ` Alan Jenkins
2019-02-12 12:29             ` Alan Jenkins
2019-02-12 15:17             ` Jens Axboe
2019-02-12 15:17               ` Jens Axboe
2019-02-12 17:21               ` Alan Jenkins
2019-02-12 17:21                 ` Alan Jenkins
2019-02-12 17:33                 ` Jens Axboe
2019-02-12 17:33                   ` Jens Axboe
2019-02-12 20:23                   ` Alan Jenkins
2019-02-12 20:23                     ` Alan Jenkins
2019-02-12 21:10                     ` Jens Axboe
2019-02-12 21:10                       ` Jens Axboe
2019-02-01 15:23 [PATCHSET v11] io_uring IO interface Jens Axboe
2019-02-01 15:24 ` [PATCH 13/18] io_uring: add file set registration Jens Axboe
2019-02-01 15:24   ` Jens Axboe
2019-01-30 21:55 [PATCHSET v10] io_uring IO interface Jens Axboe
2019-01-30 21:55 ` [PATCH 13/18] io_uring: add file set registration Jens Axboe
2019-01-30 21:55   ` Jens Axboe
2019-01-28 21:35 [PATCHSET v8] io_uring IO interface Jens Axboe
2019-01-28 21:35 ` [PATCH 13/18] io_uring: add file set registration Jens Axboe
2019-01-28 21:35   ` Jens Axboe
2019-01-29 16:36   ` Jann Horn
2019-01-29 16:36     ` Jann Horn
2019-01-29 18:13     ` Jens Axboe
2019-01-29 18:13       ` Jens Axboe
2019-01-23 15:35 [PATCHSET v7] io_uring IO interface Jens Axboe
2019-01-23 15:35 ` [PATCH 13/18] io_uring: add file set registration Jens Axboe

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=20190211173521.GI2217@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=avi@scylladb.com \
    --cc=axboe@kernel.dk \
    --cc=corbet@lwn.net \
    --cc=hch@lst.de \
    --cc=jannh@google.com \
    --cc=jmoyer@redhat.com \
    --cc=linux-aio@kvack.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.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.