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 X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5F764C43218 for ; Fri, 26 Apr 2019 08:30:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 374F7206E0 for ; Fri, 26 Apr 2019 08:30:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726083AbfDZIaT (ORCPT ); Fri, 26 Apr 2019 04:30:19 -0400 Received: from mx2.suse.de ([195.135.220.15]:44696 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726077AbfDZIaT (ORCPT ); Fri, 26 Apr 2019 04:30:19 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 66391AE28; Fri, 26 Apr 2019 08:30:17 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id E06DB1E3618; Fri, 26 Apr 2019 10:30:16 +0200 (CEST) Date: Fri, 26 Apr 2019 10:30:16 +0200 From: Jan Kara To: Andreas Gruenbacher Cc: cluster-devel@redhat.com, Christoph Hellwig , Bob Peterson , Jan Kara , Dave Chinner , Ross Lagerwall , Mark Syms , Edwin =?iso-8859-1?B?VPZy9ms=?= , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v3 1/2] iomap: Add a page_prepare callback Message-ID: <20190426083016.GA11637@quack2.suse.cz> References: <20190425160913.1878-1-agruenba@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190425160913.1878-1-agruenba@redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Thu 25-04-19 18:09:12, Andreas Gruenbacher wrote: > Move the page_done callback into a separate iomap_page_ops structure and > add a page_prepare calback to be called before a page is written to. In > gfs2, we'll want to start a transaction in page_prepare and end it in > page_done, and other filesystems that implement data journaling will > require the same kind of mechanism. ... > @@ -674,9 +675,17 @@ iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, unsigned flags, > if (fatal_signal_pending(current)) > return -EINTR; > > + if (page_ops) { > + status = page_ops->page_prepare(inode, pos, len, iomap); > + if (status) > + return status; > + } > + Looks OK for now I guess, although I'm not sure if later some fs won't need to get hold of the actual page in ->page_prepare() and then we will need to switch to ->page_prepare() returning the page to use. But let's leave that for a time when such fs wants to use iomap. > @@ -780,8 +794,8 @@ iomap_write_end(struct inode *inode, loff_t pos, unsigned len, > ret = __iomap_write_end(inode, pos, len, copied, page, iomap); > } > > - if (iomap->page_done) > - iomap->page_done(inode, pos, copied, page, iomap); > + if (page_ops) > + page_ops->page_done(inode, pos, copied, page, iomap); Looking at the code now, this is actually flawed (preexisting problem): __iomap_write_end or generic_write_end() will release the page reference and so you cannot just pass it to ->page_done(). That is a potential use-after-free... Honza -- Jan Kara SUSE Labs, CR From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Date: Fri, 26 Apr 2019 10:30:16 +0200 Subject: [Cluster-devel] [PATCH v3 1/2] iomap: Add a page_prepare callback In-Reply-To: <20190425160913.1878-1-agruenba@redhat.com> References: <20190425160913.1878-1-agruenba@redhat.com> Message-ID: <20190426083016.GA11637@quack2.suse.cz> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Thu 25-04-19 18:09:12, Andreas Gruenbacher wrote: > Move the page_done callback into a separate iomap_page_ops structure and > add a page_prepare calback to be called before a page is written to. In > gfs2, we'll want to start a transaction in page_prepare and end it in > page_done, and other filesystems that implement data journaling will > require the same kind of mechanism. ... > @@ -674,9 +675,17 @@ iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, unsigned flags, > if (fatal_signal_pending(current)) > return -EINTR; > > + if (page_ops) { > + status = page_ops->page_prepare(inode, pos, len, iomap); > + if (status) > + return status; > + } > + Looks OK for now I guess, although I'm not sure if later some fs won't need to get hold of the actual page in ->page_prepare() and then we will need to switch to ->page_prepare() returning the page to use. But let's leave that for a time when such fs wants to use iomap. > @@ -780,8 +794,8 @@ iomap_write_end(struct inode *inode, loff_t pos, unsigned len, > ret = __iomap_write_end(inode, pos, len, copied, page, iomap); > } > > - if (iomap->page_done) > - iomap->page_done(inode, pos, copied, page, iomap); > + if (page_ops) > + page_ops->page_done(inode, pos, copied, page, iomap); Looking at the code now, this is actually flawed (preexisting problem): __iomap_write_end or generic_write_end() will release the page reference and so you cannot just pass it to ->page_done(). That is a potential use-after-free... Honza -- Jan Kara SUSE Labs, CR