Re: [RESEND][PATCH 4/6] Add page becoming writable notification

[David Howells]

> > +	/* 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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/