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 DC940C3DA6E for ; Wed, 3 Jan 2024 23:48:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 629746B011B; Wed, 3 Jan 2024 18:48:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5D9C56B0120; Wed, 3 Jan 2024 18:48:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4A1766B011E; Wed, 3 Jan 2024 18:48:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 376D56B0346 for ; Wed, 3 Jan 2024 18:48:57 -0500 (EST) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 08B7940477 for ; Wed, 3 Jan 2024 23:48:57 +0000 (UTC) X-FDA: 81639642714.22.DC96B45 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf23.hostedemail.com (Postfix) with ESMTP id A1EF4140002 for ; Wed, 3 Jan 2024 23:48:54 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=cJwWBLta; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf23.hostedemail.com: domain of djwong@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=djwong@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1704325735; h=from:from: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=vfZXBT9Hsl/zgcaeO9ZGYWZ+QPffu/xer0xBPyuEWzA=; b=ObeFu8gRcxvyVLshZQe/5twfowy25TFrBQ74Q1kkf8HDzjg2M5XjlzFUbh58fokRvPl3OV xs7DSYdeACPXoHSFfh3Su88rcFp6Xw16kZlI26qzebMrkKnJPrDqWyaCu9JMaZkTDPqm6k HiBntTNtx59dwCFv7kCmNvpfET2r0Xo= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=cJwWBLta; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf23.hostedemail.com: domain of djwong@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=djwong@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1704325735; a=rsa-sha256; cv=none; b=afmmpEq8OJJIWZuPAOiK1sGeWbKa/ETHsW5SrdxOsbQFWzIlTMmTL3A7Lop/riBmQoBQYY ar2widz7tzKS33NGfru9IBILglnXtR5gneDoiPvbchNKHKsqFC8J+1oTW1untOEHQkX0Pi YvB0+Rtnkl4+Yrtqm8EWkoWAOHOBCCU= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 9D192CE16F3; Wed, 3 Jan 2024 23:48:51 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 72710C433C7; Wed, 3 Jan 2024 23:48:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1704325730; bh=kzeF/2gV5HVrILoCgSh0mi0TADbFSXOraWovke+RBZA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=cJwWBLtaTd1jteJo43e4mb5XQufIqU5Z6B/x+LZccmzFhfUlCuFBQ7z0OdQ5K6l9O h6f8SrS0Tbj6nt878Bx+vXpW5seicMicCIW1Mu9dI5rHdq4/mO8fnURb64oS5mh7LI 4Y/rSPc/74ZysJBwNZjXvHvudjxTvK8Rnmmw2DJDF0yOfZK3rHomTRXZwBs91Obamh m3Njf4SiJndYQKWWMIBQMICU+X0RlWGq2xdOq/80gjsRUa8e43AS4swXkX3D9BU3tT KVpm9DKjFQNaOZRRat+GTQekYZ7h+kdNWYQi2Cf+fy9FedioEFjtdB1SkB0jBgO1gK R4uqV60P5i9Ag== Date: Wed, 3 Jan 2024 15:48:49 -0800 From: "Darrick J. Wong" To: Christoph Hellwig Cc: Chandan Babu R , Hugh Dickins , Andrew Morton , linux-xfs@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH 05/15] xfs: remove the xfile_pread/pwrite APIs Message-ID: <20240103234849.GY361584@frogsfrogsfrogs> References: <20240103084126.513354-1-hch@lst.de> <20240103084126.513354-6-hch@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240103084126.513354-6-hch@lst.de> X-Rspamd-Queue-Id: A1EF4140002 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: iq1h6s8n6z3a8nuzhwr8nct3bykbgc4n X-HE-Tag: 1704325734-159050 X-HE-Meta: U2FsdGVkX1+cszrxksJC9u200HeGppZhL7oIjecwjOngCGF50qb/lUMoO5SOMQssVU+iCxZ8LDNSrnJR/WDVGZQefDPmByEe2JiMxDLfrsLskvn0NEZOTqLZkOnN/OSL2PjsY8D3tndKyPrVzPqdmPWDTxkjbadEcVXtdxgcQHx+Ei68fthcvQzxg5R+xfRXSwtQyVOC5AlfKLWzoVCJKdU12Hm0oj7ZIFPIdho2yW+ikVBjEtOBl3uxCnIDo321PvKic75O0z50ZMgG+WjCKd1UxXe7jMoTEDTmASioL7nRcjHvvqE7QAdTfqjRhqVMkwtYuFEby0DlSgN8XTwTTieGNSVCafVul8JRjUcux/QAFQnuuEyQ6ojXP8UDpVk5orLsnemM9qx8QJx10DWuU1l914P5UrAeqUIRcqsKwtszfyGHGCEbIgGjTzaaqobP3C0RxQCKblfiwW2ufn6g2QMY2tAPiYi3XQNqrDdz06i8vGWK4YCxeke6KphUhztoLzr6uI1ANCct0GuxYShl4zsQxBX5tmDytlJKcIZ5bQopdAtN4MOdkngHuZFN1vepEhErT2iGOA5P/meldXFwETMmRyXM95h4JEfze59brOYLsHFeiNAVc22VyM6K7UJOfEL3hefjRElxMAAMB6hP3iiS7IRxh4FwpgQtDl3cXgdy5W/uXte684lxeL5YFrxQcW0rxrs310IfEkYG46sip94TXrveIYNUc0qgWTw8KWcjvpAhxngq2qnRq+oeJSqNMdn6OgIMigjNKHyKgOW+WkY6jtyQDZNzMoEI/UYjAHeNPtCMZ8ihSQ9qUdpUsA/6o46BajcLdRs04eq2koyzneI4UtUiR6ZWooftg1X2JYySZM64h+dTyBuCkeQMeAACrNtqv2lQ2IOSj+FQIw35pMBkiS7k0ZFzgKZFQqR2X9Zu83xjd7nU1lk3zySiZKxbVzwFIu6cE+OlzVkqdHC Bise0nqI Cvyop8TjWLJVpdsphduFQmZc1Yf6KvuuyCFSiXFoQxDp1Tboi07mRuybLYrNbXJVIgTikOTG6votd9MKFQhga0O3prv9ljAnrKLdoLNXmBDl+49KT94BN9zVTMb3TkaDYFM6jL6kSKpyxm8B6x+TFjIpEFTwa0RMJlb1p2OPGB2HkmqaBEWp0C500gQSwSBLkkmdadVB/h1kcdiq1ctEmyZgSDOqQDsk4QhYWIaOK1mOGd9fZdq7waL8Xi8OabG5csNLtws9Zc6W5NT1cbl3LNm15UePoifcL1je5DryAwHaSy3/7oispmHDe0YmAf/nC5m9F/3vIlq+CUTM= 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: List-Subscribe: List-Unsubscribe: On Wed, Jan 03, 2024 at 08:41:16AM +0000, Christoph Hellwig wrote: > All current and pending xfile users use the xfile_obj_load > and xfile_obj_store API, so make those the actual implementation. > > Signed-off-by: Christoph Hellwig > --- > .../xfs/xfs-online-fsck-design.rst | 10 +--- > fs/xfs/scrub/trace.h | 4 +- > fs/xfs/scrub/xfile.c | 54 +++++++++---------- > fs/xfs/scrub/xfile.h | 32 +---------- > 4 files changed, 30 insertions(+), 70 deletions(-) > > diff --git a/Documentation/filesystems/xfs/xfs-online-fsck-design.rst b/Documentation/filesystems/xfs/xfs-online-fsck-design.rst > index 352516feef6ffe..324d5ec921e8e5 100644 > --- a/Documentation/filesystems/xfs/xfs-online-fsck-design.rst > +++ b/Documentation/filesystems/xfs/xfs-online-fsck-design.rst > @@ -1915,19 +1915,13 @@ four of those five higher level data structures. > The fifth use case is discussed in the :ref:`realtime summary ` case > study. > > -The most general storage interface supported by the xfile enables the reading > -and writing of arbitrary quantities of data at arbitrary offsets in the xfile. > -This capability is provided by ``xfile_pread`` and ``xfile_pwrite`` functions, > -which behave similarly to their userspace counterparts. > XFS is very record-based, which suggests that the ability to load and store > complete records is important. > To support these cases, a pair of ``xfile_obj_load`` and ``xfile_obj_store`` > -functions are provided to read and persist objects into an xfile. > -They are internally the same as pread and pwrite, except that they treat any > -error as an out of memory error. > +functions are provided to read and persist objects into an xfile that unlike > +the pread and pwrite system calls treat any error as an out of memory error. I don't think we need to mention pread and pwrite anymore. "To support these cases, a pair of ``xfile_obj_load`` and ``xfile_obj_store`` functions are provided to read and persist objects into an xfile. An errors encountered here are treated as an out of memory error." > For online repair, squashing error conditions in this manner is an acceptable > behavior because the only reaction is to abort the operation back to userspace. > -All five xfile usecases can be serviced by these four functions. > > However, no discussion of file access idioms is complete without answering the > question, "But what about mmap?" > diff --git a/fs/xfs/scrub/trace.h b/fs/xfs/scrub/trace.h > index ed9e044f6d603c..e6156d000fc615 100644 > --- a/fs/xfs/scrub/trace.h > +++ b/fs/xfs/scrub/trace.h > @@ -903,8 +903,8 @@ DECLARE_EVENT_CLASS(xfile_class, > DEFINE_EVENT(xfile_class, name, \ > TP_PROTO(struct xfile *xf, loff_t pos, unsigned long long bytecount), \ > TP_ARGS(xf, pos, bytecount)) > -DEFINE_XFILE_EVENT(xfile_pread); > -DEFINE_XFILE_EVENT(xfile_pwrite); > +DEFINE_XFILE_EVENT(xfile_obj_load); > +DEFINE_XFILE_EVENT(xfile_obj_store); Want to shorten the names to xfile_load and xfile_store? That's really what they're doing anyway. With those changes, Reviewed-by: Darrick J. Wong --D > DEFINE_XFILE_EVENT(xfile_seek_data); > DEFINE_XFILE_EVENT(xfile_get_page); > DEFINE_XFILE_EVENT(xfile_put_page); > diff --git a/fs/xfs/scrub/xfile.c b/fs/xfs/scrub/xfile.c > index 87654cdd5ac6f9..9e25ecf3dc2fec 100644 > --- a/fs/xfs/scrub/xfile.c > +++ b/fs/xfs/scrub/xfile.c > @@ -118,13 +118,11 @@ xfile_destroy( > } > > /* > - * Read a memory object directly from the xfile's page cache. Unlike regular > - * pread, we return -E2BIG and -EFBIG for reads that are too large or at too > - * high an offset, instead of truncating the read. Otherwise, we return > - * bytes read or an error code, like regular pread. > + * Load an object. Since we're treating this file as "memory", any error or > + * short IO is treated as a failure to allocate memory. > */ > -ssize_t > -xfile_pread( > +int > +xfile_obj_load( > struct xfile *xf, > void *buf, > size_t count, > @@ -133,16 +131,15 @@ xfile_pread( > struct inode *inode = file_inode(xf->file); > struct address_space *mapping = inode->i_mapping; > struct page *page = NULL; > - ssize_t read = 0; > unsigned int pflags; > int error = 0; > > if (count > MAX_RW_COUNT) > - return -E2BIG; > + return -ENOMEM; > if (inode->i_sb->s_maxbytes - pos < count) > - return -EFBIG; > + return -ENOMEM; > > - trace_xfile_pread(xf, pos, count); > + trace_xfile_obj_load(xf, pos, count); > > pflags = memalloc_nofs_save(); > while (count > 0) { > @@ -160,8 +157,10 @@ xfile_pread( > __GFP_NOWARN); > if (IS_ERR(page)) { > error = PTR_ERR(page); > - if (error != -ENOMEM) > + if (error != -ENOMEM) { > + error = -ENOMEM; > break; > + } > > memset(buf, 0, len); > goto advance; > @@ -185,23 +184,18 @@ xfile_pread( > count -= len; > pos += len; > buf += len; > - read += len; > } > memalloc_nofs_restore(pflags); > > - if (read > 0) > - return read; > return error; > } > > /* > - * Write a memory object directly to the xfile's page cache. Unlike regular > - * pwrite, we return -E2BIG and -EFBIG for writes that are too large or at too > - * high an offset, instead of truncating the write. Otherwise, we return > - * bytes written or an error code, like regular pwrite. > + * Store an object. Since we're treating this file as "memory", any error or > + * short IO is treated as a failure to allocate memory. > */ > -ssize_t > -xfile_pwrite( > +int > +xfile_obj_store( > struct xfile *xf, > const void *buf, > size_t count, > @@ -211,16 +205,15 @@ xfile_pwrite( > struct address_space *mapping = inode->i_mapping; > const struct address_space_operations *aops = mapping->a_ops; > struct page *page = NULL; > - ssize_t written = 0; > unsigned int pflags; > int error = 0; > > if (count > MAX_RW_COUNT) > - return -E2BIG; > + return -ENOMEM; > if (inode->i_sb->s_maxbytes - pos < count) > - return -EFBIG; > + return -ENOMEM; > > - trace_xfile_pwrite(xf, pos, count); > + trace_xfile_obj_store(xf, pos, count); > > pflags = memalloc_nofs_save(); > while (count > 0) { > @@ -239,8 +232,10 @@ xfile_pwrite( > */ > error = aops->write_begin(NULL, mapping, pos, len, &page, > &fsdata); > - if (error) > + if (error) { > + error = -ENOMEM; > break; > + } > > /* > * xfile pages must never be mapped into userspace, so we skip > @@ -259,13 +254,14 @@ xfile_pwrite( > ret = aops->write_end(NULL, mapping, pos, len, len, page, > fsdata); > if (ret < 0) { > - error = ret; > + error = -ENOMEM; > break; > } > > - written += ret; > - if (ret != len) > + if (ret != len) { > + error = -ENOMEM; > break; > + } > > count -= ret; > pos += ret; > @@ -273,8 +269,6 @@ xfile_pwrite( > } > memalloc_nofs_restore(pflags); > > - if (written > 0) > - return written; > return error; > } > > diff --git a/fs/xfs/scrub/xfile.h b/fs/xfs/scrub/xfile.h > index c602d11560d8ee..f0e11febf216a7 100644 > --- a/fs/xfs/scrub/xfile.h > +++ b/fs/xfs/scrub/xfile.h > @@ -29,38 +29,10 @@ struct xfile { > int xfile_create(const char *description, loff_t isize, struct xfile **xfilep); > void xfile_destroy(struct xfile *xf); > > -ssize_t xfile_pread(struct xfile *xf, void *buf, size_t count, loff_t pos); > -ssize_t xfile_pwrite(struct xfile *xf, const void *buf, size_t count, > +int xfile_obj_load(struct xfile *xf, void *buf, size_t count, loff_t pos); > +int xfile_obj_store(struct xfile *xf, const void *buf, size_t count, > loff_t pos); > > -/* > - * Load an object. Since we're treating this file as "memory", any error or > - * short IO is treated as a failure to allocate memory. > - */ > -static inline int > -xfile_obj_load(struct xfile *xf, void *buf, size_t count, loff_t pos) > -{ > - ssize_t ret = xfile_pread(xf, buf, count, pos); > - > - if (ret < 0 || ret != count) > - return -ENOMEM; > - return 0; > -} > - > -/* > - * Store an object. Since we're treating this file as "memory", any error or > - * short IO is treated as a failure to allocate memory. > - */ > -static inline int > -xfile_obj_store(struct xfile *xf, const void *buf, size_t count, loff_t pos) > -{ > - ssize_t ret = xfile_pwrite(xf, buf, count, pos); > - > - if (ret < 0 || ret != count) > - return -ENOMEM; > - return 0; > -} > - > loff_t xfile_seek_data(struct xfile *xf, loff_t pos); > > int xfile_get_page(struct xfile *xf, loff_t offset, unsigned int len, > -- > 2.39.2 > >