From: Christoph Hellwig <hch@lst.de>
To: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>,
"Martin K . Petersen" <martin.petersen@oracle.com>,
linux-scsi@vger.kernel.org, linux-block@vger.kernel.org,
"K. Y. Srinivasan" <kys@microsoft.com>,
Haiyang Zhang <haiyangz@microsoft.com>,
Wei Liu <wei.liu@kernel.org>, "Ewan D. Milne" <emilne@redhat.com>,
Laurence Oberman <loberman@redhat.com>
Subject: Re: [PATCH] scsi: storvsc: set max_segment_size as UINT_MAX explicitly
Date: Wed, 18 Jun 2025 07:44:44 +0200 [thread overview]
Message-ID: <20250618054444.GA28826@lst.de> (raw)
In-Reply-To: <aFEYYSCREiCMGBAH@fedora>
On Tue, Jun 17, 2025 at 03:25:21PM +0800, Ming Lei wrote:
> > @@ -473,7 +473,9 @@ struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, int priv
> > else
> > shost->max_sectors = SCSI_DEFAULT_MAX_SECTORS;
> >
> > - if (sht->max_segment_size)
> > + if (sht->virt_boundary_mask)
> > + shost->virt_boundary_mask = sht->virt_boundary_mask;
> > + else if (sht->max_segment_size)
> > shost->max_segment_size = sht->max_segment_size;
> > else
> > shost->max_segment_size = BLK_MAX_SEGMENT_SIZE;
>
> This way works, but I prefer to set it explicitly in driver, instead of
> making block layer more fragile to deal with def ->max_segment_size
> if ->virt_boundary_mask is defined
The block layer already enforces this as it is a requirement. It is
just the SCSI wrapper that broke it. Without this proper fix iser
is still broken, and srp might or might not.
> - for low level driver, if ->virt_boundary_mask is defined, ->max_segment_size
> should be UINT_MAX obviously since it implies single `virt segment`.
> Setting UINT_MAX in driver has document benefit too.
No, it doesn't. It means you need to cargo cult copy and paste code
instead of solving the problem in the proper place.
> - for logical block device(md, dm, ...), both ->virt_boundary_mask and
> ->max_segment_size may be set, and it is fine since logical block device
> driver needn't to deal with sg
Stacked devices should not inherit the hardware limits at all because
we split below them, but that's a separate story. Either way this is
not relevant here as we don't have stacking drivers that use the
SCSI layer.
next prev parent reply other threads:[~2025-06-18 5:44 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-16 16:05 [PATCH] scsi: storvsc: set max_segment_size as UINT_MAX explicitly Ming Lei
2025-06-16 16:18 ` Bart Van Assche
2025-06-16 18:51 ` Wei Liu
2025-06-16 21:56 ` Martin K. Petersen
2025-06-17 4:43 ` Christoph Hellwig
2025-06-17 5:02 ` Christoph Hellwig
2025-06-17 7:25 ` Ming Lei
2025-06-18 5:44 ` Christoph Hellwig [this message]
2025-06-21 0:12 ` Laurence Oberman
2025-06-17 13:58 ` Laurence Oberman
2025-06-18 5:48 ` Christoph Hellwig
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=20250618054444.GA28826@lst.de \
--to=hch@lst.de \
--cc=emilne@redhat.com \
--cc=haiyangz@microsoft.com \
--cc=kys@microsoft.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=loberman@redhat.com \
--cc=martin.petersen@oracle.com \
--cc=ming.lei@redhat.com \
--cc=wei.liu@kernel.org \
/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.