Re: Stack trace with Samba VFS io_uring and large transfers

[Jens Axboe <axboe@kernel.dk>]

On 7/28/21 4:02 AM, Forza wrote:
> 
> 
> On 2021-07-26 18:35, Jens Axboe wrote:
>> On 7/26/21 10:07 AM, Jens Axboe wrote:
>>> On 7/24/21 1:51 PM, Forza wrote:
>>>>
>>>>
>>>> On 2021-07-24 21:44, Jens Axboe wrote:
>>>>> On 7/24/21 12:23 PM, Forza wrote:
>>>>>> Hi!
>>>>>>
>>>>>> On 2021-07-24 19:04, Jens Axboe wrote:
>>>>>>> I'll see if I can reproduce this. I'm assuming samba is using buffered
>>>>>>> IO, and it looks like it's reading in chunks of 1MB. Hopefully it's
>>>>>>> possible to reproduce without samba with a windows client, as I don't
>>>>>>> have any of those. If synthetic reproducing fails, I can try samba
>>>>>>> with a Linux client.
>>>>>>
>>>>>> I attached the logs from both a Windows 10 client and a Linux client
>>>>>> (kernel 5.11.0).
>>>>>>
>>>>>> https://paste.tnonline.net/files/r4yebSzlGEVD_linux-client.txt
>>>>>>
>>>>>>      smbd_smb2_read: fnum 2641229669, file
>>>>>> media/vm/libvirt/images/Mint_Cinnamon.img, length=4194304
>>>>>> offset=736100352 read=4194304
>>>>>> [2021/07/24 17:26:09.120779,  3]
>>>>>> ../../source3/smbd/smb2_read.c:415(smb2_read_complete)
>>>>>>      smbd_smb2_read: fnum 2641229669, file
>>>>>> media/vm/libvirt/images/Mint_Cinnamon.img, length=4194304
>>>>>> offset=740294656 read=4194304
>>>>>> [2021/07/24 17:26:09.226593,  3]
>>>>>> ../../source3/smbd/smb2_read.c:415(smb2_read_complete)
>>>>>>      smbd_smb2_read: fnum 2641229669, file
>>>>>> media/vm/libvirt/images/Mint_Cinnamon.img, length=4194304
>>>>>> offset=748683264 read=4194304
>>>>>
>>>>> Thanks, this is useful. Before I try and reproduce it, what is the
>>>>> filesystem that is hosting the samba mount?
>>>>>
>>>>
>>>> I am using Btrfs.
>>>>
>>>> My testing was done by exporting the share with
>>>>
>>>>     vfs objects = io_uring
>>>>     vfs objects = btrfs, io_uring
>>>>
>>>> Same results in both cases. Exporting with "vfs objects = btrfs" (no
>>>> io_uring) works as expected.
>>>
>>> Seems to be specific to btrfs, I can reproduce it here. I'll dive in
>>> and see what I can find.
>>
>> This looks like a race in dealing with the task_work running. At least
>> this on top of current -git closes the gap for me and I can't reproduce
>> it anymore.
>>
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index c4d2b320cdd4..998a01cbc00f 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -1959,9 +1959,13 @@ static void tctx_task_work(struct callback_head *cb)
>>   			node = next;
>>   		}
>>   		if (wq_list_empty(&tctx->task_list)) {
>> +			spin_lock_irq(&tctx->task_lock);
>>   			clear_bit(0, &tctx->task_state);
>> -			if (wq_list_empty(&tctx->task_list))
>> +			if (wq_list_empty(&tctx->task_list)) {
>> +				spin_unlock_irq(&tctx->task_lock);
>>   				break;
>> +			}
>> +			spin_unlock_irq(&tctx->task_lock);
>>   			/* another tctx_task_work() is enqueued, yield */
>>   			if (test_and_set_bit(0, &tctx->task_state))
>>   				break;
>>
> 
> Thanks! Is there a way to get this on current stable such as 5.13.5?

Yes, we'll get it to stable once it's upstream. For your particular
testing, not sure there's an easy way... You'd have to apply it to
5.13.5 and compile it on your own, I'm afraid.

If all else fails, hopefully it'll be in 5.13.6 and you can re-test
with that :-)

-- 
Jens Axboe