Linux SPI subsystem development
 help / color / mirror / Atom feed
* [PATCH] spi: Make spi_delay_exec() warn if called from atomic context
@ 2020-05-22 15:50 Mark Brown
  2020-05-22 16:02 ` Andy Shevchenko
  2020-05-25 14:57 ` Mark Brown
  0 siblings, 2 replies; 6+ messages in thread
From: Mark Brown @ 2020-05-22 15:50 UTC (permalink / raw)
  To: Serge Semin, Andy Shevchenko; +Cc: linux-spi, Mark Brown

If the delay used is long enough the spi_delay_exec() will use a sleeping
function to implement it. Add a might_sleep() here to help avoid callers
using this from an atomic context and running into problems at runtime on
other systems.

Suggested-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index e02f434affca..c69d23379e08 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1160,6 +1160,8 @@ int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer)
 {
 	int delay;
 
+	might_sleep();
+
 	if (!_delay)
 		return -EINVAL;
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: Make spi_delay_exec() warn if called from atomic context
  2020-05-22 15:50 [PATCH] spi: Make spi_delay_exec() warn if called from atomic context Mark Brown
@ 2020-05-22 16:02 ` Andy Shevchenko
  2020-05-22 16:04   ` Andy Shevchenko
  2020-05-25 14:57 ` Mark Brown
  1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2020-05-22 16:02 UTC (permalink / raw)
  To: Mark Brown; +Cc: Serge Semin, linux-spi

On Fri, May 22, 2020 at 04:50:05PM +0100, Mark Brown wrote:
> If the delay used is long enough the spi_delay_exec() will use a sleeping
> function to implement it. Add a might_sleep() here to help avoid callers
> using this from an atomic context and running into problems at runtime on
> other systems.

Thanks!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 
> Suggested-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  drivers/spi/spi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index e02f434affca..c69d23379e08 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1160,6 +1160,8 @@ int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer)
>  {
>  	int delay;
>  
> +	might_sleep();
> +
>  	if (!_delay)
>  		return -EINVAL;
>  
> -- 
> 2.20.1
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: Make spi_delay_exec() warn if called from atomic context
  2020-05-22 16:02 ` Andy Shevchenko
@ 2020-05-22 16:04   ` Andy Shevchenko
  2020-05-22 16:10     ` Serge Semin
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2020-05-22 16:04 UTC (permalink / raw)
  To: Mark Brown; +Cc: Serge Semin, linux-spi

On Fri, May 22, 2020 at 07:02:33PM +0300, Andy Shevchenko wrote:
> On Fri, May 22, 2020 at 04:50:05PM +0100, Mark Brown wrote:
> > If the delay used is long enough the spi_delay_exec() will use a sleeping
> > function to implement it. Add a might_sleep() here to help avoid callers
> > using this from an atomic context and running into problems at runtime on
> > other systems.
> 
> Thanks!
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

We may improve it later, though, to be smarter and spill a warning only when it
uses non-atomic delays. For now this is good enough.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: Make spi_delay_exec() warn if called from atomic context
  2020-05-22 16:04   ` Andy Shevchenko
@ 2020-05-22 16:10     ` Serge Semin
  2020-05-22 16:18       ` Mark Brown
  0 siblings, 1 reply; 6+ messages in thread
From: Serge Semin @ 2020-05-22 16:10 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Serge Semin, Mark Brown, linux-spi

On Fri, May 22, 2020 at 07:04:17PM +0300, Andy Shevchenko wrote:
> On Fri, May 22, 2020 at 07:02:33PM +0300, Andy Shevchenko wrote:
> > On Fri, May 22, 2020 at 04:50:05PM +0100, Mark Brown wrote:
> > > If the delay used is long enough the spi_delay_exec() will use a sleeping
> > > function to implement it. Add a might_sleep() here to help avoid callers
> > > using this from an atomic context and running into problems at runtime on
> > > other systems.
> > 
> > Thanks!
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> We may improve it later, though, to be smarter and spill a warning only when it
> uses non-atomic delays. For now this is good enough.

IMO this wouldn't be better than the current solution. might_sleep() is called
"might" to warn that the called method may get to sleep, not shall, not will. As
I see it it's better to warn about the consequences straight away, but not at the
point when the sleeping method is actually called in the atomic context.

-Sergey

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: Make spi_delay_exec() warn if called from atomic context
  2020-05-22 16:10     ` Serge Semin
@ 2020-05-22 16:18       ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2020-05-22 16:18 UTC (permalink / raw)
  To: Serge Semin; +Cc: Andy Shevchenko, Serge Semin, linux-spi

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

On Fri, May 22, 2020 at 07:10:01PM +0300, Serge Semin wrote:
> On Fri, May 22, 2020 at 07:04:17PM +0300, Andy Shevchenko wrote:

> > We may improve it later, though, to be smarter and spill a warning only when it
> > uses non-atomic delays. For now this is good enough.

> IMO this wouldn't be better than the current solution. might_sleep() is called
> "might" to warn that the called method may get to sleep, not shall, not will. As
> I see it it's better to warn about the consequences straight away, but not at the
> point when the sleeping method is actually called in the atomic context.

Yes, this is the whole point of might_sleep() - it's to ensure that we
always get a warning about code that might sleep, rather than only
getting a warning if we happen to go down a path that does actually
sleep.  This means we're less likely to run into bugs at runtime in
unusual use cases.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] spi: Make spi_delay_exec() warn if called from atomic context
  2020-05-22 15:50 [PATCH] spi: Make spi_delay_exec() warn if called from atomic context Mark Brown
  2020-05-22 16:02 ` Andy Shevchenko
@ 2020-05-25 14:57 ` Mark Brown
  1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2020-05-25 14:57 UTC (permalink / raw)
  To: Serge Semin, Andy Shevchenko, Mark Brown; +Cc: linux-spi

On Fri, 22 May 2020 16:50:05 +0100, Mark Brown wrote:
> If the delay used is long enough the spi_delay_exec() will use a sleeping
> function to implement it. Add a might_sleep() here to help avoid callers
> using this from an atomic context and running into problems at runtime on
> other systems.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: Make spi_delay_exec() warn if called from atomic context
      commit: 8fede89f853a7bfc671f9d8f069ccfe4190061f3

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-05-25 14:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-22 15:50 [PATCH] spi: Make spi_delay_exec() warn if called from atomic context Mark Brown
2020-05-22 16:02 ` Andy Shevchenko
2020-05-22 16:04   ` Andy Shevchenko
2020-05-22 16:10     ` Serge Semin
2020-05-22 16:18       ` Mark Brown
2020-05-25 14:57 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox