Re: [bisected] Re: Module removal-related regression?

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Sun, Sep 10, 2017 at 11:39 AM, Jakub Kicinski <kubakici@wp.pl> wrote:
> On Sun, 10 Sep 2017 11:12:17 -0700, Dmitry Torokhov wrote:
>> On September 10, 2017 11:00:10 AM PDT, Jakub Kicinski <kubakici@wp.pl> wrote:
>> >On Sun, 10 Sep 2017 09:21:11 -0700, Dmitry Torokhov wrote:
>> >> On Sun, Sep 10, 2017 at 12:03:38AM +0200, Jakub Kicinski wrote:
>> >> > On Sat, 09 Sep 2017 13:59:25 -0700, Dmitry Torokhov wrote:
>> >> > > On September 9, 2017 1:17:26 PM PDT, Jakub Kicinski
>> ><kubakici@wp.pl> wrote:
>> >> > > >On Sat, 9 Sep 2017 12:55:51 -0700, Dmitry Torokhov wrote:
>> >> > > >> On Sat, Sep 9, 2017 at 12:27 PM, Jakub Kicinski
>> ><kubakici@wp.pl>
>> >> > > >wrote:
>> >> > > >> > On Sat, 9 Sep 2017 19:41:21 +0200, Jakub Kicinski wrote:
>> >
>> >> > > >> >> Hi!
>> >> > > >> >>
>> >> > > >> >> I'm having trouble with modules on linux/master.  rmmod
>> >succeeds
>> >> > > >but the
>> >> > > >> >> module is still loaded and the refcount goes to 1:
>> >> > > >> >>
>> >> > > >> >> #rmmod nfp; insmod ./src/nfp.ko nfp_pf_netdev=0 ; \
>> >> > > >> >>       /opt/netronome/bin/nfp-hwinfo -n 2  assembly.partno \
>> >> > > >> >>       lsmod | grep nfp; \
>> >> > > >> >>       rmmod nfp; \
>> >> > > >> >>       lsmod | grep nfp
>> >> > > >> >> nfp                   249856  0
>> >> > > >> >> nfp                   200704  1
>> >> > > >> >>
>> >> > > >> >> If I rmmod again the module will be actually unloaded.  The
>> >user
>> >> > > >space
>> >> > > >> >> is mostly Ubuntu 14.04.  Has anyone seen this?  I'm trying
>> >to
>> >> > > >bisect
>> >> > > >> >> now...
>> >> > > >> >
>> >> > > >> > Got 'em!
>> >> > > >> >
>> >> > > >> > commit 1455cf8dbfd06aa7651dcfccbadb7a093944ca65 (HEAD,
>> >> > > >refs/bisect/bad)
>> >> > > >> > Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> >> > > >> > Date:   Wed Jul 19 17:24:30 2017 -0700
>> >> > > >> >
>> >> > > >> >     driver core: emit uevents when device is bound to a
>> >driver
>> >> > > >>
>> >> > > >> Does it happen with all modules or only nfp one?
>> >> > > >>
>> >> > > >> It seems to work here:
>> >> > > >>
>> >> > > >> dtor@dtor-glaptop3:~ $ lsmod | grep psmouse
>> >> > > >> psmouse               135168  0
>> >> > > >> dtor@dtor-glaptop3:~ $ sudo rmmod psmouse
>> >> > > >> dtor@dtor-glaptop3:~ $ lsmod | grep psmouse
>> >> > > >> dtor@dtor-glaptop3:~ $ sudo modprobe psmouse
>> >> > > >
>> >> > > >It looks like the driver is actually reloaded.  The driver used
>> >to
>> >> > > >return EPROBE_DEFER, but I think it doesn't any more (rebuilding
>> >the
>> >> > > >kernel to test that right now).
>> >> > > >
>> >> > > >Could the uevent on unbind tickle Ubuntu 14.04's udev or somehow
>> >> > > >else cause the driver to be loaded again?
>> >> > >
>> >> > > It depends on how silly the udev rules are, but yes, this can
>> >definitely happen.
>> >> >
>> >> > I confirmed the driver doesn't use EPROBE_DEFER any more:
>> >> >
>> >> > $ grep -nrI EPROBE_DEFER drivers/net/ethernet/netronome/
>> >> > $
>> >>
>> >> Not sure why you bring the deferrals here, they have nothing to do
>> >with
>> >> module removal. Also, deferrals are rarely issued by the leaf driver,
>> >and
>> >> more often by providers of resources (GPIO, regulator, interrupt,
>> >etc).
>> >
>> >Yes, it's unusual, but this driver used to do it.  Which is exactly why
>> >I brought it up.  Turns out it was irrelevant :)
>> >
>> >> > I tested without any udev rules in /etc/udev/, just the standard
>> >distro
>> >> > ones.  Same thing.
>> >>
>> >> Right, so this is the default udev rule:
>> >>
>> >> /lib/udev/rules.d/80-drivers.rules:
>> >>
>> >> # do not edit this file, it will be overwritten on update
>> >>
>> >> ACTION=="remove", GOTO="drivers_end"
>> >>
>> >> ENV{MODALIAS}=="?*", RUN{builtin}="kmod load $env{MODALIAS}"
>> >> SUBSYSTEM=="tifm", ENV{TIFM_CARD_TYPE}=="SD", RUN{builtin}="kmod load
>> >tifm_sd"
>> >> SUBSYSTEM=="tifm", ENV{TIFM_CARD_TYPE}=="MS", RUN{builtin}="kmod load
>> >tifm_ms"
>> >> SUBSYSTEM=="memstick", RUN{builtin}="kmod load ms_block mspro_block"
>> >> SUBSYSTEM=="i2o", RUN{builtin}="kmod load i2o_block"
>> >> SUBSYSTEM=="module", KERNEL=="parport_pc", RUN{builtin}="kmod load
>> >ppdev"
>> >> SUBSYSTEM=="serio", ENV{MODALIAS}=="?*", RUN{builtin}="kmod load
>> >$env{MODALIAS}"
>> >> SUBSYSTEM=="graphics", RUN{builtin}="kmod load fbcon"
>> >> KERNEL=="mtd*ro", ENV{MTD_FTL}=="smartmedia", RUN{builtin}="kmod load
>> >sm_ftl"
>> >>
>> >> LABEL="drivers_end"
>> >>
>> >> So udev (and systemd) want to load kernel module on any action
>> >besides
>> >> device removal. Shortsighted decision I'd say. I'll send a patch to
>> >> systemd, in the mean time you can simply adjust your local rule to
>> >read
>> >>
>> >> ACTION!="add", GOTO="drivers_end"
>> >
>> >Mm.  That is a silly thing.  You will break a lot of setups, though.
>>
>> I think the priority it to have module loading working properly, and
>> for most users once module is loaded it stays loaded. Unloading is
>> mostly for developers.
>>
>> Luckily newer systemd versions drop events they do not recognize, so
>> exposure is even smaller.
>
> Could you point me to where that's done?

https://github.com/systemd/systemd/blob/master/src/libsystemd/sd-device/device-private.c#L506
- 508

-- 
Dmitry