lkml.org 
[lkml]   [2018]   [Jul]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/5] i2c: smbus: add unlocked __i2c_smbus_xfer variant
On Wed, Jun 27, 2018 at 09:08:18AM +0200, Wolfram Sang wrote:
>
> > Because, thinking more about it, the problem with those allocs are not
> > related to the locking details; adding another trylock to the mix just
> > makes it so much more obvious. I mean, first we would specifically
> > handle atomic/irq context with a trylock "documenting" that atomic/irq
> > users are welcome to at least try xfers, and then we blattantly break
> > the rulez with a GFP_KERNEL alloc...
>
> Yes, thinking more about it, I came to the conclusion that we should not
> add trylock to SMBus and keep the requirement to allow sleeping.
>
> True, SMBus is not consistent with I2C then, but actually, I'd prefer
> the consistency the other way around: I wish we had a clear statement
> that i2c_transfer may sleep. And have a dedicated irqless, non-sleeping
> callback for handling the atomic case instead.
>
> I really don't like the commit which introduced the trylock
> into i2c_transfer[1]. Its commit message even says: "It is the
> reponsability of the caller to ensure that the underlying i2c bus driver
> will not sleep either." Which seems broken to me because I can't see how
> the caller should do that? And most bus drivers will sleep. But that
> commit is upstream for 10 years now, so there are probably users. Which
> also are very hard to spot, I am afraid. I wouldn't see a way to convert
> them off the top of my head.
>
> [1] cea443a81c9c ("i2c: Support i2c_transfer in atomic contexts")
>
> > Currently, I assume they are only broken when the alloc happens to
> > need to do more than is allowed from the given context. Something
> > which might or might not be common?
>
> The only regression now would be using smbus_emulated from atomic
> context. Which is a bug on the caller side because it cannot know if
> smbus_emulated will be used or not. For the non-emulated case, it must
> not be atomic anyhow.
>
> So, unless I overlooked something, if we decide to not add trylock to
> smbus_xfer, we are all fine?
>
> And I think we should really keep this clean rule of smbus functions
> being non-atomic.
>
> D'accord?

So, if no other arguments drop in, I'll apply this series as is next
week.

[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2018-07-01 14:14    [W:0.065 / U:1.400 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site