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 E5339C83F14 for ; Mon, 28 Aug 2023 01:05:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D8C3328000E; Sun, 27 Aug 2023 21:05:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D3C518E0001; Sun, 27 Aug 2023 21:05:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C044828000E; Sun, 27 Aug 2023 21:05:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id B1B5B8E0001 for ; Sun, 27 Aug 2023 21:05:07 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 43F4712028F for ; Mon, 28 Aug 2023 01:05:07 +0000 (UTC) X-FDA: 81171719454.27.7796FEC Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [62.89.141.173]) by imf29.hostedemail.com (Postfix) with ESMTP id 6F13712000E for ; Mon, 28 Aug 2023 01:05:04 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=linux.org.uk header.s=zeniv-20220401 header.b=tBQRCz5+; spf=none (imf29.hostedemail.com: domain of viro@ftp.linux.org.uk has no SPF policy when checking 62.89.141.173) smtp.mailfrom=viro@ftp.linux.org.uk; dmarc=pass (policy=none) header.from=zeniv.linux.org.uk ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1693184704; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=joEHMg4Vnx51INzte3pGka01/tHMgi/viw3PIuubDEg=; b=n7KHInYWcT7ypjhqurVuUenxgINxwhZqTXkqJPd1ZnyEqf5JXC3TOrbyrF3swxkEXY+ALl H5dSBU2LFwcBRUzbVF357NTTAekojHPEaKN0w90LJeOmj/mz7DpM2XbK2pgVw4q3pwMgOs hhSLAkgti5rLEONXt+OuWgdW6cY3cEg= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=linux.org.uk header.s=zeniv-20220401 header.b=tBQRCz5+; spf=none (imf29.hostedemail.com: domain of viro@ftp.linux.org.uk has no SPF policy when checking 62.89.141.173) smtp.mailfrom=viro@ftp.linux.org.uk; dmarc=pass (policy=none) header.from=zeniv.linux.org.uk ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1693184704; a=rsa-sha256; cv=none; b=6uhyXD2BKdbc03EU6jS/R1Xcczz7TxrnrDfR0LgIBvoxY0Yz035kVchuwk15j+cxyotFcZ IwesobyNPHHMYwiea+i4aU3EWNQNz8W/8LI5TTkf3Ussvae3vs1t6/wUdUiGSvVvLv+Ptd W4vDr0ujJwGd6cHkAQUBcT4C9XY/qpo= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=joEHMg4Vnx51INzte3pGka01/tHMgi/viw3PIuubDEg=; b=tBQRCz5+oVdl3BS/sNcy7bWOX7 GGqnD2dBYFuddcCAHCZIxNjDzAdi2le5BAt0r8Kntb7pX1JY0YphckowafjvL81vNIQ3iZbSuHB8W r/PUBBhyIh5zGyiYSh551Ab8m+qrDF3vUESvq5gmF+156kAmT5ewUZLVGI0pVuCR+wxckZ4BuuSiW RJFEDzRbT6/CwsUyBB67HLFUFCa0gDIENAIc4MS7fD/7hjHRH/QwT9ZGKRpKtatGkE8elHlNUeY9H K7XCyGgu0m99e9gG1KyKu8v/0XPiv+PZlDfNvSozoVu79X3N9xA7HRVP+E5Pt/Irs6JBUgQAoXzdY 9136hnSA==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.96 #2 (Red Hat Linux)) id 1qaQgN-001R4E-2B; Mon, 28 Aug 2023 01:04:43 +0000 Date: Mon, 28 Aug 2023 02:04:43 +0100 From: Al Viro To: Christoph Hellwig Cc: Matthew Wilcox , Jens Axboe , Xiubo Li , Ilya Dryomov , Christian Brauner , Theodore Ts'o , Jaegeuk Kim , Chao Yu , Miklos Szeredi , Andreas Gruenbacher , "Darrick J. Wong" , Trond Myklebust , Anna Schumaker , Damien Le Moal , Andrew Morton , linux-block@vger.kernel.org, ceph-devel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, cluster-devel@redhat.com, linux-xfs@vger.kernel.org, linux-nfs@vger.kernel.org, linux-mm@kvack.org, Hannes Reinecke Subject: Re: [PATCH 03/12] filemap: update ki_pos in generic_perform_write Message-ID: <20230828010443.GV3390869@ZenIV> References: <20230601145904.1385409-1-hch@lst.de> <20230601145904.1385409-4-hch@lst.de> <20230827194122.GA325446@ZenIV> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230827194122.GA325446@ZenIV> X-Rspamd-Queue-Id: 6F13712000E X-Rspam-User: X-Stat-Signature: knwhzi4uwwbnpm9k4sz6agnnab4zotx9 X-Rspamd-Server: rspam01 X-HE-Tag: 1693184704-361120 X-HE-Meta: U2FsdGVkX19cDyLopTGsoSpandcXu3mbVzfoBcBY1hkbQ+464Va+mqgyjNrZQj+IWIUFbyOiHZweUdti+HS+imRxnET6ynSaF0KyTbACpfgHZtZ7lX+jcsPDZSTTvkP8sZMKaxmQdelHPrg6nVtstCsuwcL3U2W/Tr1GlIqTslwo7nNjUeIhDV1LTHPoAuydNDt+Ql5XFUtEJeGjNHBi2egogndLLL8D8d/YNHL7peDod5gnkm0KDhvZh66PWoso4YqaC5qvYikAqqSRt2SHNRQXbz4nWG9wafkMS8oxC5MTV9Wpp0F9GimmfJpljgpE5S6dU1PGbEQWZ1Ztsb7HGpa5+XqUlXRXztmrmJ4TeLWBMTghWo6IXIhQ2GuDZ2Vc5WOClvkU/aY7F92YwvuJPbYL2e8mqLFN2MJ6IeLv/XfCvCk7AaW75Z2ilfVPNVikW2qvCRnkcu6yR24XXEFW9Af8v1RsHUEja66H/6Yap2v99L65ckkYKnRzwNiLZlejdhx2RLBQg+17zVd6lqFTyySS05+rO5TcRdl4UmQTDOBTAB4gg6ZyzdCxCs0DuHPBVAFu7P8+tS4q7mMaiBmGTRRhZFXmNzbflYjy/fmAn5BhxRLMZ79vb2jlABlTG2fUl2Sn7M16tHAbUhESFxD0g5X4mMU+AVS4nUCgz5PW3/o7b+tcpvpJ+HlTUH23f/ZbBHFyKqSewsr5cAS4CKKlWfStnoemTzr2TQFTT9qqKd+QdR90ZFV2stkq8Haryp+YlMp9FzUJSpm8d91RQh8g8J6LXSRtZ5GetURJb4vIQjehkg2oshd/4jLza8r2O3LR/ojy2Wa7hmAMMkG/PrVtQmtGAkF+2ShLXCYkNN/PKbI09sN0ajFnWXuFDNtu/SO5QWKkqXdjZPhWW4ckYi4TMxjio35FNyjTunNQOvLyzlCyJNKwx/8v1cFuj592mA4FQasn4t5lcPd4gwIT9LZ q+hTax+h HFTyVDnNJgkOvi5i3dUQZCVp1uKRFKzkHcSTZX63xMVDas/r43qBWZVeJc7a/nsFjfp+4Hf1ddaXnoaxG3B4OQEPXL+u9P3QysgzfbTiOPCbdlet9VX4rSBBYHxyVW08hZ+JGwdbnKL9DZbROGeoJ7FGrZPtntfmx+7Tr/GEsQTef17QwwPwWWe8o4h6xS1D//QHXIqMzlZTu7Yg/oHiyMx8nfKJvXhIPhQO14+W9wOj1qcUR74OWpv69/od0e/MwrpQCT1i97OeSwUcQ7vGP/Yx/EK276SGoPSIe5605tS5pfjf/h8gJrBVFFrwGV/rar19Wh8O6xX7RIbs7B4+6OA+pQn/HP0LioSuCkJFqp+2CTqW/JfQQRwncQPkW5hdfd0uoySxWe0X3A23h5RwtWEIG1u5BgG+MDljiEFdlpqqY8+kfZ7tX6fWo6ejmea7f7xTCQ9jlnwX1xC2NRNGjtxpDcWWYaIEQpgDsXOmin99gCL4= 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: On Sun, Aug 27, 2023 at 08:41:22PM +0100, Al Viro wrote: > That part is somewhat fishy - there's a case where you return a positive value > and advance ->ki_pos by more than that amount. I really wonder if all callers > of ->write_iter() are OK with that. Speaking of which, in case of negative return value we'd better *not* use ->ki_pos; consider e.g. generic_file_write_iter() with O_DSYNC and vfs_fsync_range() failure. An error gets returned, but ->ki_pos is left advanced. Normal write(2) is fine - it will only update file->f_pos if ->write_iter() has returned a non-negative. However, io_uring kiocb_done() starts with if (req->flags & REQ_F_CUR_POS) req->file->f_pos = rw->kiocb.ki_pos; if (ret >= 0 && (rw->kiocb.ki_complete == io_complete_rw)) { if (!__io_complete_rw_common(req, ret)) { /* * Safe to call io_end from here as we're inline * from the submission path. */ io_req_io_end(req); io_req_set_res(req, final_ret, io_put_kbuf(req, issue_flags)); return IOU_OK; } } else { io_rw_done(&rw->kiocb, ret); } Note that ->f_pos update is *NOT* conditional upon ret >= 0 - it happens no matter what, provided that original request had ->kiocb.ki_pos equal to -1 (on a non-FMODE_STREAM file). Jens, is there any reason for doing that unconditionally? IMO it's a bad idea - there's a wide scope for fuckups that way, especially since write(2) is not sensitive to that and this use of -1 ki_pos is not particularly encouraged on io_uring side either, AFAICT. Worse, it's handling of failure exits in the first place, which already gets little testing...