* [PATCH] scsi: ips: Replace kmap_atomic() with kmap_local_page()
@ 2023-01-03 17:31 Fabio M. De Francesco
2023-01-05 23:24 ` Ira Weiny
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Fabio M. De Francesco @ 2023-01-03 17:31 UTC (permalink / raw)
To: Adaptec OEM Raid Solutions, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, linux-kernel
Cc: Fabio M. De Francesco, Ira Weiny
kmap_atomic() is deprecated in favor of kmap_local_page(). Therefore,
replace kmap_atomic() with kmap_local_page() in ips_is_passthru(). In
the meantime remove an unnecessary comment soon before local mapping,
align code and remove spaces (the function is short, therefore the
reviewers job won't be over-complicated by these logically unrelated
clean-ups).
kmap_atomic() is implemented like a kmap_local_page() which also disables
page-faults and preemption (the latter only for !PREEMPT_RT kernels).
The code within the mapping/unmapping in ips_is_passthru() is already
in atomic context because of a call to local_irq_save() and
kmap_local_page() can be called in atomic context too (including
interrupts).
Therefore, a mere replacement of the old API with the new one is all it
is required (i.e., there is no need to explicitly add any calls to
pagefault_disable() and/or preempt_disable()).
Suggested-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/scsi/ips.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index 16419aeec02d..bb206509265e 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -1499,17 +1499,16 @@ static int ips_is_passthru(struct scsi_cmnd *SC)
struct scatterlist *sg = scsi_sglist(SC);
char *buffer;
- /* kmap_atomic() ensures addressability of the user buffer.*/
/* local_irq_save() protects the KM_IRQ0 address slot. */
local_irq_save(flags);
- buffer = kmap_atomic(sg_page(sg)) + sg->offset;
- if (buffer && buffer[0] == 'C' && buffer[1] == 'O' &&
- buffer[2] == 'P' && buffer[3] == 'P') {
- kunmap_atomic(buffer - sg->offset);
+ buffer = kmap_local_page(sg_page(sg)) + sg->offset;
+ if (buffer && buffer[0] == 'C' && buffer[1] == 'O' &&
+ buffer[2] == 'P' && buffer[3] == 'P') {
+ kunmap_local(buffer);
local_irq_restore(flags);
return 1;
}
- kunmap_atomic(buffer - sg->offset);
+ kunmap_local(buffer);
local_irq_restore(flags);
}
return 0;
--
2.39.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] scsi: ips: Replace kmap_atomic() with kmap_local_page()
2023-01-03 17:31 [PATCH] scsi: ips: Replace kmap_atomic() with kmap_local_page() Fabio M. De Francesco
@ 2023-01-05 23:24 ` Ira Weiny
2023-01-18 23:42 ` Martin K. Petersen
2023-01-27 3:22 ` Martin K. Petersen
2 siblings, 0 replies; 4+ messages in thread
From: Ira Weiny @ 2023-01-05 23:24 UTC (permalink / raw)
To: Fabio M. De Francesco
Cc: Adaptec OEM Raid Solutions, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, linux-kernel
On Tue, Jan 03, 2023 at 06:31:31PM +0100, Fabio M. De Francesco wrote:
> kmap_atomic() is deprecated in favor of kmap_local_page(). Therefore,
> replace kmap_atomic() with kmap_local_page() in ips_is_passthru(). In
> the meantime remove an unnecessary comment soon before local mapping,
> align code and remove spaces (the function is short, therefore the
> reviewers job won't be over-complicated by these logically unrelated
> clean-ups).
>
> kmap_atomic() is implemented like a kmap_local_page() which also disables
> page-faults and preemption (the latter only for !PREEMPT_RT kernels).
> The code within the mapping/unmapping in ips_is_passthru() is already
> in atomic context because of a call to local_irq_save() and
> kmap_local_page() can be called in atomic context too (including
> interrupts).
>
> Therefore, a mere replacement of the old API with the new one is all it
> is required (i.e., there is no need to explicitly add any calls to
> pagefault_disable() and/or preempt_disable()).
>
> Suggested-by: Ira Weiny <ira.weiny@intel.com>
The discussion about preemption is irrelevant AFAICS. But what you say above
is not wrong and the code looks right.
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
> drivers/scsi/ips.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
> index 16419aeec02d..bb206509265e 100644
> --- a/drivers/scsi/ips.c
> +++ b/drivers/scsi/ips.c
> @@ -1499,17 +1499,16 @@ static int ips_is_passthru(struct scsi_cmnd *SC)
> struct scatterlist *sg = scsi_sglist(SC);
> char *buffer;
>
> - /* kmap_atomic() ensures addressability of the user buffer.*/
> /* local_irq_save() protects the KM_IRQ0 address slot. */
> local_irq_save(flags);
> - buffer = kmap_atomic(sg_page(sg)) + sg->offset;
> - if (buffer && buffer[0] == 'C' && buffer[1] == 'O' &&
> - buffer[2] == 'P' && buffer[3] == 'P') {
> - kunmap_atomic(buffer - sg->offset);
> + buffer = kmap_local_page(sg_page(sg)) + sg->offset;
> + if (buffer && buffer[0] == 'C' && buffer[1] == 'O' &&
> + buffer[2] == 'P' && buffer[3] == 'P') {
> + kunmap_local(buffer);
> local_irq_restore(flags);
> return 1;
> }
> - kunmap_atomic(buffer - sg->offset);
> + kunmap_local(buffer);
> local_irq_restore(flags);
> }
> return 0;
> --
> 2.39.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] scsi: ips: Replace kmap_atomic() with kmap_local_page()
2023-01-03 17:31 [PATCH] scsi: ips: Replace kmap_atomic() with kmap_local_page() Fabio M. De Francesco
2023-01-05 23:24 ` Ira Weiny
@ 2023-01-18 23:42 ` Martin K. Petersen
2023-01-27 3:22 ` Martin K. Petersen
2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2023-01-18 23:42 UTC (permalink / raw)
To: Fabio M. De Francesco
Cc: Adaptec OEM Raid Solutions, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, linux-kernel, Ira Weiny
Fabio,
> Therefore, a mere replacement of the old API with the new one is all
> it is required (i.e., there is no need to explicitly add any calls to
> pagefault_disable() and/or preempt_disable()).
Applied to 6.3/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] scsi: ips: Replace kmap_atomic() with kmap_local_page()
2023-01-03 17:31 [PATCH] scsi: ips: Replace kmap_atomic() with kmap_local_page() Fabio M. De Francesco
2023-01-05 23:24 ` Ira Weiny
2023-01-18 23:42 ` Martin K. Petersen
@ 2023-01-27 3:22 ` Martin K. Petersen
2 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2023-01-27 3:22 UTC (permalink / raw)
To: Adaptec OEM Raid Solutions, James E.J. Bottomley, linux-scsi,
linux-kernel, Fabio M. De Francesco
Cc: Martin K . Petersen, Ira Weiny
On Tue, 03 Jan 2023 18:31:31 +0100, Fabio M. De Francesco wrote:
> kmap_atomic() is deprecated in favor of kmap_local_page(). Therefore,
> replace kmap_atomic() with kmap_local_page() in ips_is_passthru(). In
> the meantime remove an unnecessary comment soon before local mapping,
> align code and remove spaces (the function is short, therefore the
> reviewers job won't be over-complicated by these logically unrelated
> clean-ups).
>
> [...]
Applied to 6.3/scsi-queue, thanks!
[1/1] scsi: ips: Replace kmap_atomic() with kmap_local_page()
https://git.kernel.org/mkp/scsi/c/7edd053b3327
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-01-27 3:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-03 17:31 [PATCH] scsi: ips: Replace kmap_atomic() with kmap_local_page() Fabio M. De Francesco
2023-01-05 23:24 ` Ira Weiny
2023-01-18 23:42 ` Martin K. Petersen
2023-01-27 3:22 ` Martin K. Petersen
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.