On Thu, Dec 29, 2022 at 05:19:46PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx> > > Resending rebased on top of v6.2-rc1 > > -- > > This series adds a managed variant of uart_add_one_port() and uses it in the > qcom-geni-serial driver. > > I've been asked by Greg to send it separately and he didn't seem to be > impressed by the proposition of adding devres interfaces to the tty layer > in general. I can only assume it has something to do with the ongoing > discussion about the supposed danger of using devres interfaces in conjunction > with exporting character devices to user-space. That is correct. > The bug in question can be triggered by opening a device file, unbinding the > driver that exported it and then calling any of the system calls on the > associated file descriptor. > > After some testing I noticed that many subsystems are indeed either crashing > or deadlocking in the above situation. I've sent patches that attempt to fix > the GPIO and I2C subsystems[1][2]. Neither of these issues have anything to > do with devres and all to do with the fact that certain resources are freed > on driver unbind and others need to live for as long as the character device > exists. More details on that in the cover letters and commit messages in the > links. > > I'd like to point out that the serial code is immune to this issue as before > every operation, the serial core takes the port lock and checks the uart > state. If the device no longer exists (when the uart port is removed, the > pointer to uart_port inside uart_state is to NULL), it gracefully returns > -ENODEV to user-space. > > Please consider applying the patches in the series as devres is the easiest > way to lessen the burden on driver developers when dealing with complex error > paths and resource leaks. The general rule for devres is: if it can be freed > in .remove() then it can be managed by devres, which is the case for this new > helper. Overall you are adding more code to the kernel than removing, so how is this a win? Perhaps if other drivers were converted over to this new function then I would be more inclined to be able to accept it. But as-is, with only one user, it's a non-starter, sorry. thanks, greg k-h
