All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Dongli Zhang <dongli.zhang@oracle.com>
Cc: virtualization@lists.linux.dev, jasowangio@gmail.com,
	michael.christie@oracle.com, pbonzini@redhat.com,
	stefanha@redhat.com, eperezma@redhat.com, kvm@vger.kernel.org,
	joe.jin@oracle.com
Subject: Re: [PATCH 2/2] vhost-scsi: reject invalid max_io_vqs module parameter
Date: Sat, 1 Aug 2026 05:55:37 -0400	[thread overview]
Message-ID: <20260801055436-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <d24ab42f-fa00-4a5c-b987-c05f6a9898f2@oracle.com>

On Fri, Jul 31, 2026 at 11:59:21PM -0700, Dongli Zhang wrote:
> 
> 
> On 2026-07-31 5:00 PM, Michael S. Tsirkin wrote:
> > On Fri, Jul 31, 2026 at 03:10:24PM -0700, Dongli Zhang wrote:
> >> max_io_vqs is currently validated only when a vhost-scsi device is opened.
> >> This allows sysfs to show values larger than the driver will actually use,
> >> e.g. writing 2048 succeeds even though vhost_scsi_open() later clamps it to
> >> VHOST_SCSI_MAX_IO_VQ. This makes the sysfs value differ from the value that
> >> will actually be used.
> >> 
> >> hv# echo 2048 > /sys/module/vhost_scsi/parameters/max_io_vqs
> >> 
> >> hv# cat /sys/module/vhost_scsi/parameters/max_io_vqs
> >> 2048
> >> 
> >> [  315.630495] Invalid max_io_vqs of 2048. Using 1024.
> >> 
> >> Use a module parameter callback to reject out-of-range updates when the
> >> parameter is written. This keeps the visible parameter value consistent
> >> with the effective queue limit.
> >> 
> >> With the parameter value kept in range, remove the duplicate validation
> >> from vhost_scsi_open().
> >> 
> >> Invalid values now fail with -EINVAL at write/load time instead of being
> >> accepted and silently adjusted later when the device is opened.
> >> 
> >> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
> > 
> > why should we change userspace visible behaviour now?
> > 
> 
> Everything started with PATCH 1/1.
> 
> The original purpose was to resolve the memory allocation failure addressed by
> PATCH 1/1.
> 
> To validate the bugfix, I increased max_io_vqs to see what would happen.
> 
> To my surprise, I was able to increase max_io_vqs to 2048 even though
> VHOST_SCSI_MAX_IO_VQ is 1024.
> 
> hv# echo 2048 > /sys/module/vhost_scsi/parameters/max_io_vqs
> 
> hv# cat /sys/module/vhost_scsi/parameters/max_io_vqs
> 2048
> 
> However, a value of 2048 does not work when creating a guest VM with vhost-scsi
> because vhost_scsi_open() fails.
> 
> This behavior is quite confusing.
> 
> Therefore, PATCH 2/2 was proposed to avoid confusing users. If 2048 is an
> invalid value, then setting max_io_vqs to 2048 should fail explicitly.
> 
> Is this really a "userspace-visible behavior change"? Without PATCH 1/1,
> increasing the default value to 256 could already fail because of memory
> allocation. Perhaps this is a good opportunity to define the expected behavior
> of max_io_vqs before more users start increasing its value.
> 
> This also makes the behavior of max_io_vqs consistent with inline_sg_cnt. For
> module parameters, it may be better to perform sanity checks when the parameter
> is configured rather than at runtime.
> 
> Thank you very much!
> 
> Dongli Zhang


There's no telling if someone had a large value configured and now the
module will start failing for them. I can accept making the read return
the actual value, if that helps.


      reply	other threads:[~2026-08-01  9:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-31 22:10 [PATCH 0/2] vhost-scsi: fix max_io_vqs handling Dongli Zhang
2026-07-31 22:10 ` [PATCH 1/2] vhost-scsi: use kvzalloc for vq array allocation Dongli Zhang
2026-07-31 22:24   ` sashiko-bot
2026-07-31 23:23     ` Dongli Zhang
2026-07-31 22:10 ` [PATCH 2/2] vhost-scsi: reject invalid max_io_vqs module parameter Dongli Zhang
2026-07-31 22:26   ` sashiko-bot
2026-07-31 23:31     ` Dongli Zhang
2026-08-01  0:00   ` Michael S. Tsirkin
2026-08-01  6:59     ` Dongli Zhang
2026-08-01  9:55       ` Michael S. Tsirkin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260801055436-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=dongli.zhang@oracle.com \
    --cc=eperezma@redhat.com \
    --cc=jasowangio@gmail.com \
    --cc=joe.jin@oracle.com \
    --cc=kvm@vger.kernel.org \
    --cc=michael.christie@oracle.com \
    --cc=pbonzini@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=virtualization@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.