Amir Goldstein <amir73il@xxxxxxxxx> 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
