From: "Aurélien Aptel" <aaptel@suse.com>
To: Shyam Prasad N <nspmangalore@gmail.com>
Cc: CIFS <linux-cifs@vger.kernel.org>,
linux-fsdevel@vger.kernel.org, Steve French <smfrench@gmail.com>,
Paulo Alcantara <pc@cjr.nz>
Subject: Re: [PATCH v1] cifs: make nested cifs mount point dentries always valid to deal with signaled 'df'
Date: Tue, 02 Feb 2021 12:00:16 +0100 [thread overview]
Message-ID: <87v9ba91kv.fsf@suse.com> (raw)
In-Reply-To: <CANT5p=oSrrCbCdXZSbjmPDM4P=z=1c=kj9w1DDTJO5UhtREo8g@mail.gmail.com>
Shyam Prasad N <nspmangalore@gmail.com> 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)
next prev parent reply other threads:[~2021-02-02 11:01 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-29 17:13 [PATCH v1] cifs: make nested cifs mount point dentries always valid to deal with signaled 'df' Aurélien Aptel
2021-01-31 9:28 ` Shyam Prasad N
2021-02-01 10:31 ` Aurélien Aptel
2021-02-01 16:51 ` Shyam Prasad N
2021-02-02 11:00 ` Aurélien Aptel [this message]
2021-02-02 11:16 ` [PATCH v2] cifs: report error instead of invalid when revalidating a dentry fails Aurélien Aptel
2021-02-02 17:09 ` Shyam Prasad N
2021-02-02 17:34 ` Aurélien Aptel
2021-02-02 17:42 ` [PATCH v3] " Aurélien Aptel
2021-02-02 18:26 ` Shyam Prasad N
2021-02-02 18:34 ` Aurélien Aptel
2021-02-03 4:24 ` Shyam Prasad N
2021-02-05 13:32 ` Shyam Prasad N
2021-02-05 14:42 ` [PATCH v4] " Aurélien Aptel
2021-02-05 14:52 ` Shyam Prasad N
2021-02-05 19:22 ` Steve French
2021-02-05 22:31 ` Steve French
2021-02-03 4:11 ` [PATCH v3] " Steve French
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87v9ba91kv.fsf@suse.com \
--to=aaptel@suse.com \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=nspmangalore@gmail.com \
--cc=pc@cjr.nz \
--cc=smfrench@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).