On Tue, Sep 20, 2022 at 4:46 PM Sa, Nuno <Nuno.Sa@xxxxxxxxxx> wrote: > > -----Original Message----- > > From: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > > Sent: Tuesday, September 20, 2022 3:39 PM > > On Tue, Sep 20, 2022 at 4:37 PM Andy Shevchenko > > <andy.shevchenko@xxxxxxxxx> wrote: > > > On Tue, Sep 20, 2022 at 4:18 PM Sa, Nuno <Nuno.Sa@xxxxxxxxxx> wrote: > > > > > On Tue, Sep 20, 2022 at 2:28 PM Nuno Sá <nuno.sa@xxxxxxxxxx> > > wrote: > > > > ... > > > > > > > > info = iio_priv(indio_dev); > > > > > > + mutex_init(&info->lock); > > > > > > info->irq = platform_get_irq(pdev, 0); > > > > > > if (info->irq < 0) > > > > > > return info->irq; > > > > > > > > > > Consider initializing it as late as possible, like after IRQ retrieval > > > > > in this context (maybe even deeper, but no context available). Ditto > > > > > for the rest of the series. > > > > > > > > Any special reason for it (maybe related to lockdep :wondering: ) ? Just > > > > curious as I never noticed such a pattern when initializing mutexes. > > > > > > Yes. Micro-optimization based on the rule "don't create a resource in > > > case of known error". > > > > > > OTOH, you have to be sure that the mutex (and generally speaking a > > > locking) should be initialized early enough. > > Typically not really needed during probe... Actually as long as you expose the ABI to the user anything can happen, means that your code should be ready to receive the requests in any possible callbacks / file ops. Same applies to the IRQ handler. So it's very important to initialize locking just in time. Hence I can say that "typically it needs to be carefully placed". > > Note that "micro" in the above can be multiplied by 'k', where 'k' is > > the amount of deferred probes (probably not the case here, but again, > > "generally speaking"). > > Well, I don't think 'mutex_init()' does that much that really matters in > these patches but ok, that rule is indeed a good practice that sometimes > makes a difference. And since I will definitely need a v2, I can make that > change. Where applicable, the best place is probably before registering the > IIO device... Some drivers are pedantic and want to have mutex_destroy() to be called, it also reduces the surface of returning with an undestroyed object (let's not discuss the usefulness of such destruction, but in principle). -- With Best Regards, Andy Shevchenko
