Thread (20 messages) 20 messages, 3 authors, 2021-02-06

Re: [PATCH v1] cifs: make nested cifs mount point dentries always valid to deal with signaled 'df'

From: Shyam Prasad N <hidden>
Date: 2021-01-31 09:29:43
Also in: linux-fsdevel

I'm assuming that the revalidation is failing in
cifs_revalidate_dentry, because it returns -ERESTARTSYS.

Going by the documentation of d_revalidate:
This function should return a positive value if the dentry is still
valid, and zero or a negative error code if it isn't.
In case of error, can we try returning the rc itself (rather than 0),
and see if VFS avoids a dentry put?
Because theoretically, the call execution has failed, and the dentry
is not found to be invalid.

Regards,
Shyam

On Fri, Jan 29, 2021 at 9:16 AM Aurélien Aptel [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Aurelien Aptel <redacted>

Assuming
- //HOST/a is mounted on /mnt
- //HOST/b is mounted on /mnt/b

On a slow connection, running 'df' and killing it while it's
processing /mnt/b can make cifs_get_inode_info() returns -ERESTARTSYS.

This triggers the following chain of events:
=> the dentry revalidation fail
=> dentry is put and released
=> superblock associated with the dentry is put
=> /mnt/b is unmounted

This quick fix makes cifs_d_revalidate() always succeed (mark as
valid) on cifs dentries which are also mount points.

Signed-off-by: Aurelien Aptel <redacted>
Signed-off-by: Paulo Alcantara (SUSE) <redacted>
---

I have managed to reproduce this bug with the following script.  It
uses tc with netem discipline (CONFIG_NET_SCH_NETEM=y) to simulate
network delays.

#!/bin/bash

#
# reproducing bsc#1177440
#
# nested mount point gets unmounted when process is signaled
#
set -e

share1=//192.168.2.110/scratch
share2=//192.168.2.110/test
opts="username=administrator,password=aaptel-42,vers=1.0,actimeo=0"

cleanup() {
    # reset delay
    tc qdisc del dev eth0 root
    mount|grep -q /mnt/nest/a && umount /mnt/nest/a
    mount|grep -q /mnt/nest && umount /mnt/nest

    echo 'module cifs -p' > /sys/kernel/debug/dynamic_debug/control
    echo 'file fs/cifs/* -p' > /sys/kernel/debug/dynamic_debug/control
    echo 0 > /proc/fs/cifs/cifsFYI
    echo 0 > /sys/module/dns_resolver/parameters/debug
    echo 1 > /proc/sys/kernel/printk_ratelimit

}

trap cleanup EXIT

nbcifs() {
    mount|grep cifs|wc -l
}

reset() {
    echo "unmounting and reloading cifs.ko"
    mount|grep -q /mnt/nest/a && umount /mnt/nest/a
    mount|grep -q /mnt/nest && umount /mnt/nest
    sleep 1
    lsmod|grep -q cifs && ( modprobe -r cifs &> /dev/null || true )
    lsmod|grep -q cifs || ( modprobe cifs &> /dev/null  || true )
}

mnt() {
    dmesg --clear
    echo 'module cifs +p' > /sys/kernel/debug/dynamic_debug/control
    echo 'file fs/cifs/* +p' > /sys/kernel/debug/dynamic_debug/control
    echo 1 > /proc/fs/cifs/cifsFYI
    echo 1 > /sys/module/dns_resolver/parameters/debug
    echo 0 > /proc/sys/kernel/printk_ratelimit

    echo "mounting"
    mkdir -p /mnt/nest
    mount.cifs $share1 /mnt/nest -o "$opts"
    mkdir -p /mnt/nest/a
    mount.cifs $share2 /mnt/nest/a -o "$opts"
}

# add fake delay
tc qdisc add dev eth0 root netem delay 300ms

while :; do
    reset
    mnt
    n=$(nbcifs)
    echo "starting df with $n mounts"
    df &
    pid=$!
    sleep 1.5
    kill $pid || true
    x=$(nbcifs)
    echo "stopping with $x mounts"
    if [ $x -lt $n ]; then
        echo "lost mounts"
        dmesg > kernel.log
        exit 1
    fi
done



fs/cifs/dir.c | 4 ++++
 1 file changed, 4 insertions(+)
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 68900f1629bff..876ef01628538 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -741,6 +741,10 @@ cifs_d_revalidate(struct dentry *direntry, unsigned int flags)
        if (flags & LOOKUP_RCU)
                return -ECHILD;

+       /* nested cifs mount point are always valid */
+       if (d_mountpoint(direntry))
+               return 1;
+
        if (d_really_is_positive(direntry)) {
                inode = d_inode(direntry);
                if ((flags & LOOKUP_REVAL) && !CIFS_CACHE_READ(CIFS_I(inode)))
--
2.29.2

-- 
Regards,
Shyam
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help