From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Howells In-Reply-To: <20041014203545.GA13639@infradead.org> References: <20041014203545.GA13639@infradead.org> <24449.1097780701@redhat.com> Subject: Re: [RESEND][PATCH 4/6] Add page becoming writable notification MIME-Version: 1.0 (generated by SEMI 1.14.5 - "Awara-Onsen") Content-Type: text/plain; charset=US-ASCII Date: Fri, 15 Oct 2004 16:05:03 +0100 Message-ID: <28544.1097852703@redhat.com> Sender: owner-linux-mm@kvack.org Return-Path: To: Christoph Hellwig Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org List-ID: > > + /* 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 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: aart@kvack.org