Re: [PATCH 2/2] zonefs: use zone-append for AIO as well

[Johannes Thumshirn <Johannes.Thumshirn@wdc.com>]

On 20/07/2020 15:45, Christoph Hellwig wrote:
> On Mon, Jul 20, 2020 at 10:21:18PM +0900, Johannes Thumshirn wrote:
>> On a successful completion, the position the data is written to is
>> returned via AIO's res2 field to the calling application.
> 
> That is a major, and except for this changelog, undocumented ABI
> change.  We had the whole discussion about reporting append results
> in a few threads and the issues with that in io_uring.  So let's
> have that discussion there and don't mix it up with how zonefs
> writes data.  Without that a lot of the boilerplate code should
> also go away.
> 

OK maybe I didn't remember correctly, but wasn't this all around 
io_uring and how we'd report the location back for raw block device
access?

I'll re-read the threads.

>> -	if (zi->i_ztype == ZONEFS_ZTYPE_SEQ &&
>> -	    (ret > 0 || ret == -EIOCBQUEUED)) {
>> +
>> +	if (ret > 0 || ret == -EIOCBQUEUED) {
>>  		if (ret > 0)
>>  			count = ret;
>>  		mutex_lock(&zi->i_truncate_mutex);
> 
> Don't we still need the ZONEFS_ZTYPE_SEQ after updating count, but
> before updating i_wpoffset?  Also how is this related to the rest
> of the patch?

This looks like a leftover from development that I forgot to clean up.
Will be addressing it in the next round.

> 
>> @@ -1580,6 +1666,11 @@ static int zonefs_fill_super(struct super_block *sb, void *data, int silent)
>>  	if (!sb->s_root)
>>  		goto cleanup;
>>  
>> +	sbi->s_dio_done_wq = alloc_workqueue("zonefs-dio/%s", WQ_MEM_RECLAIM,
>> +					     0, sb->s_id);
>> +	if (!sbi->s_dio_done_wq)
>> +		goto cleanup;
>> +
> 
> Can you reuse the sb->s_dio_done_wq pointer, and maybe even the helper
> to initialize it?
> 

IIRC I had some issues with that and then decided to just roll my own as
the s_dio_done_wq will be allocated for every IO if I read iomap correctly.
Zonefs on the other hand needs the dio for all file accesses on sequential 
files, so creating a dedicated wq didn't seem problematic for me.