Received-SPF: pass (google.com: domain of linux-kernel+bounces-176267-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Stephen Brennan <stephen.s.brennan@oracle.com>
To: Jan Kara <jack@suse.cz>
Cc: Amir Goldstein <amir73il@gmail.com>, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Stephen Brennan <stephen.s.brennan@oracle.com>
Subject: [PATCH 0/1] fsnotify: clear PARENT_WATCHED flags lazily
Date: Fri, 10 May 2024 15:18:59 -0700
Message-ID: <20240510221901.520546-1-stephen.s.brennan@oracle.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?bOCItzT2yg1R0I8JIScUUvAulOtcB487uVNyXfyJa941ks5pTdvVyJGQ32Q3?=
 =?us-ascii?Q?hKg+9s8AkWQuHK82v0JQXaa342t4yRNqacnt/3X8B9j864uBvL2nXqyNRrGX?=
 =?us-ascii?Q?lu5fGuncH/7F71nVPGoLXnMCwpXvHpC/DD5HZ4dw5baZbFOMwph2+hmKQ5eF?=
 =?us-ascii?Q?XJr5HiSl2/DHjnJY9/jZLnN90fech1rIXy1bEBT+rlunMjktCT1NHKJ34Bon?=
 =?us-ascii?Q?wG88BytV4RTKmxZNI8Otjc//DuvGUH0L7hkMkFEzDq0QioP3Zhf4FjscDrA0?=
 =?us-ascii?Q?cBkl/tciPKA8TR1lk/o4rLhS9pEUX8VF/q2TMNtKBwmH/y6lL6n5c3D0CD5m?=
 =?us-ascii?Q?LJJNBW6qZA7HHFxP52HoD1rVhZyGFK5hMlUJlcngtZdPsQu44QD959V558FO?=
 =?us-ascii?Q?J8UnaXrBsFK1m2AQl1zXjVFO1S7ArytrgPWuPcdZBxxd9Q1dBgsXwVejeLLh?=
 =?us-ascii?Q?K2XdhDHdfsYDGa1HUqUT45CEQ/4WkIqgSk4IINjkHEhjRJwy3cGIc9DwhoTi?=
 =?us-ascii?Q?ziRTHLHpyMWx58XbWQAgrTWopAKn56gPJ4PKaw9bDaRwhHCNJ9nZ3/zyr739?=
 =?us-ascii?Q?hZlUA1FijO5kGZAQBlJb9SbS3diIqE+qbzlMuTkExfL0QjSffSV8PenSdIzk?=
 =?us-ascii?Q?uToIv0cDiCGI8Em+n96TEcSpXL/MuQcJW4L3+hH8tpnhxdNX5/KAzPVgoNkw?=
 =?us-ascii?Q?PfTc4GAY5z/I2TuA9xBXQ48oU+HoXcsTqrNvxxJzY09v4XdpHAGMjCpb+vO8?=
 =?us-ascii?Q?CYaduLG7OtsTqV0SubCpZxgbkuTVj1IN93q973BDmUYVp2+fV2DRQt/Gsha/?=
 =?us-ascii?Q?9jn6+R5W+rKCma0/i2EpxJSZLax1yyNK5AzSRCYikEc9wsUV6jqkKcJCwQUW?=
 =?us-ascii?Q?LlfXHHdvGThyWpolepjeFLt8TMBQAf05SJ8o4UP1KdcB9jg1cN5F90X5YN/6?=
 =?us-ascii?Q?E768b6NEK/PPdg8UF2ifRugoaWQUr18NEDKPOwhN5BGXsAbtbRKyFK+Ao2Wa?=
 =?us-ascii?Q?rESnNPbS3E9KFOocNjhHPkSZEDnC+nK9uNXrM9Ge0F8PzfxurtiYy1DhRyLS?=
 =?us-ascii?Q?pO4fh8+grAoZB+XMXq4OUqg9kyMScC2y2ZeH81IU3V2jPXJL9VlcL1mZFm7R?=
 =?us-ascii?Q?57Qg7W52FzuUD8Nf5iHXEmVbXk9oinRJ48Nxd0PCe5jzxpUOYtDkBBlDAxKx?=
 =?us-ascii?Q?WPHEYV0pG2EDgMhSCo/WEMZi12xck16NC8xkSoz/Nr0s9MYpkDq4iEIsPoc3?=
 =?us-ascii?Q?gpzZMYBOC2fFRBp/5OVtDkC1RODr9DWyMMmW/+4xpMnLKgFKFMEOo80ABTA5?=
 =?us-ascii?Q?Phpe/+y1izgZu7n1md/Z1hEIPXpuJjm1qQuBJ9QtNm+GE4lyl8hcxFodtJ7Q?=
 =?us-ascii?Q?wOE+glAGkRN/putT54G1KHGQuRxHyaFKSyg0rBWVl4Mi8f5s35qgYLYeKwso?=
 =?us-ascii?Q?34r86bVwX0Afkx32XhOnLw6P7ulKo3lGjAzBMrUXb9WMeXE1M8oaUT5h/AO/?=
 =?us-ascii?Q?f4sWL0iVoZKXFjaOHKO+QMgY2afcHxkVX3zYNfTFPabpXB/QfYV8OEz+vaXO?=
 =?us-ascii?Q?IfdsFGQbI6WTes4/haZDTFoTG9xav0pnQ8rRYG91DamlCT2Z9kF9gpxFNrrZ?=
 =?us-ascii?Q?Y2besyalRfpVbYLZXRrswAU=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	RGZIlUbCgizVJMlVezX1ykUaMDhgaXn7WGNFgpaP8pvzRYgAf4bQiETVQr9swSg5qULks5oLTDrDoIkLnc5aKLHZk8sq4iFwpQrO78eZ3szukeTYUbKGyYQS+quGS28BoZ2ur8qxKCWqkCwufjxuwJ1kgDK/zBn5gdZwQ3iKulyys2RErk29okb8Nufqor9qTuHFuMGqaYUqenM5g05dVyaeYotEm0gWPfMfaBpehSpGjIFVLjeTx4mfj8XN/8srnXp+xLiK70wIm/LCtiwM3LJ0gUf/p6DptkUuq4RAfUxtHaoXlQDZJq/71lX0m/iA0qJwf9i963YoB571EZu89bMgEu0ZQ2dzn7Ukq6nOPXbcRdRt+aR7pk2ekeIamKq8pRb8oT5WslK0zcC7YtEkN3TUVM+RlC635lmUhsdbB2/JLnO6qfF4XwG+cfPfeudrPtXX0uPdFjP1wNUcMM8YHr91lMf9wSpwmsrI4vEZkrPO3uuaJ6uWVUzyyUXtrwP157n9w/DuvSF6r9krTl88AI+h0durWfKgiLVFdUKC3liEwb0zV4r8IzHE8buNQWMuRJN4jU9wvxgB6lCui8Gnnw/j/+fBinMKjPXSa2V4RVw=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c3aab3a6-46d3-4f76-5173-08dc713f3296
X-MS-Exchange-CrossTenant-AuthSource: PH8PR10MB6597.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 May 2024 22:19:02.7198
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: eXggTEOB0jCg/KKvIjK2sagq97/MkJr/Kji9R+kWGztFlBJP4YRTcnvnHuCBcKCoZeVHhkaUinzcfGmAgVeeo4xXSqBCdjT0keyTuqLgPio=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR10MB5762
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-05-10_16,2024-05-10_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 suspectscore=0 mlxscore=0
 mlxlogscore=790 spamscore=0 bulkscore=0 phishscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2405010000
 definitions=main-2405100161
X-Proofpoint-GUID: UqdjBcZ4oXmpDUh9zBviTu246PiJ2WGb
X-Proofpoint-ORIG-GUID: UqdjBcZ4oXmpDUh9zBviTu246PiJ2WGb

Hi Amir, Jan, et al,

It's been a while since I worked with you on the patch series[1] that aimed to
make __fsnotify_update_child_dentry_flags() a sleepable function. That work got
to a point that it was close to ready, but there were some locking issues which
Jan found, and the kernel test robot reported, and I didn't find myself able to
tackle them in the amount of time I had.

But looking back on that series, I think I threw out the baby with the
bathwater. While I may not have resolved the locking issues associated with the
larger change, there was one patch which Amir shared, that probably resolves
more than 90% of the issues that people may see. I'm sending that here, since it
still applies to the latest master branch, and I think it's a very good idea.

To refresh you, the underlying issue I was trying to resolve was when
directories have many dentries (frequently, a ton of negative dentries), the
__fsnotify_update_child_dentry_flags() operation can take a while, and it
happens under spinlock.

Case #1 - if the directory has tens of millions of dentries, then you could get
a soft lockup from a single call to this function. I have seen some cases where
a single directory had this many dentries, but it's pretty rare.

Case #2 - suppose you have a system with many CPUs and a busy directory. Suppose
the directory watch is removed. The caller will begin executing
__fsnotify_update_child_dentry_flags() to clear the PARENT_WATCHED flag, but in
parallel, many other CPUs could wind up in __fsnotify_parent() and decide that
they, too, must call __fsnotify_update_child_dentry_flags() to clear the flags.
These CPUs will all spin waiting their turn, at which point they'll re-do the
long (and likely, useless) call. Even if the original call only took a second or
two, if you have a dozen or so CPUs that end up in that call, some CPUs will
spin a long time.

Amir's patch to clear PARENT_WATCHED flags lazily resolves that easily. In
__fsnotify_parent(), if callers notice that the parent is no longer watching,
they merely update the flags for the current dentry (not all the other
children). The __fsnotify_recalc_mask() function further avoids excess calls by
only updating children if the parent started watching. This easily handles case
#2 above. Perhaps case #1 could still cause issues, for the cases of truly huge
dentry counts, but we shouldn't let "perfect" get in the way of "good enough" :)


Thanks,
Stephen

[1]: https://lore.kernel.org/all/20221013222719.277923-1-stephen.s.brennan@oracle.com/

Amir Goldstein (1):
  fsnotify: clear PARENT_WATCHED flags lazily

 fs/notify/fsnotify.c             | 26 ++++++++++++++++++++------
 fs/notify/fsnotify.h             |  3 ++-
 fs/notify/mark.c                 | 32 +++++++++++++++++++++++++++++---
 include/linux/fsnotify_backend.h |  8 +++++---
 4 files changed, 56 insertions(+), 13 deletions(-)

-- 
2.43.0