Re: [PATCH 1/6] iomap: cleanup up iomap_dio_bio_end_io()

[Jens Axboe <axboe@kernel.dk>]

On 7/19/23 10:50?PM, Christoph Hellwig wrote:
>> +	/*
>> +	 * Synchronous dio, task itself will handle any completion work
>> +	 * that needs after IO. All we need to do is wake the task.
>> +	 */
>> +	if (dio->wait_for_completion) {
>> +		struct task_struct *waiter = dio->submit.waiter;
>> +		WRITE_ONCE(dio->submit.waiter, NULL);
> 
> I know the existing code got it wrong, but can you please add an empty
> line after the variable declaration here?

Sure, will add.

>> +	/*
>> +	 * If this dio is an async write, queue completion work for async
>> +	 * handling. Reads can always complete inline.
>> +	 */
>> +	if (dio->flags & IOMAP_DIO_WRITE) {
>> +		struct inode *inode = file_inode(iocb->ki_filp);
>> +
>> +		WRITE_ONCE(iocb->private, NULL);
>> +		INIT_WORK(&dio->aio.work, iomap_dio_complete_work);
>> +		queue_work(inode->i_sb->s_dio_done_wq, &dio->aio.work);
>> +	} else {
> 
> If we already do the goto style I'd probably do it here as well instead
> of the else.

It does end up like that later on, but I can do it earlier and leave the
least desirable method (workqueue) at the end from this patch.

-- 
Jens Axboe