From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Howells Subject: Re: [RESEND][PATCH 4/6] Add page becoming writable notification Date: Fri, 15 Oct 2004 16:05:03 +0100 Sender: linux-fsdevel-owner@vger.kernel.org Message-ID: <28544.1097852703@redhat.com> References: <20041014203545.GA13639@infradead.org> <24449.1097780701@redhat.com> Mime-Version: 1.0 (generated by SEMI 1.14.5 - "Awara-Onsen") Content-Type: text/plain; charset=US-ASCII Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org Return-path: Received: from mx1.redhat.com ([66.187.233.31]:36230 "EHLO mx1.redhat.com") by vger.kernel.org with ESMTP id S268013AbUJOPF0 (ORCPT ); Fri, 15 Oct 2004 11:05:26 -0400 In-Reply-To: <20041014203545.GA13639@infradead.org> To: Christoph Hellwig List-Id: linux-fsdevel.vger.kernel.org > > + /* notification that a page is about to become writable */ > > + int (*page_mkwrite)(struct page *page); > > This doesn't fit into address_space operations at all. The vm_operation > below is enough. Filesystems shouldn't be overloading vm_operations on ordinary files, or so I've been instructed. > > +static inline int do_wp_page_mk_pte_writable(struct mm_struct *mm, > > + struct vm_area_struct *vma, > > + unsigned long address, > > + pte_t *page_table, > > + struct page *old_page, > > + pte_t pte) > > This prototype shows pretty much that splitting it out doesn't make much > sense. Why not add a goto reuse_page; where you call it currently and > append it at the end of do_wp_page? Judging by the CodingStyle doc - which you like throwing at me - it should be split into a separate inline function. I could come up with a better name, I suppose to keep Willy happy too - perhaps make_pte_writable(); it's just that I wanted to name it to show its derivation. David