On Fri, Sep 30, 2022 at 12:52:47PM +0300, Serge Semin wrote: > On Thu, Sep 29, 2022 at 05:53:43PM -0600, Keith Busch wrote: > > On Fri, Sep 30, 2022 at 01:46:46AM +0300, Serge Semin wrote: > > > Inability to allocate a buffer is a critical error which shouldn't be > > > tolerated since most likely the rest of the driver won't work correctly. > > > Thus instead of returning the zero status let's return the -ENOMEM error > > > if the nvme_hwmon_data structure instance couldn't be created. > > > > > Nak for this one. The hwmon is not necessary for the rest of the driver to > > function, so having the driver detach from the device seems a bit harsh. > > Even if it is as you say, neither the method semantic nor the way it's > called imply that. Any failures except the allocation one are > perceived as erroneous. This is called by nvme_init_ctrl_finish(), and returns the error to nvme_reset_work() only if it's < 0, which indicates we can't go on and the driver unbinds. This particular condition for hwmon is not something that prevents us from making forward progress. > > The > > driver can participate in memory reclaim, so failing on a low memory condition > > can make matters worse. > > Yes it can, so can many other places in the driver utilizing kmalloc > with just GFP_KERNEL flag passed including on the same path as the > nvme_hwmon_init() execution. Kmalloc will make sure the reclaim is > either finished or executed in background anyway in all cases. This path is in the first initialization before we've set up a namespace that can be used as a reclaim destination. > Don't > really see why memory allocation failure is less worse in this case > than in many others in the same driver especially seeing as I said The other initialization kmalloc's are required to make forward progress toward setting up a namespace. This one is not required.
