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.