Hi, > Gesendet: Mittwoch, 24. Mai 2017 um 11:06 Uhr > Von: "Stefan Wahren" <stefan.wahren@xxxxxxxx> > An: "Lino Sanfilippo" <LinoSanfilippo@xxxxxx>, "Rob Herring" <robh+dt@xxxxxxxxxx>, "Mark Rutland" <mark.rutland@xxxxxxx>, "David S. Miller" <davem@xxxxxxxxxxxxx> > Cc: linux-serial@xxxxxxxxxxxxxxx, "Jiri Slaby" <jslaby@xxxxxxxx>, "Greg Kroah-Hartman" <gregkh@xxxxxxxxxxxxxxxxxxx>, netdev@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, "Jakub Kicinski" <kubakici@xxxxx>, devicetree@xxxxxxxxxxxxxxx > Betreff: Re: [PATCH v6 net-next 17/17] net: qualcomm: add QCA7000 UART driver > > Am 23.05.2017 um 23:01 schrieb Lino Sanfilippo: > > On 23.05.2017 21:38, Stefan Wahren wrote: > >>> Lino Sanfilippo <LinoSanfilippo@xxxxxx> hat am 23. Mai 2017 um 20:16 geschrieben: > >>> > >>> I suggest to avoid this possible race by first unregistering the netdevice and then > >>> calling cancel_work_sync(). > >> What makes you sure that's safe to unregister the netdev while the tx work queue is possibly active? > > unregister_netdevice() calls netdev_close() if the interface is still up. netdev_close() calls flush_work() > > so the unregistration is delayed until the tx work function is finished. Furthermore both close() and > > tx work are synchronized by means of the qca->lock which also guarantees that unregister_netdevice() wont > > be finished until the tx work is done. > > > > Thanks for the explanation. I suspect there could be the same race > between serdev_device_close() and the tx work queue. > > So i would propose a variant of your original suggestion: > > unregister_netdev(qca->net_dev); > > /* Flush any pending characters in the driver. */ > serdev_device_close(serdev); > cancel_work_sync(&qca->tx_work); > > Since we have the same pattern in the error path of the probe function, > the same applies there. Agreed, it is much cleaner to have the same cleanup pattern in remove() as we have in (error case of) probe(). Regards, Lino -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
