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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Shyam Prasad N <nspmangalore@xxxxxxxxx> writes:
> But the point that I'm trying to make here is that VFS reacts
> differently when d_validate returns an error VS when it just returns
> invalid:
> https://elixir.bootlin.com/linux/latest/source/fs/namei.c#L1409

I've tried returning the error with the repro script and it also worked.

> Notice how it calls d_invalidate only when there's no error. And
> d_invalidate seems to have detach_mounts.
> It is likely that the umount happens there.

I've dumped call stacks, the revalidation is triggered by filename_lookup()

[   31.913092]  [<ffffffff8133e3d1>] ? SendReceive+0x2b1/0x2d0
[   31.913093]  [<ffffffff8132cbd2>] ? CIFSSMBQPathInfo+0x152/0x260
[   31.913096]  [<ffffffff81343c9f>] ? cifs_query_path_info+0x6f/0x1a0
[   31.913098]  [<ffffffff81366953>] ? cifs_get_inode_info.cold+0x44f/0x6bb
[   31.913100]  [<ffffffff810bf935>] ? wake_up_process+0x15/0x20
[   31.913102]  [<ffffffff810e9c30>] ? vprintk_emit+0x200/0x500
[   31.913103]  [<ffffffff810ea099>] ? vprintk_default+0x29/0x40
[   31.913105]  [<ffffffff811a896f>] ? printk+0x50/0x52
[   31.913107]  [<ffffffff813678c1>] ? cifs_revalidate_dentry_attr.cold+0x71/0x79
[   31.913109]  [<ffffffff8133b194>] ? cifs_revalidate_dentry+0x14/0x30
[   31.913110]  [<ffffffff813327e5>] ? cifs_d_revalidate+0x25/0xb0
[   31.913112]  [<ffffffff812404ff>] ? lookup_fast+0x1bf/0x220
[   31.913113]  [<ffffffff812407bc>] ? walk_component+0x3c/0x3f0
[   31.913114]  [<ffffffff8123fe5b>] ? path_init+0x23b/0x450
[   31.913116]  [<ffffffff812412bf>] ? path_lookupat+0x7f/0x110
[   31.913118]  [<ffffffff81242ae7>] ? filename_lookup+0x97/0x190
[   31.913120]  [<ffffffff811e1e39>] ? handle_pte_fault+0x1d9/0x240
[   31.913122]  [<ffffffff81242cc9>] ? user_path_at_empty+0x59/0x90
[   31.913124]  [<ffffffff8126ab59>] ? user_statfs+0x39/0xa0
[   31.913125]  [<ffffffff8126abd6>] ? SYSC_statfs+0x16/0x40
[   31.913127]  [<ffffffff8106de53>] ? trace_do_page_fault+0x43/0x150
[   31.913130]  [<ffffffff8180359c>] ? async_page_fault+0x3c/0x60
[   31.913131]  [<ffffffff8126ad6e>] ? SyS_statfs+0xe/0x10
[   31.913132]  [<ffffffff81800021>] ? entry_SYSCALL_64_fastpath+0x20/0xee


d_invalidate()->...->drop_mountpoint() adds a callback to unmount at later times:

[   31.913246]  [<ffffffff812554da>] ? drop_mountpoint+0x6a/0x70
[   31.913247]  [<ffffffff8126b0b8>] ? pin_kill+0x88/0x160
[   31.913249]  [<ffffffff810d6a20>] ? prepare_to_wait_event+0x100/0x100
[   31.913250]  [<ffffffff810d6a20>] ? prepare_to_wait_event+0x100/0x100
[   31.913252]  [<ffffffff8126b205>] ? group_pin_kill+0x25/0x50
[   31.913253]  [<ffffffff812564fa>] ? __detach_mounts+0x13a/0x140
[   31.913255]  [<ffffffff8124bf24>] ? d_invalidate+0xa4/0x100
[   31.913256]  [<ffffffff812404cd>] ? lookup_fast+0x18d/0x220
[   31.913257]  [<ffffffff812407bc>] ? walk_component+0x3c/0x3f0
[   31.913258]  [<ffffffff8123fe5b>] ? path_init+0x23b/0x450
[   31.913259]  [<ffffffff812412bf>] ? path_lookupat+0x7f/0x110
[   31.913261]  [<ffffffff81242ae7>] ? filename_lookup+0x97/0x190
[   31.913262]  [<ffffffff811e1e39>] ? handle_pte_fault+0x1d9/0x240
[   31.913263]  [<ffffffff81242cc9>] ? user_path_at_empty+0x59/0x90
[   31.913265]  [<ffffffff8126ab59>] ? user_statfs+0x39/0xa0
[   31.913266]  [<ffffffff8126abd6>] ? SYSC_statfs+0x16/0x40
[   31.913268]  [<ffffffff8106de53>] ? trace_do_page_fault+0x43/0x150
[   31.913269]  [<ffffffff8180359c>] ? async_page_fault+0x3c/0x60
[   31.913270]  [<ffffffff8126ad6e>] ? SyS_statfs+0xe/0x10
[   31.913272]  [<ffffffff81800021>] ? entry_SYSCALL_64_fastpath+0x20/0xee

The actual unmount call:

[   31.913594]  [<ffffffff81362248>] ? cifs_put_tcon.part.0+0x71/0x123
[   31.913597]  [<ffffffff81362309>] ? cifs_put_tlink.cold+0x5/0xa
[   31.913599]  [<ffffffff813318a5>] ? cifs_umount+0x65/0xd0
[   31.913601]  [<ffffffff8132976f>] ? cifs_kill_sb+0x1f/0x30
[   31.913603]  [<ffffffff8123527a>] ? deactivate_locked_super+0x4a/0xd0
[   31.913605]  [<ffffffff81235344>] ? deactivate_super+0x44/0x50
[   31.913609]  [<ffffffff81253adb>] ? cleanup_mnt+0x3b/0x90
[   31.913610]  [<ffffffff81253b82>] ? __cleanup_mnt+0x12/0x20
[   31.913613]  [<ffffffff810af9eb>] ? task_work_run+0x7b/0xb0
[   31.913616]  [<ffffffff810a0a3a>] ? get_signal+0x4ea/0x4f0
[   31.913618]  [<ffffffff811e1e39>] ? handle_pte_fault+0x1d9/0x240
[   31.913621]  [<ffffffff810185d1>] ? do_signal+0x21/0x100
[   31.913624]  [<ffffffff81242cc9>] ? user_path_at_empty+0x59/0x90
[   31.913626]  [<ffffffff8126ab59>] ? user_statfs+0x39/0xa0
[   31.913628]  [<ffffffff8126abd6>] ? SYSC_statfs+0x16/0x40
[   31.913630]  [<ffffffff81003517>] ? exit_to_usermode_loop+0x87/0xc0
[   31.913632]  [<ffffffff81003bed>] ? syscall_return_slowpath+0xed/0x180
[   31.913634]  [<ffffffff818001b1>] ? int_ret_from_sys_call+0x8/0x6d


> I'm suggesting that we should return errors inside d_validate
> handlers, rather than just 0 or 1.
> Makes sense?

Yes that worked too. I'll send v2.

Cheers,
-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux