On 7/8/20 9:02 AM, Matthew Wilcox wrote:

On Wed, Jul 08, 2020 at 08:59:50AM -0600, Jens Axboe wrote:

quoted

On 7/8/20 8:58 AM, Matthew Wilcox wrote:

quoted

On Wed, Jul 08, 2020 at 08:54:07AM -0600, Jens Axboe wrote:

quoted

On 7/8/20 6:58 AM, Kanchan Joshi wrote:

quoted

quoted

quoted

+#define IOCB_NO_CMPL		(15 << 28)

 struct kiocb {
[...]
-	void (*ki_complete)(struct kiocb *iocb, long ret, long ret2);
+	loff_t __user *ki_uposp;
-	int			ki_flags;
+	unsigned int		ki_flags;

+typedef void ki_cmpl(struct kiocb *, long ret, long ret2);
+static ki_cmpl * const ki_cmpls[15];

+void ki_complete(struct kiocb *iocb, long ret, long ret2)
+{
+	unsigned int id = iocb->ki_flags >> 28;
+
+	if (id < 15)
+		ki_cmpls[id](iocb, ret, ret2);
+}

+int kiocb_cmpl_register(void (*cb)(struct kiocb *, long, long))
+{
+	for (i = 0; i < 15; i++) {
+		if (ki_cmpls[id])
+			continue;
+		ki_cmpls[id] = cb;
+		return id;
+	}
+	WARN();
+	return -1;
+}

That could work, we don't really have a lot of different completion
types in the kernel.

Thanks, this looks sorted.

Not really, someone still needs to do that work. I took a quick look, and
most of it looks straight forward. The only potential complication is
ocfs2, which does a swap of the completion for the kiocb. That would just
turn into an upper flag swap. And potential sync kiocb with NULL
ki_complete. The latter should be fine, I think we just need to reserve
completion nr 0 for being that.

I was reserving completion 15 for that ;-)

+#define IOCB_NO_CMPL		(15 << 28)
...
+	if (id < 15)
+		ki_cmpls[id](iocb, ret, ret2);

Saves us one pointer in the array ...

That works. Are you going to turn this into an actual series of patches,
adding the functionality and converting users?

I was under the impression Kanchan was going to do that, but I can run it
off quickly ...

I just wanted to get clarification there, because to me it sounded like
you expected Kanchan to do it, and Kanchan assuming it "was sorted". I'd
consider that a prerequisite for the append series as far as io_uring is
concerned, hence _someone_ needs to actually do it ;-)

-- 
Jens Axboe