Re: [PATCH net] net: sysfs: Fix deadlock situation in sysfs accesses

On Tue, Dec 17, 2024 at 8:18 AM Christophe Leroy
[off-list ref] wrote:

The following problem is encountered on kernel built with
CONFIG_PREEMPT. An snmp daemon running with normal priority is
regularly calling ioctl(SIOCGMIIPHY). Another process running with
SCHED_FIFO policy is regularly reading /sys/class/net/eth0/carrier.

After some random time, the snmp daemon gets preempted while holding
the RTNL mutex then the high priority process is busy looping into
carrier_show which bails out early due to a non-successfull
rtnl_trylock() which implies restart_syscall(). Because the snmp
daemon has a lower priority, it never gets the chances to release
the RTNL mutex and the high-priority task continues to loop forever.

Replace the trylock by lock_interruptible. This will increase the
priority of the task holding the lock so that it can release it and
allow the reader of /sys/class/net/eth0/carrier to actually perform
its read.

The problem can be reproduced with the following two simple apps:

The one below runs with normal SCHED_OTHER priority:

        int main(int argc, char **argv)
        {
                int sk = socket(AF_INET, SOCK_DGRAM, 0);
                char buf[32];
                struct ifreq ifr = {.ifr_name = "eth0"};

                for (;;)
                        ioctl(sk, SIOCGMIIPHY, &ifr);

                exit(0);
        }

And the following one is started with chrt -f 80 so it runs with
SCHED_FIFO policy:

        int main(int argc, char **argv)
        {
                int fd = open("/sys/class/net/eth0/carrier", O_RDONLY);
                char buf[32];

                for (;;) {
                        read(fd, buf, sizeof(buf));
                        lseek(fd, 0, SEEK_SET);
                        usleep(5000);
                }

                exit(0);
        }

When running alone, that high priority task takes approx 6% CPU time.

When running together with the first one above, the high priority task
reaches almost 100% of CPU time.

With this fix applied, the high priority task remains at 6% CPU time
while the other one takes the remaining CPU time available.

Fixes: 336ca57c3b4e ("net-sysfs: Use rtnl_trylock in sysfs methods.")
Signed-off-by: Christophe Leroy <redacted>
---

At a first glance, this might resurface the deadlock issue Eric W. Biederman
was trying to fix in 336ca57c3b4e ("net-sysfs: Use rtnl_trylock in
sysfs methods.")

I was hoping that at some point, some sysfs write methods could be
marked as : "We do not need to hold the sysfs lock"