Date:	Mon, 14 Mar 2016 13:30:45 +0800
From:	"Huang\, Ying" <ying.huang@...el.com>
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	<lkp@...org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [LKP] [lkp] [namei] fda89e6574: kernel BUG at fs/namei.c:679!

Al Viro <viro@...IV.linux.org.uk> writes:

> On Mon, Mar 14, 2016 at 08:48:26AM +0800, kernel test robot wrote:
>> FYI, we noticed the below changes on
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git work.lookups
>> commit fda89e65743179d09e55bc6c265d06fa5efa8803 ("namei: untanlge lookup_fast()")
>
> Unfortunately, while with my normal .config it reliably triggers an oops
> in x86_pmu_enable() (with or without those patches), yours triggers nothing
> but a pile of OOMs.  How much RAM do you give those suckers?  I'm _not_
> testing those on bare hardware, obviously - it's KVM image.

qemu-system-x86_64 -enable-kvm -cpu qemu64,+ssse3 -kernel /pkg/linux/x86_64-rhel/gcc-4.9/fda89e65743179d09e55bc6c265d06fa5efa8803/vmlinuz-4.5.0-rc4-00017-gfda89e6 -append 'root=/dev/ram0 user=lkp job=/lkp/scheduled/vm-kbuild-4G-3/bisect_trinity-300s-debian-x86_64-2015-02-07.cgz-x86_64-rhel-fda89e65743179d09e55bc6c265d06fa5efa8803-20160312-104797-ow7uw0-0.yaml ARCH=x86_64 kconfig=x86_64-rhel branch=linux-devel/devel-catchup-201603120257 commit=fda89e65743179d09e55bc6c265d06fa5efa8803 BOOT_IMAGE=/pkg/linux/x86_64-rhel/gcc-4.9/fda89e65743179d09e55bc6c265d06fa5efa8803/vmlinuz-4.5.0-rc4-00017-gfda89e6 max_uptime=1500 RESULT_ROOT=/result/trinity/300s/vm-kbuild-4G/debian-x86_64-2015-02-07.cgz/x86_64-rhel/gcc-4.9/fda89e65743179d09e55bc6c265d06fa5efa8803/0 LKP_SERVER=inn earlyprintk=ttyS0,115200 systemd.log_level=err debug apic=debug sysrq_always_enabled rcupdate.rcu_cpu_stall_timeout=100 panic=-1 softlockup_panic=1 nmi_watchdog=panic oops=panic load_ramdisk=2 prompt_ramdisk=0 console=ttyS0,115200 console=tty0 vga=normal rw ip=::::vm-kbuild-4G-3::dhcp'  -initrd /fs/sdg1/initrd-vm-kbuild-4G-3 -m 4096 -smp 4 -device e1000,netdev=net0 -netdev user,id=net0,hostfwd=tcp::23034-:22 -boot order=nc -no-reboot -watchdog i6300esb -rtc base=localtime -drive file=/fs/sdg1/disk0-vm-kbuild-4G-3,media=disk,if=virtio -drive file=/fs/sdg1/disk1-vm-kbuild-4G-3,media=disk,if=virtio -drive file=/fs/sdg1/disk2-vm-kbuild-4G-3,media=disk,if=virtio -drive file=/fs/sdg1/disk3-vm-kbuild-4G-3,media=disk,if=virtio -drive file=/fs/sdg1/disk4-vm-kbuild-4G-3,media=disk,if=virtio -drive file=/fs/sdg1/disk5-vm-kbuild-4G-3,media=disk,if=virtio -drive file=/fs/sdg1/disk6-vm-kbuild-4G-3,media=disk,if=virtio -pidfile /dev/shm/kboot/pid-vm-kbuild-4G-3 -serial file:/dev/shm/kboot/serial-vm-kbuild-4G-3 -daemonize -display none -monitor null 

This is the qemu command line we used for testing.

Best Regards,
Huang, Ying

> FWIW, see below for hopefully cleaner fix (will fold once I manage to trigger
> the damn thing and verify that fix indeed fixes).  It's on top of offending
> commit.  Folks, could you please check if it fixes that crap on your setup?
>
> diff --git a/fs/namei.c b/fs/namei.c
> index 7a5f79f..d721821 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1519,6 +1519,7 @@ static int lookup_fast(struct nameidata *nd,
>  	struct vfsmount *mnt = nd->path.mnt;
>  	struct dentry *dentry, *parent = nd->path.dentry;
>  	int err;
> +	int status = 1;
>  
>  	/*
>  	 * Rename seqlock is not required here because in the off chance
> @@ -1555,54 +1556,45 @@ static int lookup_fast(struct nameidata *nd,
>  			return -ECHILD;
>  
>  		*seqp = seq;
> -		if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE)) {
> -			int status = d_revalidate(dentry, nd->flags);
> -			if (unlikely(status <= 0)) {
> -				if (unlazy_walk(nd, dentry, seq))
> -					return -ECHILD;
> -				if (status == -ECHILD)
> -					status = d_revalidate(dentry, nd->flags);
> -				if (status <= 0) {
> -					if (!status) {
> -						d_invalidate(dentry);
> -						status = 1;
> -					}
> -					dput(dentry);
> -					return status;
> -				}
> -			}
> +		if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE))
> +			status = d_revalidate(dentry, nd->flags);
> +		if (unlikely(status <= 0)) {
> +			if (unlazy_walk(nd, dentry, seq))
> +				return -ECHILD;
> +			if (status == -ECHILD)
> +				status = d_revalidate(dentry, nd->flags);
> +		} else {
> +			/*
> +			 * Note: do negative dentry check after revalidation in
> +			 * case that drops it.
> +			 */
> +			if (unlikely(negative))
> +				return -ENOENT;
> +			path->mnt = mnt;
> +			path->dentry = dentry;
> +			if (likely(__follow_mount_rcu(nd, path, inode, seqp)))
> +				return 0;
> +			if (unlazy_walk(nd, dentry, seq))
> +				return -ECHILD;
>  		}
> -		/*
> -		 * Note: do negative dentry check after revalidation in
> -		 * case that drops it.
> -		 */
> -		if (unlikely(negative))
> -			return -ENOENT;
> -		path->mnt = mnt;
> -		path->dentry = dentry;
> -		if (likely(__follow_mount_rcu(nd, path, inode, seqp)))
> -			return 0;
> -		if (unlazy_walk(nd, dentry, seq))
> -			return -ECHILD;
>  	} else {
>  		dentry = __d_lookup(parent, &nd->last);
>  		if (unlikely(!dentry))
>  			return 1;
> -		if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE)) {
> -			int status = d_revalidate(dentry, nd->flags);
> -			if (unlikely(status <= 0)) {
> -				if (!status) {
> -					d_invalidate(dentry);
> -					status = 1;
> -				}
> -				dput(dentry);
> -				return status;
> -			}
> -		}
> -		if (unlikely(d_is_negative(dentry))) {
> -			dput(dentry);
> -			return -ENOENT;
> +		if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE))
> +			status = d_revalidate(dentry, nd->flags);
> +	}
> +	if (unlikely(status <= 0)) {
> +		if (!status) {
> +			d_invalidate(dentry);
> +			status = 1;
>  		}
> +		dput(dentry);
> +		return status;
> +	}
> +	if (unlikely(d_is_negative(dentry))) {
> +		dput(dentry);
> +		return -ENOENT;
>  	}
>  
>  	path->mnt = mnt;
> _______________________________________________
> LKP mailing list
> LKP@...ts.01.org
> https://lists.01.org/mailman/listinfo/lkp

Powered by blists - more mailing lists