From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id ED5ACC433F5 for ; Tue, 11 Jan 2022 01:57:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4A3BD6B0072; Mon, 10 Jan 2022 20:57:11 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 45B636B0073; Mon, 10 Jan 2022 20:57:11 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 318D06B0074; Mon, 10 Jan 2022 20:57:11 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0082.hostedemail.com [216.40.44.82]) by kanga.kvack.org (Postfix) with ESMTP id 21FBC6B0072 for ; Mon, 10 Jan 2022 20:57:11 -0500 (EST) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id C1F608248D52 for ; Tue, 11 Jan 2022 01:57:10 +0000 (UTC) X-FDA: 79016343420.05.177287D Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by imf04.hostedemail.com (Postfix) with ESMTP id 3F75B40009 for ; Tue, 11 Jan 2022 01:57:10 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: krisman) with ESMTPSA id 7F9351F43FEA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1641866228; bh=R/DhRZqR1OleZWVgQaX/aK/IMQUF6iIiUKVlt02sFis=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=jLBG22UarhZakpVXJK8ZWN/qPTuq0w9AjGLpSdnEX3OVMGUJN82q+G9IvFTXoTTji +2Wcd8r8uIEuPXcEY/QmB6JEgzFmOwKI2QnkqIhnwXbjY0AqDRWP+/vlIvwSIxkAby JlcuoWLVp2Jgq8OUa/dsSz70yNkV+ebL3RacPsSGT39eYKNZ3O3L8ixYVKdMMNbCxJ +5NwXBGWy7yfPpLTP1rZUa9tzreN2oZ3NnXsxnZKd0z78+ZnyBa6Wf6JTdwJz69eGb EHOtDaEnu/91LQEnOtIlP+6sv1lMRTNOSB0vomqow66xHFrLCPYkaoY+Oa4BiBAhyz OYRejx4BJukmw== From: Gabriel Krisman Bertazi To: Amir Goldstein Cc: Hugh Dickins , Andrew Morton , Linux MM , Jan Kara , Matthew Bobrowski , Khazhismel Kumykov , kernel@collabora.com Subject: Re: [PATCH 0/2] shmem: Notify user space when file system is full Organization: Collabora References: <20211116220742.584975-1-krisman@collabora.com> Date: Mon, 10 Jan 2022 20:57:05 -0500 In-Reply-To: (Amir Goldstein's message of "Wed, 17 Nov 2021 11:00:16 +0200") Message-ID: <87fspv9gr2.fsf@collabora.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 3F75B40009 X-Stat-Signature: ji183kwzyf5ak59mudf9uias16r6663q Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=collabora.com header.s=mail header.b=jLBG22Ua; spf=pass (imf04.hostedemail.com: domain of krisman@collabora.com designates 46.235.227.227 as permitted sender) smtp.mailfrom=krisman@collabora.com; dmarc=pass (policy=none) header.from=collabora.com X-HE-Tag: 1641866230-176479 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Amir Goldstein writes: > Two things bother me about this proposal. > One is that it makes more sense IMO to report ENOSPC events > from vfs code. Hi Amir, I reimplemented this with FS_WB_ERROR in the branch below. It reports writeback errors on mapping_set_error, as suggested. https://gitlab.collabora.com/krisman/linux/-/tree/wb-error It is a WIP, and I'm not proposing it yet, cause I'm thinking about the ENOSPC case a bit more... > Why should the requirement to monitor ENOSPC conditions be specific to tmpfs? > Especially, as I mentioned, there are already wrappers in place to report > writeback errors on an inode (mapping_set_error), where the fsnotify hook > can fit nicely. mapping_set_error would trigger the ENOSPC event only when it happens on an actual writeback error (i.e. BLK_STS_NOSPC), which is not the main case I'm solving here. In fact, most of the time, -ENOSPC will happen before any IO is submitted, for instance, if an inode could not be allocated during .create() or a block can't be allocated in .write_begin(). In this case, it isn't really a writeback error (semantically), and it is not registered as such by any file system. We can treat those under the same FAN_WB_ERROR mask, but that is a bit weird. Maybe we should have a mask specifically for ENOSPC, or a, "IO error" mask? The patchset is quite trivial, but it has to touch many places in the VFS (say, catch ENOSPC on create, fallocate, write, writev, etc). Please, see the branch above to what that would look like. Is that a viable solution? Are you ok with reporting those cases under the same writeback mask? Btw, I'm thinking it could be tidy to transform many of those VFS calls, from if (!error) fsnotify_modify(file); else if (error == -ENOSPC) fsnotify_nospace(file); Into unconditionally calling the fsnotify_modify hook, which sends the correct event depending on the operation result: void fsnotify_modify(int error, struct file *file) { if (likely(!error)) { fsnotify_file(file, FS_MODIFY); } else if (error == -ENOSPC) { fsnotify_wb_error(file); } } (same for fsnotify_mkdir, fsnotify_open, etc). If you are ok with that? > I understand that you wanted to differentiate errors caused by memory > pressure, but I don't understand why it makes sense for a filesystem monitor > to get a different feedback than the writing application. Maybe the differentiation of those two cases could be done as part of the file system specific payload that I wanted to write for FAN_FS_ERROR? If so, it would be easier for the ENOSPC event trigger to be implemented at the filesystem-level. > The second thing that bothers me is that I think the ENOSPC condition > should not be reported on the same event mask as filesystem corruption > condition because it seems like a valid use case for filesystem monitor > to want to be notified about corruption and not about ENOSPC. -- Gabriel Krisman Bertazi