Re: [PATCH 1/4] file: export __alloc_fd()

From: Christian Brauner <christian@brauner.io>
To: Martijn Coenen <maco@android.com>
Cc: "Matthew Wilcox" <willy@infradead.org>,
	"Christoph Hellwig" <hch@infradead.org>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>,
	"Arve Hjønnevåg" <arve@android.com>,
	"Todd Kjos" <tkjos@android.com>,
	rlove@google.com, ben@decadent.org.uk
Subject: Re: [PATCH 1/4] file: export __alloc_fd()
Date: Tue, 31 Jul 2018 12:07:01 +0200	[thread overview]
Message-ID: <20180731100701.GA18871@mailbox.org> (raw)
In-Reply-To: <CAB0TPYG5T_d32iwt8Lgq_nDO_COWfWBfuSzErraASSuTYGm3yQ@mail.gmail.com>

On Tue, Jul 31, 2018 at 10:44:33AM +0200, Martijn Coenen wrote:
> On Mon, Jul 30, 2018 at 10:36 PM, Matthew Wilcox <willy@infradead.org> wrote:
> > I'm not entirely sure I understand the binder code (... does anyone?)
> > but from what I can see, it intends to open a file descriptor in the
> > process which is the target of the message being sent.
> 
> You're right.
> 
> > That strikes
> > me as wrong-headed; it should be allocating a struct file and passing
> > that file to the other process.  When that process receives the message,
> > *it* allocates a file descriptor for itself.ho
> 
> We're looking into cleaning this up (historically it was done this way
> because VIVT caches made this not very efficient), and this is indeed
> a very good candidate for fixing it.

Ah, this wasn't brought up in the original thread when discussing to
turn this into a module. If using internal functions like this is going
away it makes sense to wait for this work to happen first. Is there a
time-frame for this?

Thanks!
Christian

> 
> >
> > But I think the binder user-space API relies on this.  The userspace API
> > seems to rely on passing fd numbers around ... but I'm having trouble
> > figuring most of this user API out.  Perhaps Martijn can help here.
> 
> The UAPI does expect a file descriptor, but we may be able to do the
> mapping from fd to struct file (and vice-versa) in the kernel driver,
> so userspace wouldn't notice.
> 
> Thanks,
> Martijn