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 E5762C04E69 for ; Thu, 27 Jul 2023 19:23:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 71D286B0074; Thu, 27 Jul 2023 15:23:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6A60C6B0075; Thu, 27 Jul 2023 15:23:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 546A66B0078; Thu, 27 Jul 2023 15:23:53 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 3FBDC6B0074 for ; Thu, 27 Jul 2023 15:23:53 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 17F284063D for ; Thu, 27 Jul 2023 19:23:53 +0000 (UTC) X-FDA: 81058366746.24.C2F4AA7 Received: from mail-yb1-f172.google.com (mail-yb1-f172.google.com [209.85.219.172]) by imf10.hostedemail.com (Postfix) with ESMTP id 2AFB2C0006 for ; Thu, 27 Jul 2023 19:23:49 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=Wj44Om9r; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf10.hostedemail.com: domain of hughd@google.com designates 209.85.219.172 as permitted sender) smtp.mailfrom=hughd@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1690485830; 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=3p55Sq7jOPOyztMgwNy28i6gxdMWDYbV2zRCVkaZOto=; b=HruNno2s0NLRloLE6QKsd9cRNobgDpLLlBd3f0vPmyrAG2v6hd2dprMeTkb3dQfX/BfDze JtbGuPHol6Muj26/2uZdNWrr60I8s/GkqoMT86Q9bbzTHukwfZ1NRMNGcm0mUhVaNUlfDy AAGdMfW2pHRWn8orDhxsYcBMLfZDj2U= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=Wj44Om9r; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf10.hostedemail.com: domain of hughd@google.com designates 209.85.219.172 as permitted sender) smtp.mailfrom=hughd@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1690485830; a=rsa-sha256; cv=none; b=VF2oeGS9vPN7roxJfhat3AtYM4hXH21doR/1i+VKbhOyv0pqrZzcTqdxs6jsNKMRunFyh5 8E7ewHYJAgI4xWBUOawegjHnOkSQG1+bUqCLqJSqFaE2QcwebY6cOS9E9buAcYsMhgQDxG FuaFW4AFk7BcEyCl/TA/vOqEbQB5e6Y= Received: by mail-yb1-f172.google.com with SMTP id 3f1490d57ef6-cc4f4351ac7so1245175276.2 for ; Thu, 27 Jul 2023 12:23:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1690485829; x=1691090629; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to; bh=3p55Sq7jOPOyztMgwNy28i6gxdMWDYbV2zRCVkaZOto=; b=Wj44Om9rKG337qfaoqNek+dwsW6frepjR1UGJOMg/1FCMzqBFifNdPcPRBDFpRsC+r SWWlcSfWx4A39OvqV8Fzs0i6HEBcBLtqv+jj7oms/GxCe5D3QAICKVlc4Q4xiXyXd7Bh HDmP7jiSSAfsp1QdKpksw4CJOCcT6DZ/DXPn6hD4XdXjL1fIlE9XixdqqLB+fsqmB6a8 QQQEugaI34wzJTOX5tOC4ZFDdun96gc60Khp3ZZumpXuwKLHH1GLC8oVlOGnmsnI75T9 4YJLXKFkte2bKZPAmdiWpRY9eKwH4uLEhjMrqI7UTGuYdagkrJz3Yod2HVz88u5g22vd Rd9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690485829; x=1691090629; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=3p55Sq7jOPOyztMgwNy28i6gxdMWDYbV2zRCVkaZOto=; b=QZ+Vxcu31Ehx//z2jHlpxUcpQ8bDABOWIDR+aeIb8Ksx14u4+oLXdM92ZyaRr7y+Dc zZpmW92OPRo08Zp/JfjBkxojhoaVnJTZQG+xMxQoWclihTm4Xi/cimiLDcmq3osK2eiA xK5Ot6kMsKzMCLn2YrOm89x8Dlz1N3aTZZZZ48utmdu71IcJW6zKKCzTHJ9xsiiUbVRY IPvnihH67i3FoOQ9Y863ftx/NhT1kvHaNQwRMjgsH4LvLc+AjFq19s2cW4PbyZX3pO78 Mt725ev3hEU+vg3/ZID87jL8POMVY0p/PhjWTy6jgkJ801pPzqCbv5BsOG3jdedoHZNR HYVg== X-Gm-Message-State: ABy/qLYLgBb/WiFV5hE+PZ2EHSzy0V0PQedtW/wI8qzicEvI3qqPj43Y lCKt8P1PFTvmbu1WBQWILo4JYQ== X-Google-Smtp-Source: APBJJlHy7zEq/TtzCz5Zjkh2FW+fxbAj2FT57ApWwlXXwTCIV6gmpUv/jH1vywotXR7udrz7isqt/w== X-Received: by 2002:a25:260d:0:b0:d25:129e:8766 with SMTP id m13-20020a25260d000000b00d25129e8766mr311316ybm.29.1690485829111; Thu, 27 Jul 2023 12:23:49 -0700 (PDT) Received: from ripple.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id f11-20020a5b0d4b000000b00d13b72fae3esm433941ybr.2.2023.07.27.12.23.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Jul 2023 12:23:48 -0700 (PDT) Date: Thu, 27 Jul 2023 12:23:46 -0700 (PDT) From: Hugh Dickins X-X-Sender: hugh@ripple.attlocal.net To: David Howells cc: Hugh Dickins , Andrew Morton , Christoph Hellwig , Jens Axboe , Al Viro , John Hubbard , David Hildenbrand , Matthew Wilcox , Chuck Lever , linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH 2/2] shmem: Apply a couple of filemap_splice_read() fixes to shmem_splice_read() In-Reply-To: <20230727161016.169066-3-dhowells@redhat.com> Message-ID: References: <20230727161016.169066-1-dhowells@redhat.com> <20230727161016.169066-3-dhowells@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Rspamd-Queue-Id: 2AFB2C0006 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: 3uzzeeqx9f3zr7zb3n49r1fy1okxodji X-HE-Tag: 1690485829-336202 X-HE-Meta: U2FsdGVkX186+UuqEK/yBHHDp8HHJnIANwOW+F7s3K4ff/lcSPBn8c4OHWQlKRkuAa5SnPtGO7/GLz5aMArQ8JCzam1OtVl4C03tBcY2hqx+SnP5sJn7Cn5SrnANCcD1wt1899Bmqyavx9YQ2ANh9sGFeRbmp4imBY16+NMtLjS5anZOm3BjRvT9eKq//n0fvedvt1s4DKi+rQIBqM6KkVjR12tTvegtWKFIPot+QfDTti+x0usNPZL9LbqdqRHirNSTwCL9rt6bH4wMksVdfzANyR1t+qHNH7rwIGIs194QiR8G6WWl2CH5aJJCCEtLku/+NkKW26eXwDOwu2PAPbNpuRqNqfyEgfEDSg7P6WS8qVYO4MIhMCvhc8up/kN4HcRJa28CY/wM7k2abZhwOIMHciYn/kJXk4xrGBtwtt4LU1zkP6qqML94N3fRobd1OWrgX18650EhDDSjRv9sIVmGOw38Wzx4a9URtI17ltOzQxMm1RFgXAfFfNMxQbP5usStn1dTpOgnjugsuQp9mNXWTlg1t1cr+vwstnXM321lL5hGkYm15NGmtj83JVR9ORlsqSlfE5h6B9v9+GNeSEfKPlWmeH4FoGzBRYAftfh9QM6OlzxKRTpR5qweD+eWfq2sO/w52Mrk0BsFfu1WqsJvk5krwra8/XgLFzBT6EBmeE4EvstgeuRTAVLELapQAxKVPv8bviL0NJJZnZdQ9U7VGd7KxSb+1lzxBpoRKytymP5mdjFDal2Vqi1vNt9/JfO2jID4h+hWCU5WjusxbsYlz23AZe0YvTBpKdz8WVMc1iYFqIhISg7FZOrQic7QpoyxdINx90VBGoPr1+dzVUGqfzmKRZ+dC45JAZYGYJjudpZqaArYRKFu7w5kZgdkuqyYjcYShTOtY2MvIrNQiM5ftpOJw/upB2XBqls8TwGqzAsqsOhn4iv/DIn68++xIw5ZWZ30mE6Z22c5yYz 8raLcpgA WWqw45Xsog7dcOq282mOfe9PcmGHkzWEDZ72fi5RiXqRKlDw1D/DesIRquOo3TpAz3QTq51zV1ynFMe0Q5+sjW0+708T5Qlvqoii2FJ6XUuVbyKrAuoyKQuOySrQaJJSw0+dRGuuajjlK/IiOgvG2ELh0yGNFdllyHqXOUbxpIBmGsA7qtLXZJkoVQVYM5oUtuO1mFmvno7gkrtVWO3/txz4pHYRXK3a5HxFS0DKHncu0kwrgB0mWSAblm/IC/GfPG23JWzcuayRWUxklarLqUsUs5JRzG7pclHJnsuY9K6DmMB8R+Os5HK2laVEnADh1n3M/spSCHGImnq0gJ7WKTKpdzsCQfZED0WtLhRKi+LfR9KU51n3xem/Zo4BC/nFZoVyOR5ZG9kB8Gv0STNEXCJ4zuSb3MGPzbLPFrq2BTWWxHizj/wnzuhj2nD7O5HSW2+i05W313hI9pkM0NDWlK15DZhkNEhD+q75vyidGoqyJyMNUvQAH0eGxp+2VlpHoKDI9zH0grd71z74D5uGl+5COZcY9bfeDSy+UnMg15NCX3Q7mWKNbNcHIIMjuLigYepEwxC4LePOlmZyy0V63VtDqrFqqV6vwThd+c1A3qWq/MYfdkJyOeRTG5Gavtr4nh/43nCOFgLyLt1g12X648C57gh26M6lza7UYrXba1bO0HZN5L0vivCRzpvN3XcVRV85tqLwy+P2Xg4xHCErJ6RFuMddrz/epu0zIOplqKYJbPbI= 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 Thu, 27 Jul 2023, David Howells wrote: > Fix shmem_splice_read() to use the inode from in->f_mapping->host rather > than file_inode(in) and to skip the splice if it starts after s_maxbytes, > analogously with fixes to filemap_splice_read(). > > Fixes: bd194b187115 ("shmem: Implement splice-read") > Signed-off-by: David Howells Thanks for trying to keep them in synch, but I already had to look into both of these two "fixes" before sending my patch to Andrew, and neither appears to be needed. Neither of them does any harm either, but I think I'd prefer Andrew to drop this patch from mm-unstable, just because it changes nothing. Comment on each below... > cc: Hugh Dickins > cc: Christoph Hellwig > cc: Jens Axboe > cc: Al Viro > cc: John Hubbard > cc: David Hildenbrand > cc: Matthew Wilcox > cc: Chuck Lever > cc: linux-block@vger.kernel.org > cc: linux-fsdevel@vger.kernel.org > cc: linux-mm@kvack.org > --- > mm/shmem.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/mm/shmem.c b/mm/shmem.c > index 0164cccdcd71..8a16d4c7092b 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -2780,13 +2780,16 @@ static ssize_t shmem_file_splice_read(struct file *in, loff_t *ppos, > struct pipe_inode_info *pipe, > size_t len, unsigned int flags) > { > - struct inode *inode = file_inode(in); > + struct inode *inode = in->f_mapping->host; Haha, it's years and years since I had to worry about that distinction: I noticed you'd made that change in filemap, and had to refresh old memories, before concluding that this change is not needed. shmem_file_splice_read() is specified only in shmem_file_operations, and shmem_file_operations is connected up only when S_IFREG; so block and char device nodes will not ever arrive at shmem_file_splice_read(). And shmem does not appear to be out of step there: I did not search through many filesystems, but it appeared to be normal that only the regular files reach the splice_read. Which made me wonder whether the file_inode -> f_mapping->host change was appropriate elsewhere. Wouldn't the splice_read always get called on the bd_inode? Ah, perhaps not: init_special_inode() sets i_fop to def_blk_fops (for shmem and everyone else), and that points to filemap_splice_read(), which explains why just that one needs to pivot to f_mapping->host (I think). > struct address_space *mapping = inode->i_mapping; > struct folio *folio = NULL; > size_t total_spliced = 0, used, npages, n, part; > loff_t isize; > int error = 0; > > + if (unlikely(*ppos >= inode->i_sb->s_maxbytes)) > + return 0; > + > /* Work out how much data we can actually add into the pipe */ > used = pipe_occupancy(pipe->head, pipe->tail); > npages = max_t(ssize_t, pipe->max_usage - used, 0); len = min_t(size_t, len, npages * PAGE_SIZE); do { if (*ppos >= i_size_read(inode)) break; I've added to the context there, to show that the very first thing the do loop does is check *ppos against i_size: so there's no need for that preliminary check against s_maxbytes - something would be wrong already if the file has grown beyond s_maxbytes. So, thanks for the patch, but shmem does not need it. Hugh