Re: Stack trace with Samba VFS io_uring and large transfers

[Forza <forza@tnonline.net>]

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?