* [PATCH] scsi: fix the return value of scsi_logical_block_count
@ 2024-08-07 0:57 Chaotian Jing
2024-08-13 1:52 ` Martin K. Petersen
0 siblings, 1 reply; 6+ messages in thread
From: Chaotian Jing @ 2024-08-07 0:57 UTC (permalink / raw)
To: James.Bottomley, martin.petersen
Cc: Matthias Brugger, AngeloGioacchino Del Regno, linux-scsi,
linux-kernel, linux-arm-kernel, linux-mediatek, srv_heupstream,
Chaotian Jing, stable, Bart Van Assche
scsi_logical_block_count() should return the block count of scsi device,
but the original code has a wrong implement.
Cc: stable@vger.kernel.org
Fixes: 6a20e21ae1e2 ("scsi: core: Add helper to return number of logical
blocks in a request")
Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
---
include/scsi/scsi_cmnd.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 45c40d200154..f0be0caa295a 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -236,7 +236,7 @@ static inline unsigned int scsi_logical_block_count(struct scsi_cmnd *scmd)
{
unsigned int shift = ilog2(scmd->device->sector_size) - SECTOR_SHIFT;
- return blk_rq_bytes(scsi_cmd_to_rq(scmd)) >> shift;
+ return blk_rq_sectors(scsi_cmd_to_rq(scmd)) >> shift;
}
/*
--
2.46.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: fix the return value of scsi_logical_block_count
2024-08-07 0:57 [PATCH] scsi: fix the return value of scsi_logical_block_count Chaotian Jing
@ 2024-08-13 1:52 ` Martin K. Petersen
2024-08-13 3:35 ` Chaotian Jing (井朝天)
0 siblings, 1 reply; 6+ messages in thread
From: Martin K. Petersen @ 2024-08-13 1:52 UTC (permalink / raw)
To: Chaotian Jing
Cc: James.Bottomley, martin.petersen, Matthias Brugger,
AngeloGioacchino Del Regno, linux-scsi, linux-kernel,
linux-arm-kernel, linux-mediatek, srv_heupstream, stable,
Bart Van Assche
Chaotian,
> @@ -236,7 +236,7 @@ static inline unsigned int scsi_logical_block_count(struct scsi_cmnd *scmd)
> {
> unsigned int shift = ilog2(scmd->device->sector_size) - SECTOR_SHIFT;
>
> - return blk_rq_bytes(scsi_cmd_to_rq(scmd)) >> shift;
> + return blk_rq_sectors(scsi_cmd_to_rq(scmd)) >> shift;
> }
There's no point in shifting twice by converting to sectors first.
Please just remove the SECTOR_SHIFT subtraction.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: fix the return value of scsi_logical_block_count
2024-08-13 1:52 ` Martin K. Petersen
@ 2024-08-13 3:35 ` Chaotian Jing (井朝天)
0 siblings, 0 replies; 6+ messages in thread
From: Chaotian Jing (井朝天) @ 2024-08-13 3:35 UTC (permalink / raw)
To: martin.petersen@oracle.com
Cc: linux-mediatek@lists.infradead.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, bvanassche@acm.org,
linux-scsi@vger.kernel.org, srv_heupstream@mediatek.com,
James.Bottomley@HansenPartnership.com,
linux-arm-kernel@lists.infradead.org, matthias.bgg@gmail.com,
angelogioacchino.delregno@collabora.com
On Mon, 2024-08-12 at 21:52 -0400, Martin K. Petersen wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
> Chaotian,
>
> > @@ -236,7 +236,7 @@ static inline unsigned int
> scsi_logical_block_count(struct scsi_cmnd *scmd)
> > {
> > unsigned int shift = ilog2(scmd->device->sector_size) -
> SECTOR_SHIFT;
> >
> > - return blk_rq_bytes(scsi_cmd_to_rq(scmd)) >> shift;
> > + return blk_rq_sectors(scsi_cmd_to_rq(scmd)) >> shift;
> > }
>
> There's no point in shifting twice by converting to sectors first.
> Please just remove the SECTOR_SHIFT subtraction.
>
Thanks, will fix it at next version.
> --
> Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] scsi: fix the return value of scsi_logical_block_count
@ 2024-08-06 7:26 Chaotian Jing
2024-08-06 20:51 ` Bart Van Assche
2024-08-07 8:47 ` John Garry
0 siblings, 2 replies; 6+ messages in thread
From: Chaotian Jing @ 2024-08-06 7:26 UTC (permalink / raw)
To: James.Bottomley, martin.petersen
Cc: Matthias Brugger, AngeloGioacchino Del Regno, linux-scsi,
linux-kernel, linux-arm-kernel, linux-mediatek, srv_heupstream,
Chaotian Jing
scsi_logical_block_count() should return the block count of scsi device,
but the original code has a wrong implement.
Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
---
include/scsi/scsi_cmnd.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 45c40d200154..f0be0caa295a 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -236,7 +236,7 @@ static inline unsigned int scsi_logical_block_count(struct scsi_cmnd *scmd)
{
unsigned int shift = ilog2(scmd->device->sector_size) - SECTOR_SHIFT;
- return blk_rq_bytes(scsi_cmd_to_rq(scmd)) >> shift;
+ return blk_rq_sectors(scsi_cmd_to_rq(scmd)) >> shift;
}
/*
--
2.46.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: fix the return value of scsi_logical_block_count
2024-08-06 7:26 Chaotian Jing
@ 2024-08-06 20:51 ` Bart Van Assche
2024-08-07 8:47 ` John Garry
1 sibling, 0 replies; 6+ messages in thread
From: Bart Van Assche @ 2024-08-06 20:51 UTC (permalink / raw)
To: Chaotian Jing, James.Bottomley, martin.petersen
Cc: Matthias Brugger, AngeloGioacchino Del Regno, linux-scsi,
linux-kernel, linux-arm-kernel, linux-mediatek, srv_heupstream
On 8/6/24 12:26 AM, Chaotian Jing wrote:
> scsi_logical_block_count() should return the block count of scsi device,
> but the original code has a wrong implement.
>
> Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> ---
> include/scsi/scsi_cmnd.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 45c40d200154..f0be0caa295a 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -236,7 +236,7 @@ static inline unsigned int scsi_logical_block_count(struct scsi_cmnd *scmd)
> {
> unsigned int shift = ilog2(scmd->device->sector_size) - SECTOR_SHIFT;
>
> - return blk_rq_bytes(scsi_cmd_to_rq(scmd)) >> shift;
> + return blk_rq_sectors(scsi_cmd_to_rq(scmd)) >> shift;
> }
>
> /*
Please add the following to this patch:
Cc: stable@vger.kernel.org
Fixes: 6a20e21ae1e2 ("scsi: core: Add helper to return number of logical
blocks in a request")
Anyway:
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] scsi: fix the return value of scsi_logical_block_count
2024-08-06 7:26 Chaotian Jing
2024-08-06 20:51 ` Bart Van Assche
@ 2024-08-07 8:47 ` John Garry
1 sibling, 0 replies; 6+ messages in thread
From: John Garry @ 2024-08-07 8:47 UTC (permalink / raw)
To: Chaotian Jing, James.Bottomley, martin.petersen
Cc: Matthias Brugger, AngeloGioacchino Del Regno, linux-scsi,
linux-kernel, linux-arm-kernel, linux-mediatek, srv_heupstream
On 06/08/2024 08:26, Chaotian Jing wrote:
> scsi_logical_block_count() should return the block count of scsi device,
scsi command, not scsi device
> but the original code has a wrong implement.
>
> Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> ---
> include/scsi/scsi_cmnd.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 45c40d200154..f0be0caa295a 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -236,7 +236,7 @@ static inline unsigned int scsi_logical_block_count(struct scsi_cmnd *scmd)
I find it questionable why we have this in the core code, since it's
only used in error handling logs for one driver. And obviously no one
was paying attention to it.
> {
> unsigned int shift = ilog2(scmd->device->sector_size) - SECTOR_SHIFT;
>
> - return blk_rq_bytes(scsi_cmd_to_rq(scmd)) >> shift;
> + return blk_rq_sectors(scsi_cmd_to_rq(scmd)) >> shift;
blk_rq_sectors() converts from bytes to linux sectors, and then we shift
it by diff in linux sectors and LBS.
To me, if this were used in fastpath - which it isn't - the following
seems better:
return blk_rq_bytes(scsi_cmd_to_rq(scmd)) / scmd->device->sector_size;
> }
>
> /*
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-08-13 3:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-07 0:57 [PATCH] scsi: fix the return value of scsi_logical_block_count Chaotian Jing
2024-08-13 1:52 ` Martin K. Petersen
2024-08-13 3:35 ` Chaotian Jing (井朝天)
-- strict thread matches above, loose matches on Subject: below --
2024-08-06 7:26 Chaotian Jing
2024-08-06 20:51 ` Bart Van Assche
2024-08-07 8:47 ` John Garry
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox