[{"id":3683601,"web_url":"http://patchwork.ozlabs.org/comment/3683601/","msgid":"<d43b1d1023d13746819a780f65da9341@manguebit.org>","list_archive_url":null,"date":"2026-04-28T17:28:11","subject":"Re: [PATCH v3 03/19] cifs: invalidate cfid on unlink/rename/rmdir","submitter":{"id":91025,"url":"http://patchwork.ozlabs.org/api/people/91025/","name":"Paulo Alcantara","email":"pc@manguebit.org"},"content":"nspmangalore@gmail.com writes:\n\n> From: Shyam Prasad N <sprasad@microsoft.com>\n>\n> Today we do not invalidate the cached_dirent or the entire\n> parent cfid when a dentry in a dir has been removed/moved.\n>\n> This change invalidates the parent cfid so that we don't serve\n> directory contents from the cache.\n>\n> Cc: <stable@vger.kernel.org>\n> Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>\n> ---\n>  fs/smb/client/inode.c | 40 ++++++++++++++++++++++++++++++++++++++--\n>  1 file changed, 38 insertions(+), 2 deletions(-)\n>\n> diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c\n> index 888f9e35f14b8..e077df844c819 100644\n> --- a/fs/smb/client/inode.c\n> +++ b/fs/smb/client/inode.c\n> @@ -28,6 +28,23 @@\n>  #include \"cached_dir.h\"\n>  #include \"reparse.h\"\n>  \n> +static void cifs_invalidate_cached_dir(struct cifs_tcon *tcon,\n> +\t\t\t\t       struct dentry *parent)\n> +{\n> +\tstruct cached_fid *parent_cfid = NULL;\n> +\n> +\tif (!tcon || !parent)\n> +\t\treturn;\n\nThe NULL check for @tcon is unnecessary.  This seems to be called only\nwith a valid @tcon.\n\n> +\n> +\tif (!open_cached_dir_by_dentry(tcon, parent, &parent_cfid)) {\n> +\t\tmutex_lock(&parent_cfid->dirents.de_mutex);\n> +\t\tparent_cfid->dirents.is_valid = false;\n> +\t\tparent_cfid->dirents.is_failed = true;\n> +\t\tmutex_unlock(&parent_cfid->dirents.de_mutex);\n> +\t\tclose_cached_dir(parent_cfid);\n> +\t}\n> +}\n> +\n>  /*\n>   * Set parameters for the netfs library\n>   */\n> @@ -322,7 +339,7 @@ cifs_unix_basic_to_fattr(struct cifs_fattr *fattr, FILE_UNIX_BASIC_INFO *info,\n>  \t\t\t\tfattr->cf_uid = uid;\n>  \t\t}\n>  \t}\n> -\t\n> +\n\nWhy are you adding a blank newline?\n\n>  \tfattr->cf_gid = cifs_sb->ctx->linux_gid;\n>  \tif (!(sbflags & CIFS_MOUNT_OVERR_GID)) {\n>  \t\tu64 id = le64_to_cpu(info->Gid);\n> @@ -2067,6 +2084,9 @@ static int __cifs_unlink(struct inode *dir, struct dentry *dentry, bool sillyren\n>  \t\tcifs_set_file_info(inode, attrs, xid, full_path, origattr);\n>  \n>  out_reval:\n> +\tif (!rc && dentry->d_parent)\n> +\t\tcifs_invalidate_cached_dir(tcon, dentry->d_parent);\n\nThe non-NULL check of @dentry->d_parent is unnecessary.\ncifs_invalidate_cached_dir() already handles it.\n\n> +\n>  \tif (inode) {\n>  \t\tcifs_inode = CIFS_I(inode);\n>  \t\tcifs_inode->time = 0;\t/* will force revalidate to get info\n> @@ -2378,7 +2398,6 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry)\n>  \t}\n>  \n>  \trc = server->ops->rmdir(xid, tcon, full_path, cifs_sb);\n> -\tcifs_put_tlink(tlink);\n>  \n>  \tcifsInode = CIFS_I(d_inode(direntry));\n>  \n> @@ -2388,6 +2407,8 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry)\n>  \t\ti_size_write(d_inode(direntry), 0);\n>  \t\tclear_nlink(d_inode(direntry));\n>  \t\tspin_unlock(&d_inode(direntry)->i_lock);\n> +\t\tif (direntry->d_parent)\n> +\t\t\tcifs_invalidate_cached_dir(tcon, direntry->d_parent);\n\nDitto.\n\n>  \t}\n>  \n>  \t/* force revalidate to go get info when needed */\n> @@ -2402,6 +2423,7 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry)\n>  \n>  \tinode_set_ctime_current(d_inode(direntry));\n>  \tinode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));\n> +\tcifs_put_tlink(tlink);\n>  \n>  rmdir_exit:\n>  \tfree_dentry_path(page);\n> @@ -2501,6 +2523,8 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,\n>  \tstruct cifs_sb_info *cifs_sb;\n>  \tstruct tcon_link *tlink;\n>  \tstruct cifs_tcon *tcon;\n> +\tstruct dentry *source_parent;\n> +\tstruct dentry *target_parent;\n>  \tbool rehash = false;\n>  \tunsigned int xid;\n>  \tint rc, tmprc;\n> @@ -2532,6 +2556,8 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,\n>  \tif (IS_ERR(tlink))\n>  \t\treturn PTR_ERR(tlink);\n>  \ttcon = tlink_tcon(tlink);\n> +\tsource_parent = source_dentry->d_parent ? dget(source_dentry->d_parent) : NULL;\n> +\ttarget_parent = target_dentry->d_parent ? dget(target_dentry->d_parent) : NULL;\n\nWhy do you need to dget() ->d_parent?\n\n>  \tserver = tcon->ses->server;\n>  \n>  \tpage1 = alloc_dentry_path();\n> @@ -2668,11 +2694,21 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,\n>  \t}\n>  \n>  \t/* force revalidate to go get info when needed */\n> +\tif (!rc) {\n> +\t\tcifs_invalidate_cached_dir(tcon, source_parent);\n> +\t\tif (target_parent != source_parent)\n> +\t\t\tcifs_invalidate_cached_dir(tcon, target_parent);\n> +\t}\n> +\n>  \tCIFS_I(source_dir)->time = CIFS_I(target_dir)->time = 0;\n>  \n>  cifs_rename_exit:\n>  \tif (rehash)\n>  \t\td_rehash(target_dentry);\n> +\tif (target_parent)\n> +\t\tdput(target_parent);\n> +\tif (source_parent)\n> +\t\tdput(source_parent);\n\nThe non-NULL checks are unnecessary.  dput() already handles NULL\ndentries.","headers":{"Return-Path":"\n <linux-cifs+bounces-11262-incoming=patchwork.ozlabs.org@vger.kernel.org>","X-Original-To":["incoming@patchwork.ozlabs.org","linux-cifs@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n secure) header.d=manguebit.org header.i=@manguebit.org header.a=rsa-sha256\n header.s=dkim header.b=IB2Qdp55;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c09:e001:a7::12fc:5321; helo=sto.lore.kernel.org;\n envelope-from=linux-cifs+bounces-11262-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=manguebit.org header.i=@manguebit.org\n header.b=\"IB2Qdp55\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=143.255.12.172","smtp.subspace.kernel.org;\n dmarc=pass (p=quarantine dis=none) header.from=manguebit.org","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=manguebit.org"],"Received":["from sto.lore.kernel.org (sto.lore.kernel.org\n [IPv6:2600:3c09:e001:a7::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g4nVL0Fjnz1yHv\n\tfor <incoming@patchwork.ozlabs.org>; Wed, 29 Apr 2026 03:29:14 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sto.lore.kernel.org (Postfix) with ESMTP id 4938C301E712\n\tfor <incoming@patchwork.ozlabs.org>; Tue, 28 Apr 2026 17:28:42 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 9A0D444BCBE;\n\tTue, 28 Apr 2026 17:28:17 +0000 (UTC)","from mx1.manguebit.org (mx1.manguebit.org [143.255.12.172])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id B34D543CECE;\n\tTue, 28 Apr 2026 17:28:15 +0000 (UTC)","from pc by mx1.manguebit.org with local (Exim 4.99.1)\n\tid 1wHmEF-0000000043A-2kKy;\n\tTue, 28 Apr 2026 14:28:11 -0300"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1777397297; cv=none;\n b=AO+7wqUVqI4zLEYMvA241rDcSs4IHEyKOzczHeNYn765poJzg1o5wJtdG5mhy/0O0vFEeB/2OYrupGWVvKjl0g54dV+nRwGtLG4Tx2wjPiGApkQAwzmQprpT/4ellBFYJNniwiN2SLwnrPHM0ssHoyUcvcNHjyKW+xg6MYTzCNU=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1777397297; c=relaxed/simple;\n\tbh=aK1A0F52g1hEvh2240PVi8iidcBqxIpd6FloalqeCR0=;\n\th=Message-ID:From:To:Cc:Subject:In-Reply-To:References:Date:\n\t MIME-Version:Content-Type;\n b=G3ByD/na+4eiMH50wXaeAzS6hDq1+U/49InEmxwuTd1c0b9cT5m12gy8qoKjc7kXqxOEkKA5wUJMq9mbL+b5waJ9GLY1JEE6pMgT76jLe8Hoq3VvUWYfBNYq7bgq2H1kMPXOEAAPlTXJ6t6HKbjiJMYGjk+n2ln/9fw6jOEOsYQ=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=quarantine dis=none) header.from=manguebit.org;\n spf=pass smtp.mailfrom=manguebit.org;\n dkim=pass (2048-bit key) header.d=manguebit.org header.i=@manguebit.org\n header.b=IB2Qdp55; arc=none smtp.client-ip=143.255.12.172","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;\n\td=manguebit.org; s=dkim; h=Content-Type:MIME-Version:Date:References:\n\tIn-Reply-To:Subject:Cc:To:From:Message-ID:Sender:Reply-To:\n\tContent-Transfer-Encoding:Content-ID:Content-Description;\n\tbh=G1J5OR3MUF730xBsui9yjwFfvvjC4RiDCVHbp7kYe74=; b=IB2Qdp556p5N5+ybQOgDxtxtwz\n\tYD2f1IKgFPnWbNncPmBG6H2HalQTSP2g1SNFYNaVOcDVslsPMWNspKMM6rDLa9asiEP2N7hjOBbe7\n\th9o4nDtfKnEXtjbuC6h6SILf4NUR/C8PkIqcXSEgvpDuP1QFqMjmAw9xG3geWkRajILd15Pu6Cp6x\n\tKJxeErRkl4DLnwlE78d9O3e9XFC7IEzwcPsL/ae67W63mNGwFR7NRylW9HpPe+iKx8CeXThs5Im+1\n\tBAoAL88qiUSndZ7EZBzgnISTCjQEZTvKnI34alFbhxI+XWRR6OoLhwtvGBJtglmQvk3XItRLjPlK3\n\tKqZBfpdQ==;","Message-ID":"<d43b1d1023d13746819a780f65da9341@manguebit.org>","From":"Paulo Alcantara <pc@manguebit.org>","To":"nspmangalore@gmail.com, linux-cifs@vger.kernel.org, smfrench@gmail.com,\n bharathsm@microsoft.com, dhowells@redhat.com, henrique.carvalho@suse.com,\n ematsumiya@suse.de","Cc":"Shyam Prasad N <sprasad@microsoft.com>, stable@vger.kernel.org","Subject":"Re: [PATCH v3 03/19] cifs: invalidate cfid on unlink/rename/rmdir","In-Reply-To":"<20260428160804.281745-3-sprasad@microsoft.com>","References":"<20260428160804.281745-1-sprasad@microsoft.com>\n <20260428160804.281745-3-sprasad@microsoft.com>","Date":"Tue, 28 Apr 2026 14:28:11 -0300","Precedence":"bulk","X-Mailing-List":"linux-cifs@vger.kernel.org","List-Id":"<linux-cifs.vger.kernel.org>","List-Subscribe":"<mailto:linux-cifs+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:linux-cifs+unsubscribe@vger.kernel.org>","MIME-Version":"1.0","Content-Type":"text/plain"}}]