All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Stefano Garzarella <sgarzare@redhat.com>
Cc: virtualization@lists.linux.dev,
	"Xuan Zhuo" <xuanzhuo@linux.alibaba.com>,
	"Jason Wang" <jasowang@redhat.com>,
	linux-kernel@vger.kernel.org,
	"Eugenio Pérez" <eperezma@redhat.com>
Subject: Re: [PATCH] vdpa_sim_blk: add `capacity` module parameter
Date: Fri, 5 Jul 2024 07:30:51 -0400	[thread overview]
Message-ID: <20240705073017-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20240705112821.144819-1-sgarzare@redhat.com>

On Fri, Jul 05, 2024 at 01:28:21PM +0200, Stefano Garzarella wrote:
> The vDPA block simulator always allocated a 128 MiB ram-disk, but some
> filesystems (e.g. XFS) may require larger minimum sizes (see
> https://issues.redhat.com/browse/RHEL-45951).
> 
> So to allow us to test these filesystems, let's add a module parameter
> to control the size of the simulated virtio-blk devices.
> The value is mapped directly to the `capacity` field of the virtio-blk
> configuration space, so it must be expressed in sector numbers of 512
> bytes.
> 
> The default value (0x40000) is the same as the previous value, so the
> behavior without setting `capacity` remains unchanged.
> 
> Before this patch or with this patch without setting `capacity`:
>   $ modprobe vdpa-sim-blk
>   $ vdpa dev add mgmtdev vdpasim_blk name blk0
>   virtio_blk virtio6: 1/0/0 default/read/poll queues
>   virtio_blk virtio6: [vdb] 262144 512-byte logical blocks (134 MB/128 MiB)
> 
> After this patch:
>   $ modprobe vdpa-sim-blk capacity=614400
>   $ vdpa dev add mgmtdev vdpasim_blk name blk0
>   virtio_blk virtio6: 1/0/0 default/read/poll queues
>   virtio_blk virtio6: [vdb] 614400 512-byte logical blocks (315 MB/300 MiB)
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>

What a hack. Cindy was working on adding control over config
space, why can't that be used?

> ---
>  drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> index b137f3679343..18f390149836 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> @@ -33,7 +33,6 @@
>  				 (1ULL << VIRTIO_BLK_F_DISCARD)  | \
>  				 (1ULL << VIRTIO_BLK_F_WRITE_ZEROES))
>  
> -#define VDPASIM_BLK_CAPACITY	0x40000
>  #define VDPASIM_BLK_SIZE_MAX	0x1000
>  #define VDPASIM_BLK_SEG_MAX	32
>  #define VDPASIM_BLK_DWZ_MAX_SECTORS UINT_MAX
> @@ -43,6 +42,10 @@
>  #define VDPASIM_BLK_AS_NUM	1
>  #define VDPASIM_BLK_GROUP_NUM	1
>  
> +static unsigned long capacity = 0x40000;
> +module_param(capacity, ulong, 0444);
> +MODULE_PARM_DESC(capacity, "virtio-blk device capacity (in 512-byte sectors)");
> +
>  struct vdpasim_blk {
>  	struct vdpasim vdpasim;
>  	void *buffer;
> @@ -79,10 +82,10 @@ static void vdpasim_blk_buffer_unlock(struct vdpasim_blk *blk)
>  static bool vdpasim_blk_check_range(struct vdpasim *vdpasim, u64 start_sector,
>  				    u64 num_sectors, u64 max_sectors)
>  {
> -	if (start_sector > VDPASIM_BLK_CAPACITY) {
> +	if (start_sector > capacity) {
>  		dev_dbg(&vdpasim->vdpa.dev,
> -			"starting sector exceeds the capacity - start: 0x%llx capacity: 0x%x\n",
> -			start_sector, VDPASIM_BLK_CAPACITY);
> +			"starting sector exceeds the capacity - start: 0x%llx capacity: 0x%lx\n",
> +			start_sector, capacity);
>  	}
>  
>  	if (num_sectors > max_sectors) {
> @@ -92,10 +95,10 @@ static bool vdpasim_blk_check_range(struct vdpasim *vdpasim, u64 start_sector,
>  		return false;
>  	}
>  
> -	if (num_sectors > VDPASIM_BLK_CAPACITY - start_sector) {
> +	if (num_sectors > capacity - start_sector) {
>  		dev_dbg(&vdpasim->vdpa.dev,
> -			"request exceeds the capacity - start: 0x%llx num: 0x%llx capacity: 0x%x\n",
> -			start_sector, num_sectors, VDPASIM_BLK_CAPACITY);
> +			"request exceeds the capacity - start: 0x%llx num: 0x%llx capacity: 0x%lx\n",
> +			start_sector, num_sectors, capacity);
>  		return false;
>  	}
>  
> @@ -369,7 +372,7 @@ static void vdpasim_blk_get_config(struct vdpasim *vdpasim, void *config)
>  
>  	memset(config, 0, sizeof(struct virtio_blk_config));
>  
> -	blk_config->capacity = cpu_to_vdpasim64(vdpasim, VDPASIM_BLK_CAPACITY);
> +	blk_config->capacity = cpu_to_vdpasim64(vdpasim, capacity);
>  	blk_config->size_max = cpu_to_vdpasim32(vdpasim, VDPASIM_BLK_SIZE_MAX);
>  	blk_config->seg_max = cpu_to_vdpasim32(vdpasim, VDPASIM_BLK_SEG_MAX);
>  	blk_config->num_queues = cpu_to_vdpasim16(vdpasim, VDPASIM_BLK_VQ_NUM);
> @@ -437,8 +440,7 @@ static int vdpasim_blk_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>  	if (blk->shared_backend) {
>  		blk->buffer = shared_buffer;
>  	} else {
> -		blk->buffer = kvzalloc(VDPASIM_BLK_CAPACITY << SECTOR_SHIFT,
> -				       GFP_KERNEL);
> +		blk->buffer = kvzalloc(capacity << SECTOR_SHIFT, GFP_KERNEL);
>  		if (!blk->buffer) {
>  			ret = -ENOMEM;
>  			goto put_dev;
> @@ -495,8 +497,7 @@ static int __init vdpasim_blk_init(void)
>  		goto parent_err;
>  
>  	if (shared_backend) {
> -		shared_buffer = kvzalloc(VDPASIM_BLK_CAPACITY << SECTOR_SHIFT,
> -					 GFP_KERNEL);
> +		shared_buffer = kvzalloc(capacity << SECTOR_SHIFT, GFP_KERNEL);
>  		if (!shared_buffer) {
>  			ret = -ENOMEM;
>  			goto mgmt_dev_err;
> -- 
> 2.45.2


  reply	other threads:[~2024-07-05 11:31 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-05 11:28 [PATCH] vdpa_sim_blk: add `capacity` module parameter Stefano Garzarella
2024-07-05 11:30 ` Michael S. Tsirkin [this message]
2024-07-05 12:41   ` Stefano Garzarella
2024-07-08  7:05     ` Cindy Lu
2024-07-08  7:59       ` Jason Wang
2024-07-08  8:15         ` Stefano Garzarella
2024-07-09  2:56           ` Jason Wang
2024-07-09 12:41             ` Stefano Garzarella
2024-07-10  3:08               ` Jason Wang
2024-07-10  7:18                 ` Stefano Garzarella
2024-07-10  7:28                   ` Jason Wang
2024-07-10  7:36                     ` Stefano Garzarella

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=20240705073017-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=eperezma@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sgarzare@redhat.com \
    --cc=virtualization@lists.linux.dev \
    --cc=xuanzhuo@linux.alibaba.com \
    /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.