| Message ID | f0b283a1-cc63-dc3d-cc0c-0da7f684d4d2@sberdevices.ru (mailing list archive) |
|---|---|
| Headers | show |
| Series | allocate multiple skbuffs on tx | expand |
Hello Stefano, thanks for review! Since both patches are R-b, i can wait for a few days, then send this as 'net-next'? Thanks, Arseniy On 22.03.2023 21:34, Arseniy Krasnov wrote: > This adds small optimization for tx path: instead of allocating single > skbuff on every call to transport, allocate multiple skbuff's until > credit space allows, thus trying to send as much as possible data without > return to af_vsock.c. > > Also this patchset includes second patch which adds check and return from > 'virtio_transport_get_credit()' and 'virtio_transport_put_credit()' when > these functions are called with 0 argument. This is needed, because zero > argument makes both functions to behave as no-effect, but both of them > always tries to acquire spinlock. Moreover, first patch always calls > function 'virtio_transport_put_credit()' with zero argument in case of > successful packet transmission. > > Link to v1: > https://lore.kernel.org/netdev/2c52aa26-8181-d37a-bccd-a86bd3cbc6e1@sberdevices.ru/ > Link to v2: > https://lore.kernel.org/netdev/ea5725eb-6cb5-cf15-2938-34e335a442fa@sberdevices.ru/ > Link to v3: > https://lore.kernel.org/netdev/f33ef593-982e-2b3f-0986-6d537a3aaf08@sberdevices.ru/ > Link to v4: > https://lore.kernel.org/netdev/0e0c1421-7cdc-2582-b120-cad6f42824bb@sberdevices.ru/ > > Changelog: > v1 -> v2: > - If sent something, return number of bytes sent (even in > case of error). Return error only if failed to sent first > skbuff. > > v2 -> v3: > - Handle case when transport callback returns unexpected value which > is not equal to 'skb->len'. Break loop. > - Don't check for zero value of 'rest_len' before calling > 'virtio_transport_put_credit()'. Decided to add this check directly > to 'virtio_transport_put_credit()' in separate patch. > > v3 -> v4: > - Use WARN_ONCE() to handle case when transport callback returns > unexpected value. > - Remove useless 'ret = -EFAULT;' assignment for case above. > > v4 -> v5: > - Remove extra 'ret' initialization. > - Remove empty extra line before 'if (ret < 0)'. > - Add R-b tag for the first patch. > - Add second patch, thus creating patchset of 2 patches. > > Arseniy Krasnov (2): > virtio/vsock: allocate multiple skbuffs on tx > virtio/vsock: check argument to avoid no effect call > > net/vmw_vsock/virtio_transport_common.c | 63 +++++++++++++++++++------ > 1 file changed, 49 insertions(+), 14 deletions(-) >
On Thu, Mar 23, 2023 at 01:01:40PM +0300, Arseniy Krasnov wrote: >Hello Stefano, > >thanks for review! You're welcome! > >Since both patches are R-b, i can wait for a few days, then send this >as 'net-next'? Yep, maybe even this series could have been directly without RFC ;-) Thanks, Stefano
On 23.03.2023 13:48, Stefano Garzarella wrote: > On Thu, Mar 23, 2023 at 01:01:40PM +0300, Arseniy Krasnov wrote: >> Hello Stefano, >> >> thanks for review! > > You're welcome! > >> >> Since both patches are R-b, i can wait for a few days, then send this >> as 'net-next'? > > Yep, maybe even this series could have been directly without RFC ;-) "directly", You mean 'net' tag? Of just without RFC, like [PATCH v5]. In this case it will be merged to 'net' right? Thanks, Arseniy > > Thanks, > Stefano >
On Thu, Mar 23, 2023 at 01:53:40PM +0300, Arseniy Krasnov wrote: > > >On 23.03.2023 13:48, Stefano Garzarella wrote: >> On Thu, Mar 23, 2023 at 01:01:40PM +0300, Arseniy Krasnov wrote: >>> Hello Stefano, >>> >>> thanks for review! >> >> You're welcome! >> >>> >>> Since both patches are R-b, i can wait for a few days, then send this >>> as 'net-next'? >> >> Yep, maybe even this series could have been directly without RFC ;-) > >"directly", You mean 'net' tag? Of just without RFC, like [PATCH v5]. In this case >it will be merged to 'net' right? Sorry for the confusion. I meant without RFC but with net-next. Being enhancements and not fixes this is definitely net-next material, so even in RFCs you can already use the net-next tag, so the reviewer knows which branch to apply them to. (It's not super important since being RFCs it's expected that it's not complete, but it's definitely an help for the reviewer). Speaking of the RFC, we usually use it for patches that we don't think are ready to be merged. But when they reach a good state (like this series for example), we can start publishing them already without the RFC tag. Anyway, if you are not sure, use RFC and then when a maintainer has reviewed them all, surely you can remove the RFC tag. Hope this helps, at least that's what I usually do, so don't take that as a strict rule ;-) Thanks, Stefano
On 23.03.2023 14:11, Stefano Garzarella wrote: > On Thu, Mar 23, 2023 at 01:53:40PM +0300, Arseniy Krasnov wrote: >> >> >> On 23.03.2023 13:48, Stefano Garzarella wrote: >>> On Thu, Mar 23, 2023 at 01:01:40PM +0300, Arseniy Krasnov wrote: >>>> Hello Stefano, >>>> >>>> thanks for review! >>> >>> You're welcome! >>> >>>> >>>> Since both patches are R-b, i can wait for a few days, then send this >>>> as 'net-next'? >>> >>> Yep, maybe even this series could have been directly without RFC ;-) >> >> "directly", You mean 'net' tag? Of just without RFC, like [PATCH v5]. In this case >> it will be merged to 'net' right? > > Sorry for the confusion. I meant without RFC but with net-next. > > Being enhancements and not fixes this is definitely net-next material, > so even in RFCs you can already use the net-next tag, so the reviewer > knows which branch to apply them to. (It's not super important since > being RFCs it's expected that it's not complete, but it's definitely an > help for the reviewer). > > Speaking of the RFC, we usually use it for patches that we don't think > are ready to be merged. But when they reach a good state (like this > series for example), we can start publishing them already without the > RFC tag. > > Anyway, if you are not sure, use RFC and then when a maintainer has > reviewed them all, surely you can remove the RFC tag. > > Hope this helps, at least that's what I usually do, so don't take that > as a strict rule ;-) Ah ok, I see now, thanks for details Thanks, Arseniy > > Thanks, > Stefano >