On Thu, Oct 08, 2020 at 04:28:40PM -0400, Michael S. Tsirkin wrote: > On Thu, Oct 08, 2020 at 10:24:36PM +0200, Stefano Garzarella wrote: > > On Thu, Oct 08, 2020 at 04:00:51PM -0400, Michael S. Tsirkin wrote: > > > On Thu, Oct 08, 2020 at 06:13:11PM +0200, Stefano Garzarella wrote: > > > > If riov and wiov are both defined and they point to different > > > > objects, only riov is initialized. If the wiov is not initialized > > > > by the caller, the function fails returning -EINVAL and printing > > > > "Readable desc 0x... after writable" error message. > > > > > > > > Let's replace the 'else if' clause with 'if' to initialize both > > > > riov and wiov if they are not NULL. > > > > > > > > As checkpatch pointed out, we also avoid crashing the kernel > > > > when riov and wiov are both NULL, replacing BUG() with WARN_ON() > > > > and returning -EINVAL. > > > > > > > > Fixes: f87d0fbb5798 ("vringh: host-side implementation of virtio rings.") > > > > Cc: stable@xxxxxxxxxxxxxxx > > > > Signed-off-by: Stefano Garzarella <sgarzare@xxxxxxxxxx> > > > > > > Can you add more detail please? when does this trigger? > > > > I'm developing vdpa_sim_blk and I'm using vringh_getdesc_iotlb() > > to get readable and writable buffers. > > > > With virtio-blk devices a descriptors has both readable and writable > > buffers (eg. virtio_blk_outhdr in the readable buffer and status as last byte > > of writable buffer). > > So, I'm calling vringh_getdesc_iotlb() one time to get both type of buffer > > and put them in 2 iovecs: > > > > ret = vringh_getdesc_iotlb(&vq->vring, &vq->riov, &vq->wiov, > > &vq->head, GFP_ATOMIC); > > > > With this patch applied it works well, without the function fails > > returning -EINVAL and printing "Readable desc 0x... after writable". > > > > Am I using vringh_getdesc_iotlb() in the wrong way? > > > > Thanks, > > Stefano > > > > > I think it's ok, this info just needs to be in the commit log ... Sure, I'll send a v2 adding this info. Sorry for not adding it earlier! Thanks, Stefano