Re: [PATCH net-next v4 2/6] virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit.

2021-09-05 Thread Michael S. Tsirkin
On Fri, Sep 03, 2021 at 09:15:20AM +0300, Arseny Krasnov wrote:
> This bit is used to handle POSIX MSG_EOR flag passed from
> userspace in 'send*()' system calls. It marks end of each
> record and is visible to receiver using 'recvmsg()' system
> call.
> 
> Signed-off-by: Arseny Krasnov 
> Reviewed-by: Stefano Garzarella 

Spec patch for this?

> ---
>  include/uapi/linux/virtio_vsock.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/uapi/linux/virtio_vsock.h 
> b/include/uapi/linux/virtio_vsock.h
> index 8485b004a5f8..64738838bee5 100644
> --- a/include/uapi/linux/virtio_vsock.h
> +++ b/include/uapi/linux/virtio_vsock.h
> @@ -98,6 +98,7 @@ enum virtio_vsock_shutdown {
>  /* VIRTIO_VSOCK_OP_RW flags values */
>  enum virtio_vsock_rw {
>   VIRTIO_VSOCK_SEQ_EOM = 1,
> + VIRTIO_VSOCK_SEQ_EOR = 2,
>  };
>  
>  #endif /* _UAPI_LINUX_VIRTIO_VSOCK_H */
> -- 
> 2.25.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH net-next v5 0/6] virtio/vsock: introduce MSG_EOR flag for SEQPACKET

2021-09-05 Thread Michael S. Tsirkin
On Sun, Sep 05, 2021 at 07:21:10PM +0300, Arseny Krasnov wrote:
> 
> On 05.09.2021 19:19, Michael S. Tsirkin wrote:
> > On Sun, Sep 05, 2021 at 07:02:44PM +0300, Arseny Krasnov wrote:
> >> On 05.09.2021 18:55, Michael S. Tsirkin wrote:
> >>> On Fri, Sep 03, 2021 at 03:30:13PM +0300, Arseny Krasnov wrote:
>   This patchset implements support of MSG_EOR bit for SEQPACKET
>  AF_VSOCK sockets over virtio transport.
>   First we need to define 'messages' and 'records' like this:
>  Message is result of sending calls: 'write()', 'send()', 'sendmsg()'
>  etc. It has fixed maximum length, and it bounds are visible using
>  return from receive calls: 'read()', 'recv()', 'recvmsg()' etc.
>  Current implementation based on message definition above.
>   Record has unlimited length, it consists of multiple message,
>  and bounds of record are visible via MSG_EOR flag returned from
>  'recvmsg()' call. Sender passes MSG_EOR to sending system call and
>  receiver will see MSG_EOR when corresponding message will be processed.
>   Idea of patchset comes from POSIX: it says that SEQPACKET
>  supports record boundaries which are visible for receiver using
>  MSG_EOR bit. So, it looks like MSG_EOR is enough thing for SEQPACKET
>  and we don't need to maintain boundaries of corresponding send -
>  receive system calls. But, for 'sendXXX()' and 'recXXX()' POSIX says,
>  that all these calls operates with messages, e.g. 'sendXXX()' sends
>  message, while 'recXXX()' reads messages and for SEQPACKET, 'recXXX()'
>  must read one entire message from socket, dropping all out of size
>  bytes. Thus, both message boundaries and MSG_EOR bit must be supported
>  to follow POSIX rules.
>   To support MSG_EOR new bit was added along with existing
>  'VIRTIO_VSOCK_SEQ_EOR': 'VIRTIO_VSOCK_SEQ_EOM'(end-of-message) - now it
>  works in the same way as 'VIRTIO_VSOCK_SEQ_EOR'. But 
>  'VIRTIO_VSOCK_SEQ_EOR'
>  is used to mark 'MSG_EOR' bit passed from userspace.
>   This patchset includes simple test for MSG_EOR.
> >>> I'm prepared to merge this for this window,
> >>> but I'm not sure who's supposed to ack the net/vmw_vsock/af_vsock.c
> >>> bits. It's a harmless variable renaming so maybe it does not matter.
> >>>
> >>> The rest is virtio stuff so I guess my tree is ok.
> >>>
> >>> Objections, anyone?
> >> https://lkml.org/lkml/2021/9/3/76 this is v4. It is same as v5 in 
> >> af_vsock.c changes.
> >>
> >> It has Reviewed by from Stefano Garzarella.
> > Is Stefano the maintainer for af_vsock then?
> > I wasn't sure.
> Ack, let's wait for maintainer's comment


The specific patch is a trivial variable renaming so
I parked this in my tree for now, will merge unless I
hear any objections in the next couple of days.

> >>>
>   Arseny Krasnov(6):
>    virtio/vsock: rename 'EOR' to 'EOM' bit.
>    virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit.
>    vhost/vsock: support MSG_EOR bit processing
>    virtio/vsock: support MSG_EOR bit processing
>    af_vsock: rename variables in receive loop
>    vsock_test: update message bounds test for MSG_EOR
> 
>   drivers/vhost/vsock.c   | 28 +--
>   include/uapi/linux/virtio_vsock.h   |  3 ++-
>   net/vmw_vsock/af_vsock.c| 10 
>   net/vmw_vsock/virtio_transport_common.c | 23 ---
>   tools/testing/vsock/vsock_test.c|  8 ++-
>   5 files changed, 45 insertions(+), 27 deletions(-)
> 
>   v4 -> v5:
>   - Move bitwise and out of le32_to_cpu() in 0003.
> 
>   v3 -> v4:
>   - 'sendXXX()' renamed to 'send*()' in 0002- commit msg.
>   - Comment about bit restore updated in 0003-.
>   - 'same' renamed to 'similar' in 0003- commit msg.
>   - u32 used instead of uint32_t in 0003-.
> 
>   v2 -> v3:
>   - 'virtio/vsock: rename 'EOR' to 'EOM' bit.' - commit message updated.
>   - 'VIRTIO_VSOCK_SEQ_EOR' bit add moved to separate patch.
>   - 'vhost/vsock: support MSG_EOR bit processing' - commit message
> updated.
>   - 'vhost/vsock: support MSG_EOR bit processing' - removed unneeded
> 'le32_to_cpu()', because input argument was already in CPU
> endianness.
> 
>   v1 -> v2:
>   - 'VIRTIO_VSOCK_SEQ_EOR' is renamed to 'VIRTIO_VSOCK_SEQ_EOM', to
> support backward compatibility.
>   - use bitmask of flags to restore in vhost.c, instead of separated
> bool variable for each flag.
>   - test for EAGAIN removed, as logically it is not part of this
> patchset(will be sent separately).
>   - cover letter updated(added part with POSIX description).
> 
>  Signed-off-by: Arseny Krasnov 
>  -- 
>  2.25.1
> >

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org

Re: [PATCH] vhost_net: Convert from atomic_t to refcount_t on vhost_net_ubuf_ref->refcount

2021-09-05 Thread Michael S. Tsirkin
On Sat, Jul 17, 2021 at 06:20:30PM +0800, Xiyu Yang wrote:
> refcount_t type and corresponding API can protect refcounters from
> accidental underflow and overflow and further use-after-free situations.
> 
> Signed-off-by: Xiyu Yang 
> Signed-off-by: Xin Tan 

Pls resubmit after addressing the build bot comments.
Thanks!

> ---
>  drivers/vhost/net.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 6414bd5741b8..e23150ca7d4c 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -5,6 +5,7 @@
>   * virtio-net server in host kernel.
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -92,7 +93,7 @@ struct vhost_net_ubuf_ref {
>*  1: no outstanding ubufs
>* >1: outstanding ubufs
>*/
> - atomic_t refcount;
> + refcount_t refcount;
>   wait_queue_head_t wait;
>   struct vhost_virtqueue *vq;
>  };
> @@ -240,7 +241,7 @@ vhost_net_ubuf_alloc(struct vhost_virtqueue *vq, bool 
> zcopy)
>   ubufs = kmalloc(sizeof(*ubufs), GFP_KERNEL);
>   if (!ubufs)
>   return ERR_PTR(-ENOMEM);
> - atomic_set(>refcount, 1);
> + refcount_set(>refcount, 1);
>   init_waitqueue_head(>wait);
>   ubufs->vq = vq;
>   return ubufs;
> @@ -248,7 +249,8 @@ vhost_net_ubuf_alloc(struct vhost_virtqueue *vq, bool 
> zcopy)
>  
>  static int vhost_net_ubuf_put(struct vhost_net_ubuf_ref *ubufs)
>  {
> - int r = atomic_sub_return(1, >refcount);
> + refcount_dec(>refcount);
> + int r = refcount_read(>refcount);
>   if (unlikely(!r))
>   wake_up(>wait);
>   return r;
> @@ -257,7 +259,7 @@ static int vhost_net_ubuf_put(struct vhost_net_ubuf_ref 
> *ubufs)
>  static void vhost_net_ubuf_put_and_wait(struct vhost_net_ubuf_ref *ubufs)
>  {
>   vhost_net_ubuf_put(ubufs);
> - wait_event(ubufs->wait, !atomic_read(>refcount));
> + wait_event(ubufs->wait, !refcount_read(>refcount));
>  }
>  
>  static void vhost_net_ubuf_put_wait_and_free(struct vhost_net_ubuf_ref 
> *ubufs)
> @@ -909,7 +911,7 @@ static void handle_tx_zerocopy(struct vhost_net *net, 
> struct socket *sock)
>   ctl.ptr = ubuf;
>   msg.msg_controllen = sizeof(ctl);
>   ubufs = nvq->ubufs;
> - atomic_inc(>refcount);
> + refcount_inc(>refcount);
>   nvq->upend_idx = (nvq->upend_idx + 1) % UIO_MAXIOV;
>   } else {
>   msg.msg_control = NULL;
> @@ -1384,7 +1386,7 @@ static void vhost_net_flush(struct vhost_net *n)
>   vhost_net_ubuf_put_and_wait(n->vqs[VHOST_NET_VQ_TX].ubufs);
>   mutex_lock(>vqs[VHOST_NET_VQ_TX].vq.mutex);
>   n->tx_flush = false;
> - atomic_set(>vqs[VHOST_NET_VQ_TX].ubufs->refcount, 1);
> + refcount_set(>vqs[VHOST_NET_VQ_TX].ubufs->refcount, 1);
>   mutex_unlock(>vqs[VHOST_NET_VQ_TX].vq.mutex);
>   }
>  }
> -- 
> 2.7.4

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH net-next v5 0/6] virtio/vsock: introduce MSG_EOR flag for SEQPACKET

2021-09-05 Thread Michael S. Tsirkin
On Fri, Sep 03, 2021 at 03:30:13PM +0300, Arseny Krasnov wrote:
>   This patchset implements support of MSG_EOR bit for SEQPACKET
> AF_VSOCK sockets over virtio transport.
>   First we need to define 'messages' and 'records' like this:
> Message is result of sending calls: 'write()', 'send()', 'sendmsg()'
> etc. It has fixed maximum length, and it bounds are visible using
> return from receive calls: 'read()', 'recv()', 'recvmsg()' etc.
> Current implementation based on message definition above.
>   Record has unlimited length, it consists of multiple message,
> and bounds of record are visible via MSG_EOR flag returned from
> 'recvmsg()' call. Sender passes MSG_EOR to sending system call and
> receiver will see MSG_EOR when corresponding message will be processed.
>   Idea of patchset comes from POSIX: it says that SEQPACKET
> supports record boundaries which are visible for receiver using
> MSG_EOR bit. So, it looks like MSG_EOR is enough thing for SEQPACKET
> and we don't need to maintain boundaries of corresponding send -
> receive system calls. But, for 'sendXXX()' and 'recXXX()' POSIX says,
> that all these calls operates with messages, e.g. 'sendXXX()' sends
> message, while 'recXXX()' reads messages and for SEQPACKET, 'recXXX()'
> must read one entire message from socket, dropping all out of size
> bytes. Thus, both message boundaries and MSG_EOR bit must be supported
> to follow POSIX rules.
>   To support MSG_EOR new bit was added along with existing
> 'VIRTIO_VSOCK_SEQ_EOR': 'VIRTIO_VSOCK_SEQ_EOM'(end-of-message) - now it
> works in the same way as 'VIRTIO_VSOCK_SEQ_EOR'. But 'VIRTIO_VSOCK_SEQ_EOR'
> is used to mark 'MSG_EOR' bit passed from userspace.
>   This patchset includes simple test for MSG_EOR.


I'm prepared to merge this for this window,
but I'm not sure who's supposed to ack the net/vmw_vsock/af_vsock.c
bits. It's a harmless variable renaming so maybe it does not matter.

The rest is virtio stuff so I guess my tree is ok.

Objections, anyone?



>  Arseny Krasnov(6):
>   virtio/vsock: rename 'EOR' to 'EOM' bit.
>   virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit.
>   vhost/vsock: support MSG_EOR bit processing
>   virtio/vsock: support MSG_EOR bit processing
>   af_vsock: rename variables in receive loop
>   vsock_test: update message bounds test for MSG_EOR
> 
>  drivers/vhost/vsock.c   | 28 +--
>  include/uapi/linux/virtio_vsock.h   |  3 ++-
>  net/vmw_vsock/af_vsock.c| 10 
>  net/vmw_vsock/virtio_transport_common.c | 23 ---
>  tools/testing/vsock/vsock_test.c|  8 ++-
>  5 files changed, 45 insertions(+), 27 deletions(-)
> 
>  v4 -> v5:
>  - Move bitwise and out of le32_to_cpu() in 0003.
> 
>  v3 -> v4:
>  - 'sendXXX()' renamed to 'send*()' in 0002- commit msg.
>  - Comment about bit restore updated in 0003-.
>  - 'same' renamed to 'similar' in 0003- commit msg.
>  - u32 used instead of uint32_t in 0003-.
> 
>  v2 -> v3:
>  - 'virtio/vsock: rename 'EOR' to 'EOM' bit.' - commit message updated.
>  - 'VIRTIO_VSOCK_SEQ_EOR' bit add moved to separate patch.
>  - 'vhost/vsock: support MSG_EOR bit processing' - commit message
>updated.
>  - 'vhost/vsock: support MSG_EOR bit processing' - removed unneeded
>'le32_to_cpu()', because input argument was already in CPU
>endianness.
> 
>  v1 -> v2:
>  - 'VIRTIO_VSOCK_SEQ_EOR' is renamed to 'VIRTIO_VSOCK_SEQ_EOM', to
>support backward compatibility.
>  - use bitmask of flags to restore in vhost.c, instead of separated
>bool variable for each flag.
>  - test for EAGAIN removed, as logically it is not part of this
>patchset(will be sent separately).
>  - cover letter updated(added part with POSIX description).
> 
> Signed-off-by: Arseny Krasnov 
> -- 
> 2.25.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 1/1] virtio-blk: add num_request_queues module parameter

2021-09-05 Thread Michael S. Tsirkin
On Sun, Sep 05, 2021 at 01:20:24PM +0300, Leon Romanovsky wrote:
> On Sun, Sep 05, 2021 at 01:49:46AM -0700, Chaitanya Kulkarni wrote:
> > 
> > On 9/5/2021 12:46 AM, Leon Romanovsky wrote:
> > > > +static unsigned int num_request_queues;
> > > > +module_param_cb(num_request_queues, _count_ops, 
> > > > _request_queues,
> > > > +   0644);
> > > > +MODULE_PARM_DESC(num_request_queues,
> > > > +"Number of request queues to use for blk device. 
> > > > Should > 0");
> > > > +
> > > Won't it limit all virtio block devices to the same limit?
> > > 
> > > It is very common to see multiple virtio-blk devices on the same system
> > > and they probably need different limits.
> > > 
> > > Thanks
> > 
> > 
> > Without looking into the code, that can be done adding a configfs
> > 
> > interface and overriding a global value (module param) when it is set from
> > 
> > configfs.
> 
> So why should we do double work instead of providing one working
> interface from the beginning?
> 
> Thanks
> 
> > 
> > 

The main way to do it is really from the hypervisor. This one
is a pretty blunt instrument, Max here says it's useful to reduce
memory usage of the driver. If that's the usecase then a global limit
seems sufficient.

-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH net-next v5 0/6] virtio/vsock: introduce MSG_EOR flag for SEQPACKET

2021-09-05 Thread Michael S. Tsirkin
On Sun, Sep 05, 2021 at 07:02:44PM +0300, Arseny Krasnov wrote:
> 
> On 05.09.2021 18:55, Michael S. Tsirkin wrote:
> > On Fri, Sep 03, 2021 at 03:30:13PM +0300, Arseny Krasnov wrote:
> >>This patchset implements support of MSG_EOR bit for SEQPACKET
> >> AF_VSOCK sockets over virtio transport.
> >>First we need to define 'messages' and 'records' like this:
> >> Message is result of sending calls: 'write()', 'send()', 'sendmsg()'
> >> etc. It has fixed maximum length, and it bounds are visible using
> >> return from receive calls: 'read()', 'recv()', 'recvmsg()' etc.
> >> Current implementation based on message definition above.
> >>Record has unlimited length, it consists of multiple message,
> >> and bounds of record are visible via MSG_EOR flag returned from
> >> 'recvmsg()' call. Sender passes MSG_EOR to sending system call and
> >> receiver will see MSG_EOR when corresponding message will be processed.
> >>Idea of patchset comes from POSIX: it says that SEQPACKET
> >> supports record boundaries which are visible for receiver using
> >> MSG_EOR bit. So, it looks like MSG_EOR is enough thing for SEQPACKET
> >> and we don't need to maintain boundaries of corresponding send -
> >> receive system calls. But, for 'sendXXX()' and 'recXXX()' POSIX says,
> >> that all these calls operates with messages, e.g. 'sendXXX()' sends
> >> message, while 'recXXX()' reads messages and for SEQPACKET, 'recXXX()'
> >> must read one entire message from socket, dropping all out of size
> >> bytes. Thus, both message boundaries and MSG_EOR bit must be supported
> >> to follow POSIX rules.
> >>To support MSG_EOR new bit was added along with existing
> >> 'VIRTIO_VSOCK_SEQ_EOR': 'VIRTIO_VSOCK_SEQ_EOM'(end-of-message) - now it
> >> works in the same way as 'VIRTIO_VSOCK_SEQ_EOR'. But 'VIRTIO_VSOCK_SEQ_EOR'
> >> is used to mark 'MSG_EOR' bit passed from userspace.
> >>This patchset includes simple test for MSG_EOR.
> >
> > I'm prepared to merge this for this window,
> > but I'm not sure who's supposed to ack the net/vmw_vsock/af_vsock.c
> > bits. It's a harmless variable renaming so maybe it does not matter.
> >
> > The rest is virtio stuff so I guess my tree is ok.
> >
> > Objections, anyone?
> 
> https://lkml.org/lkml/2021/9/3/76 this is v4. It is same as v5 in af_vsock.c 
> changes.
> 
> It has Reviewed by from Stefano Garzarella.

Is Stefano the maintainer for af_vsock then?
I wasn't sure.

> >
> >
> >>  Arseny Krasnov(6):
> >>   virtio/vsock: rename 'EOR' to 'EOM' bit.
> >>   virtio/vsock: add 'VIRTIO_VSOCK_SEQ_EOR' bit.
> >>   vhost/vsock: support MSG_EOR bit processing
> >>   virtio/vsock: support MSG_EOR bit processing
> >>   af_vsock: rename variables in receive loop
> >>   vsock_test: update message bounds test for MSG_EOR
> >>
> >>  drivers/vhost/vsock.c   | 28 +--
> >>  include/uapi/linux/virtio_vsock.h   |  3 ++-
> >>  net/vmw_vsock/af_vsock.c| 10 
> >>  net/vmw_vsock/virtio_transport_common.c | 23 ---
> >>  tools/testing/vsock/vsock_test.c|  8 ++-
> >>  5 files changed, 45 insertions(+), 27 deletions(-)
> >>
> >>  v4 -> v5:
> >>  - Move bitwise and out of le32_to_cpu() in 0003.
> >>
> >>  v3 -> v4:
> >>  - 'sendXXX()' renamed to 'send*()' in 0002- commit msg.
> >>  - Comment about bit restore updated in 0003-.
> >>  - 'same' renamed to 'similar' in 0003- commit msg.
> >>  - u32 used instead of uint32_t in 0003-.
> >>
> >>  v2 -> v3:
> >>  - 'virtio/vsock: rename 'EOR' to 'EOM' bit.' - commit message updated.
> >>  - 'VIRTIO_VSOCK_SEQ_EOR' bit add moved to separate patch.
> >>  - 'vhost/vsock: support MSG_EOR bit processing' - commit message
> >>updated.
> >>  - 'vhost/vsock: support MSG_EOR bit processing' - removed unneeded
> >>'le32_to_cpu()', because input argument was already in CPU
> >>endianness.
> >>
> >>  v1 -> v2:
> >>  - 'VIRTIO_VSOCK_SEQ_EOR' is renamed to 'VIRTIO_VSOCK_SEQ_EOM', to
> >>support backward compatibility.
> >>  - use bitmask of flags to restore in vhost.c, instead of separated
> >>bool variable for each flag.
> >>  - test for EAGAIN removed, as logically it is not part of this
> >>patchset(will be sent separately).
> >>  - cover letter updated(added part with POSIX description).
> >>
> >> Signed-off-by: Arseny Krasnov 
> >> -- 
> >> 2.25.1
> >

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v14] i2c: virtio: add a virtio i2c frontend driver

2021-09-05 Thread Viresh Kumar
On 04-09-21, 16:01, Michael S. Tsirkin wrote:
> Same as with qemu bits, I am confused as to what is the status
> of proposed spec changes and whether the driver will work
> with them.

This is already merged as well.

The current version simply fails to transmit the messages in case the
length of a buffer is 0. I have patch ready to make it work with the
proposed spec changes, just waiting for them to be accepted.

> Let's let the dust settle on them then, then
> resubmit?

It doesn't break anything for now since we don't have much users and
we know zero length transfers don't work. I will submit the necessary
changes once spec is finalized.

-- 
viresh
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v13 03/13] file: Export receive_fd() to modules

2021-09-05 Thread Michael S. Tsirkin
On Tue, Aug 31, 2021 at 06:36:24PM +0800, Xie Yongji wrote:
> Export receive_fd() so that some modules can use
> it to pass file descriptor between processes without
> missing any security stuffs.
> 
> Signed-off-by: Xie Yongji 
> Acked-by: Jason Wang 

This needs some acks from fs devels.
Viro?


> ---
>  fs/file.c| 6 ++
>  include/linux/file.h | 7 +++
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/file.c b/fs/file.c
> index 86dc9956af32..210e540672aa 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -1134,6 +1134,12 @@ int receive_fd_replace(int new_fd, struct file *file, 
> unsigned int o_flags)
>   return new_fd;
>  }
>  
> +int receive_fd(struct file *file, unsigned int o_flags)
> +{
> + return __receive_fd(file, NULL, o_flags);
> +}
> +EXPORT_SYMBOL_GPL(receive_fd);
> +
>  static int ksys_dup3(unsigned int oldfd, unsigned int newfd, int flags)
>  {
>   int err = -EBADF;
> diff --git a/include/linux/file.h b/include/linux/file.h
> index 2de2e4613d7b..51e830b4fe3a 100644
> --- a/include/linux/file.h
> +++ b/include/linux/file.h
> @@ -94,6 +94,9 @@ extern void fd_install(unsigned int fd, struct file *file);
>  
>  extern int __receive_fd(struct file *file, int __user *ufd,
>   unsigned int o_flags);
> +
> +extern int receive_fd(struct file *file, unsigned int o_flags);
> +
>  static inline int receive_fd_user(struct file *file, int __user *ufd,
> unsigned int o_flags)
>  {
> @@ -101,10 +104,6 @@ static inline int receive_fd_user(struct file *file, int 
> __user *ufd,
>   return -EFAULT;
>   return __receive_fd(file, ufd, o_flags);
>  }
> -static inline int receive_fd(struct file *file, unsigned int o_flags)
> -{
> - return __receive_fd(file, NULL, o_flags);
> -}
>  int receive_fd_replace(int new_fd, struct file *file, unsigned int o_flags);
>  
>  extern void flush_delayed_fput(void);
> -- 
> 2.11.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v2 1/1] virtio-blk: add num_io_queues module parameter

2021-09-05 Thread Michael S. Tsirkin
On Thu, Sep 02, 2021 at 02:45:52PM +0100, Stefan Hajnoczi wrote:
> On Tue, Aug 31, 2021 at 04:50:35PM +0300, Max Gurtovoy wrote:
> > Sometimes a user would like to control the amount of IO queues to be
> > created for a block device. For example, for limiting the memory
> > footprint of virtio-blk devices.
> > 
> > Signed-off-by: Max Gurtovoy 
> > ---
> > 
> > changes from v1:
> >  - use param_set_uint_minmax (from Christoph)
> >  - added "Should > 0" to module description
> > 
> > Note: This commit apply on top of Jens's branch for-5.15/drivers
> > ---
> >  drivers/block/virtio_blk.c | 20 +++-
> >  1 file changed, 19 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> > index 4b49df2dfd23..9332fc4e9b31 100644
> > --- a/drivers/block/virtio_blk.c
> > +++ b/drivers/block/virtio_blk.c
> > @@ -24,6 +24,22 @@
> >  /* The maximum number of sg elements that fit into a virtqueue */
> >  #define VIRTIO_BLK_MAX_SG_ELEMS 32768
> >  
> > +static int virtblk_queue_count_set(const char *val,
> > +   const struct kernel_param *kp)
> > +{
> > +   return param_set_uint_minmax(val, kp, 1, nr_cpu_ids);
> > +}
> > +
> > +static const struct kernel_param_ops queue_count_ops = {
> > +   .set = virtblk_queue_count_set,
> > +   .get = param_get_uint,
> > +};
> > +
> > +static unsigned int num_io_queues;
> > +module_param_cb(num_io_queues, _count_ops, _io_queues, 0644);
> > +MODULE_PARM_DESC(num_io_queues,
> > +"Number of IO virt queues to use for blk device. Should > 0");
> > +
> >  static int major;
> >  static DEFINE_IDA(vd_index_ida);
> >  
> > @@ -501,7 +517,9 @@ static int init_vq(struct virtio_blk *vblk)
> > if (err)
> > num_vqs = 1;
> >  
> > -   num_vqs = min_t(unsigned int, nr_cpu_ids, num_vqs);
> > +   num_vqs = min_t(unsigned int,
> > +   min_not_zero(num_io_queues, nr_cpu_ids),
> > +   num_vqs);
> 
> If you respin, please consider calling them request queues. That's the
> terminology from the VIRTIO spec and it's nice to keep it consistent.
> But the purpose of num_io_queues is clear, so:
> 
> Reviewed-by: Stefan Hajnoczi 

I did this:
+static unsigned int num_io_request_queues;
+module_param_cb(num_io_request_queues, _count_ops, 
_io_request_queues, 0644);
+MODULE_PARM_DESC(num_io_request_queues,
+"Limit number of IO request virt queues to use for each 
device. 0 for now limit");

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v13 03/13] file: Export receive_fd() to modules

2021-09-05 Thread Al Viro
On Sun, Sep 05, 2021 at 11:57:22AM -0400, Michael S. Tsirkin wrote:
> On Tue, Aug 31, 2021 at 06:36:24PM +0800, Xie Yongji wrote:
> > Export receive_fd() so that some modules can use
> > it to pass file descriptor between processes without
> > missing any security stuffs.
> > 
> > Signed-off-by: Xie Yongji 
> > Acked-by: Jason Wang 
> 
> This needs some acks from fs devels.
> Viro?

*shrug*

I still think that having random ioctls messing with descriptor table
is a bad idea, and I'm not thrilled with the way it's currently
factored, but we can change that later.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[vhost:vhost 37/40] drivers/block/virtio_blk.c:30:16: error: implicit declaration of function 'param_set_uint_minmax'; did you mean 'param_set_uint'?

2021-09-05 Thread kernel test robot
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
head:   ebf3b42574b049bd40f6ea04f4e6845664d5b6d5
commit: dc1aba74cc840c5b97e7f50a7dbc135b764207a1 [37/40] virtio-blk: add 
num_io_queues module parameter
config: arc-randconfig-r043-20210906 (attached as .config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/commit/?id=dc1aba74cc840c5b97e7f50a7dbc135b764207a1
git remote add vhost 
https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
git fetch --no-tags vhost vhost
git checkout dc1aba74cc840c5b97e7f50a7dbc135b764207a1
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross 
ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   drivers/block/virtio_blk.c: In function 'virtblk_queue_count_set':
>> drivers/block/virtio_blk.c:30:16: error: implicit declaration of function 
>> 'param_set_uint_minmax'; did you mean 'param_set_uint'? 
>> [-Werror=implicit-function-declaration]
  30 | return param_set_uint_minmax(val, kp, 1, nr_cpu_ids);
 |^
 |param_set_uint
   cc1: some warnings being treated as errors


vim +30 drivers/block/virtio_blk.c

26  
27  static int virtblk_queue_count_set(const char *val,
28  const struct kernel_param *kp)
29  {
  > 30  return param_set_uint_minmax(val, kp, 1, nr_cpu_ids);
31  }
32  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[vhost:vhost 37/40] drivers/block/virtio_blk.c:30:9: error: implicit declaration of function 'param_set_uint_minmax'; did you mean 'param_set_uint'?

2021-09-05 Thread kernel test robot
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
head:   ebf3b42574b049bd40f6ea04f4e6845664d5b6d5
commit: dc1aba74cc840c5b97e7f50a7dbc135b764207a1 [37/40] virtio-blk: add 
num_io_queues module parameter
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# 
https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/commit/?id=dc1aba74cc840c5b97e7f50a7dbc135b764207a1
git remote add vhost 
https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
git fetch --no-tags vhost vhost
git checkout dc1aba74cc840c5b97e7f50a7dbc135b764207a1
# save the attached .config to linux build tree
make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

   drivers/block/virtio_blk.c: In function 'virtblk_queue_count_set':
>> drivers/block/virtio_blk.c:30:9: error: implicit declaration of function 
>> 'param_set_uint_minmax'; did you mean 'param_set_uint'? 
>> [-Werror=implicit-function-declaration]
  30 |  return param_set_uint_minmax(val, kp, 1, nr_cpu_ids);
 | ^
 | param_set_uint
   cc1: some warnings being treated as errors


vim +30 drivers/block/virtio_blk.c

26  
27  static int virtblk_queue_count_set(const char *val,
28  const struct kernel_param *kp)
29  {
  > 30  return param_set_uint_minmax(val, kp, 1, nr_cpu_ids);
31  }
32  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 1/1] virtio: add VIRTIO_F_IN_ORDER to header file

2021-09-05 Thread Michael S. Tsirkin
On Sun, Sep 05, 2021 at 03:09:11PM +0300, Max Gurtovoy wrote:
> For now only add this definition from the spec. In the future, The
> drivers should negotiate this feature to optimize the performance.
> 
> Signed-off-by: Max Gurtovoy 

So I think IN_ORDER was a mistake since it breaks ability
to do pagefaults efficiently without stopping the ring.
I think that VIRTIO_F_PARTIAL_ORDER is a better option -
am working on finalizing that proposal, will post RSN now.


> ---
>  include/uapi/linux/virtio_config.h | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/uapi/linux/virtio_config.h 
> b/include/uapi/linux/virtio_config.h
> index b5eda06f0d57..3fcdc4ab6f19 100644
> --- a/include/uapi/linux/virtio_config.h
> +++ b/include/uapi/linux/virtio_config.h
> @@ -82,6 +82,12 @@
>  /* This feature indicates support for the packed virtqueue layout. */
>  #define VIRTIO_F_RING_PACKED 34
>  
> +/*
> + * This feature indicates that all buffers are used by the device in the same
> + * order in which they have been made available.
> + */
> +#define VIRTIO_F_IN_ORDER  35
> +
>  /*
>   * This feature indicates that memory accesses by the driver and the
>   * device are ordered in a way described by the platform.
> -- 
> 2.18.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[vhost:vhost 37/40] drivers/block/virtio_blk.c:30:9: error: implicit declaration of function 'param_set_uint_minmax'

2021-09-05 Thread kernel test robot
tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
head:   ebf3b42574b049bd40f6ea04f4e6845664d5b6d5
commit: dc1aba74cc840c5b97e7f50a7dbc135b764207a1 [37/40] virtio-blk: add 
num_io_queues module parameter
config: hexagon-randconfig-r041-20210905 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 
6fe2beba7d2a41964af658c8c59dd172683ef739)
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/commit/?id=dc1aba74cc840c5b97e7f50a7dbc135b764207a1
git remote add vhost 
https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
git fetch --no-tags vhost vhost
git checkout dc1aba74cc840c5b97e7f50a7dbc135b764207a1
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=hexagon 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All errors (new ones prefixed by >>):

>> drivers/block/virtio_blk.c:30:9: error: implicit declaration of function 
>> 'param_set_uint_minmax' [-Werror,-Wimplicit-function-declaration]
   return param_set_uint_minmax(val, kp, 1, nr_cpu_ids);
  ^
   drivers/block/virtio_blk.c:30:9: note: did you mean 'param_set_uint'?
   include/linux/moduleparam.h:432:12: note: 'param_set_uint' declared here
   extern int param_set_uint(const char *val, const struct kernel_param *kp);
  ^
   1 error generated.


vim +/param_set_uint_minmax +30 drivers/block/virtio_blk.c

26  
27  static int virtblk_queue_count_set(const char *val,
28  const struct kernel_param *kp)
29  {
  > 30  return param_set_uint_minmax(val, kp, 1, nr_cpu_ids);
31  }
32  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v6 42/76] x86/sev-es: Setup early #VC handler

2021-09-05 Thread Juergen Gross via Virtualization

On 04.09.21 11:39, Lai Jiangshan wrote:

@@ -363,6 +370,33 @@ SYM_CODE_START_LOCAL(early_idt_handler_common)
 jmp restore_regs_and_return_to_kernel
  SYM_CODE_END(early_idt_handler_common)

+#ifdef CONFIG_AMD_MEM_ENCRYPT
+/*
+ * VC Exception handler used during very early boot. The
+ * early_idt_handler_array can't be used because it returns via the
+ * paravirtualized INTERRUPT_RETURN and pv-ops don't work that early.


Hello Joerg, Juergen

The commit ae755b5a4548 ("x86/paravirt: Switch iret pvops to ALTERNATIVE")
( https://lore.kernel.org/lkml/20210311142319.4723-12-jgr...@suse.com/ )
had been merged and the paravirt_iret is deferenced based via %rip.

Can INTERRUPT_RETURN still be a problem if early_idt_handler_array
is used instead for bringup IDT?


Even before my patch the dereferencing was done via %rip.

I vaguely remember having discussed the pvops usage with Joerg when he
wrote the SEV support. I'm not sure why pvops shouldn't have worked, but
I'm sure its usage makes no sense at all, as long as we don't have SEV
support for Xen PV guests.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v13 05/13] vdpa: Add reset callback in vdpa_config_ops

2021-09-05 Thread Michael S. Tsirkin
On Tue, Aug 31, 2021 at 06:36:26PM +0800, Xie Yongji wrote:
> This adds a new callback to support device specific reset
> behavior. The vdpa bus driver will call the reset function
> instead of setting status to zero during resetting.
> 
> Signed-off-by: Xie Yongji 


This does gloss over a significant change though:


> ---
> @@ -348,12 +352,12 @@ static inline struct device *vdpa_get_dma_dev(struct 
> vdpa_device *vdev)
>   return vdev->dma_dev;
>  }
>  
> -static inline void vdpa_reset(struct vdpa_device *vdev)
> +static inline int vdpa_reset(struct vdpa_device *vdev)
>  {
>   const struct vdpa_config_ops *ops = vdev->config;
>  
>   vdev->features_valid = false;
> - ops->set_status(vdev, 0);
> + return ops->reset(vdev);
>  }
>  
>  static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features)


Unfortunately this breaks virtio_vdpa:


static void virtio_vdpa_reset(struct virtio_device *vdev)
{
struct vdpa_device *vdpa = vd_get_vdpa(vdev);

vdpa_reset(vdpa);
}


and there's no easy way to fix this, kernel can't recover
from a reset failure e.g. during driver unbind.

Find a way to disable virtio_vdpa for now?


> -- 
> 2.11.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v12 05/13] vdpa: Add reset callback in vdpa_config_ops

2021-09-05 Thread Michael S. Tsirkin
On Mon, Aug 30, 2021 at 10:17:29PM +0800, Xie Yongji wrote:
> This adds a new callback to support device specific reset
> behavior. The vdpa bus driver will call the reset function
> instead of setting status to zero during resetting.
> 
> Signed-off-by: Xie Yongji 

This does gloss over a significant change though:

> @@ -348,12 +352,12 @@ static inline struct device *vdpa_get_dma_dev(struct 
> vdpa_device *vdev)
>   return vdev->dma_dev;
>  }
>  
> -static inline void vdpa_reset(struct vdpa_device *vdev)
> +static inline int vdpa_reset(struct vdpa_device *vdev)
>  {
>   const struct vdpa_config_ops *ops = vdev->config;
>  
>   vdev->features_valid = false;
> - ops->set_status(vdev, 0);
> + return ops->reset(vdev);
>  }
>  
>  static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features)

Unfortunately this breaks virtio_vdpa:


static void virtio_vdpa_reset(struct virtio_device *vdev)
{
struct vdpa_device *vdpa = vd_get_vdpa(vdev);

vdpa_reset(vdpa);
}


and there's no easy way to fix this, kernel can't recover
from a reset failure e.g. during driver unbind.

Find a way to disable virtio_vdpa for now?

> -- 
> 2.11.0

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 1/1] virtio-blk: add num_request_queues module parameter

2021-09-05 Thread Leon Romanovsky
On Sun, Sep 05, 2021 at 02:16:58PM +0300, Max Gurtovoy wrote:
> 
> On 9/5/2021 1:19 PM, Leon Romanovsky wrote:
> > On Sun, Sep 05, 2021 at 12:19:09PM +0300, Max Gurtovoy wrote:
> > > On 9/5/2021 11:49 AM, Chaitanya Kulkarni wrote:
> > > > On 9/5/2021 12:46 AM, Leon Romanovsky wrote:
> > > > > > +static unsigned int num_request_queues;
> > > > > > +module_param_cb(num_request_queues, _count_ops,
> > > > > > _request_queues,
> > > > > > +    0644);
> > > > > > +MODULE_PARM_DESC(num_request_queues,
> > > > > > + "Number of request queues to use for blk device.
> > > > > > Should > 0");
> > > > > > +
> > > > > Won't it limit all virtio block devices to the same limit?
> > > > > 
> > > > > It is very common to see multiple virtio-blk devices on the same 
> > > > > system
> > > > > and they probably need different limits.
> > > > > 
> > > > > Thanks
> > > > 
> > > > Without looking into the code, that can be done adding a configfs
> > > > 
> > > > interface and overriding a global value (module param) when it is set
> > > > from
> > > > 
> > > > configfs.
> > > > 
> > > > 
> > > You have many ways to overcome this issue.
> > > 
> > > For example:
> > > 
> > > # ls -l /sys/block/vda/mq/
> > > drwxr-xr-x 18 root root 0 Sep  5 12:14 0
> > > drwxr-xr-x 18 root root 0 Sep  5 12:14 1
> > > drwxr-xr-x 18 root root 0 Sep  5 12:14 2
> > > drwxr-xr-x 18 root root 0 Sep  5 12:14 3
> > > 
> > > # echo virtio0 > /sys/bus/virtio/drivers/virtio_blk/unbind
> > > 
> > > # echo 8 > /sys/module/virtio_blk/parameters/num_request_queues
> > This is global to all virtio-blk devices.
> 
> You define a global module param but you bind/unbind a specific device.
> 
> Do you have a better way to control it ?

One of the possible solutions will be to extend virtio bus to allow
setting of such pre-probe parameters. However I don't know if it is
really worth doing it,

> 
> if the device is already probed, it's too late to change the queue_num.
> 
> 
> > 
> > > # echo virtio0 > /sys/bus/virtio/drivers/virtio_blk/bind
> > > 
> > > # ls -l /sys/block/vda/mq/
> > > drwxr-xr-x 10 root root 0 Sep  5 12:17 0
> > > drwxr-xr-x 10 root root 0 Sep  5 12:17 1
> > > drwxr-xr-x 10 root root 0 Sep  5 12:17 2
> > > drwxr-xr-x 10 root root 0 Sep  5 12:17 3
> > > drwxr-xr-x 10 root root 0 Sep  5 12:17 4
> > > drwxr-xr-x 10 root root 0 Sep  5 12:17 5
> > > drwxr-xr-x 10 root root 0 Sep  5 12:17 6
> > > drwxr-xr-x 10 root root 0 Sep  5 12:17 7
> > > 
> > > -Max.
> > > 
> > > 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 1/1] virtio-blk: add num_request_queues module parameter

2021-09-05 Thread Leon Romanovsky
On Thu, Sep 02, 2021 at 11:46:22PM +0300, Max Gurtovoy wrote:
> Sometimes a user would like to control the amount of request queues to
> be created for a block device. For example, for limiting the memory
> footprint of virtio-blk devices.
> 
> Reviewed-by: Christoph Hellwig 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Max Gurtovoy 
> ---
> 
> changes from v2:
>  - renamed num_io_queues to num_request_queues (from Stefan)
>  - added Reviewed-by signatures (from Stefan and Christoph)
> 
> changes from v1:
>  - use param_set_uint_minmax (from Christoph)
>  - added "Should > 0" to module description
> 
> Note: This commit apply on top of Jens's branch for-5.15/drivers
> 
> ---
>  drivers/block/virtio_blk.c | 21 -
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 4b49df2dfd23..aaa2833a4734 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -24,6 +24,23 @@
>  /* The maximum number of sg elements that fit into a virtqueue */
>  #define VIRTIO_BLK_MAX_SG_ELEMS 32768
>  
> +static int virtblk_queue_count_set(const char *val,
> + const struct kernel_param *kp)
> +{
> + return param_set_uint_minmax(val, kp, 1, nr_cpu_ids);
> +}
> +
> +static const struct kernel_param_ops queue_count_ops = {
> + .set = virtblk_queue_count_set,
> + .get = param_get_uint,
> +};
> +
> +static unsigned int num_request_queues;
> +module_param_cb(num_request_queues, _count_ops, _request_queues,
> + 0644);
> +MODULE_PARM_DESC(num_request_queues,
> +  "Number of request queues to use for blk device. Should > 0");
> +

Won't it limit all virtio block devices to the same limit?

It is very common to see multiple virtio-blk devices on the same system
and they probably need different limits.

Thanks
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 1/1] virtio-blk: add num_request_queues module parameter

2021-09-05 Thread Leon Romanovsky
On Sun, Sep 05, 2021 at 12:19:09PM +0300, Max Gurtovoy wrote:
> 
> On 9/5/2021 11:49 AM, Chaitanya Kulkarni wrote:
> > 
> > On 9/5/2021 12:46 AM, Leon Romanovsky wrote:
> > > > +static unsigned int num_request_queues;
> > > > +module_param_cb(num_request_queues, _count_ops,
> > > > _request_queues,
> > > > +    0644);
> > > > +MODULE_PARM_DESC(num_request_queues,
> > > > + "Number of request queues to use for blk device.
> > > > Should > 0");
> > > > +
> > > Won't it limit all virtio block devices to the same limit?
> > > 
> > > It is very common to see multiple virtio-blk devices on the same system
> > > and they probably need different limits.
> > > 
> > > Thanks
> > 
> > 
> > Without looking into the code, that can be done adding a configfs
> > 
> > interface and overriding a global value (module param) when it is set
> > from
> > 
> > configfs.
> > 
> > 
> You have many ways to overcome this issue.
> 
> For example:
> 
> # ls -l /sys/block/vda/mq/
> drwxr-xr-x 18 root root 0 Sep  5 12:14 0
> drwxr-xr-x 18 root root 0 Sep  5 12:14 1
> drwxr-xr-x 18 root root 0 Sep  5 12:14 2
> drwxr-xr-x 18 root root 0 Sep  5 12:14 3
> 
> # echo virtio0 > /sys/bus/virtio/drivers/virtio_blk/unbind
> 
> # echo 8 > /sys/module/virtio_blk/parameters/num_request_queues

This is global to all virtio-blk devices.

> 
> # echo virtio0 > /sys/bus/virtio/drivers/virtio_blk/bind
> 
> # ls -l /sys/block/vda/mq/
> drwxr-xr-x 10 root root 0 Sep  5 12:17 0
> drwxr-xr-x 10 root root 0 Sep  5 12:17 1
> drwxr-xr-x 10 root root 0 Sep  5 12:17 2
> drwxr-xr-x 10 root root 0 Sep  5 12:17 3
> drwxr-xr-x 10 root root 0 Sep  5 12:17 4
> drwxr-xr-x 10 root root 0 Sep  5 12:17 5
> drwxr-xr-x 10 root root 0 Sep  5 12:17 6
> drwxr-xr-x 10 root root 0 Sep  5 12:17 7
> 
> -Max.
> 
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 1/1] virtio-blk: add num_request_queues module parameter

2021-09-05 Thread Leon Romanovsky
On Sun, Sep 05, 2021 at 01:49:46AM -0700, Chaitanya Kulkarni wrote:
> 
> On 9/5/2021 12:46 AM, Leon Romanovsky wrote:
> > > +static unsigned int num_request_queues;
> > > +module_param_cb(num_request_queues, _count_ops, 
> > > _request_queues,
> > > + 0644);
> > > +MODULE_PARM_DESC(num_request_queues,
> > > +  "Number of request queues to use for blk device. Should > 0");
> > > +
> > Won't it limit all virtio block devices to the same limit?
> > 
> > It is very common to see multiple virtio-blk devices on the same system
> > and they probably need different limits.
> > 
> > Thanks
> 
> 
> Without looking into the code, that can be done adding a configfs
> 
> interface and overriding a global value (module param) when it is set from
> 
> configfs.

So why should we do double work instead of providing one working
interface from the beginning?

Thanks

> 
> 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization