Re: [PATCH] direct-io: allow file systems to do their own waiting for io V2

[Christoph Hellwig <hch@infradead.org>]

I have to say I still hate the flag magic in here.  Spent some time to
look over things to be a bit more constructive in getting what you
guys want in a nicer way:

>  static void dio_bio_end_io(struct bio *bio, int error)
>  {
>  	struct dio *dio = bio->bi_private;
>  	unsigned long flags;
> +	unsigned long remaining;
> +	bool own_waiting = ((dio->rw & WRITE) &&
> +			    (dio->flags & DIO_OWN_WAITING));
> +
> +	if (own_waiting)
> +		dio_bio_complete(dio, bio);
>  
>  	spin_lock_irqsave(&dio->bio_lock, flags);
> +	if (!own_waiting) {
> +		bio->bi_private = dio->bio_list;
> +		dio->bio_list = bio;
> +	}
> +	remaining = --dio->refcount;
> +	if (remaining == 1 && dio->waiter)
>  		wake_up_process(dio->waiter);
>  	spin_unlock_irqrestore(&dio->bio_lock, flags);
> +
> +	if (remaining == 0) {
> +		BUG_ON(!(dio->flags & DIO_OWN_WAITING));
> +		dio_complete(dio, dio->iocb->ki_pos, 0, false);
> +		kmem_cache_free(dio_cache, dio);
> +	}

This own_waiting case of this is not identical to dio_bio_end_aio
except for the inverted is_async argument of dio_complete.

So even if we allow for the flag I think we should test it in dio_end_io
and use common code for the case where we don't use the linked list of
bios to complete.  In that case you could also just call the current
aio version from btrfs as it already calls dio_end_io directly and
remove the flag given that dio_await_completion would become a no-op.

That being said I would much, much prefer to consolidate code here
rather than adding more special cases.

What I would really like to understand is what the point for the
bio_list batching is to start with, given that it also requires nasty
workarounds like dio_bio_reap() to work around the amount of memory it
might have to use.

The only thing I could think of is to allow ->end_io callbacks from user
context, but that is a bigger problem as we can't do that for AIO.  I'd
much prefer a unified approach with my generic user context callbacks
from a few weeks ago to actually simplify this code.  (and yeah, it's
probably up to me to demonstrate at least a prototype of this)