RE: [PATCH] Drivers: hv: kvp/vss: Avoid accessing a ringbuffer not initialized yet
From: Michael Kelley
Date: Thu Oct 31 2024 - 21:39:20 EST
From: Dexuan Cui <decui@xxxxxxxxxxxxx> Sent: Thursday, October 31, 2024 5:17 PM
>
> > From: Michael Kelley <mhklinux@xxxxxxxxxxx>
> > Sent: Wednesday, October 30, 2024 5:12 PM
> > [...]
> > What do you think about this (compile tested only), which splits the
> > "init" function into two parts for devices that have char devs? I'm
> > trying to avoid adding yet another synchronization point by just
> > doing the init operations in the right order -- i.e., don't create the
> > user space /dev entry until the VMBus channel is ready.
> >
> > Michael
>
> Thanks, I think this works! This is a better fix.
>
> > + if (srv->util_init_transport) {
> > + ret = srv->util_init_transport();
> > + if (ret) {
> > + ret = -ENODEV;
> IMO we don't need the line above, since the 'ret' from
> srv->util_init_transport() is already a standard error code.
I was just now looking at call_driver_probe(), and it behaves
slightly differently for ENODEV and ENXIO vs. other error
codes. ENODEV and ENXIO don't output a message to the
console unless debugging is enabled, while other error codes
always output a message to the console. Forcing the error to
ENODEV has been there since the util_probe() code came out
of staging in year 2011. But I don't really have a preference
either way.
>
> BTW, I noticed that the line "ret = -ENODEV;"
> if (srv->util_init) {
> ret = srv->util_init(srv);
> if (ret) {
> ret = -ENODEV;
> goto error1;
> }
> }
> I think we don't really need that line, either.
> The existing 4 .util_init callbacks also already return a
> standard error code. We can make a separate patch to clean
> that up.
Same here.
Michael