All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: David Howells <dhowells@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [RESEND][PATCH 4/6] Add page becoming writable notification
Date: Fri, 15 Oct 2004 16:34:40 +0100	[thread overview]
Message-ID: <20041015153440.GA22607@infradead.org> (raw)
In-Reply-To: <28544.1097852703@redhat.com>

On Fri, Oct 15, 2004 at 04:05:03PM +0100, David Howells wrote:
> 
> > > +	/* 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.

huh?  that doesn't make any sense.  if a filesystem needs to do something
special win regards to the VM it should overload vm_operations.  Currently
that's only ncpfs and xfs.

> > 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.

Splitting in helpers makes sense if there's a sane interface.  The number of
arguments doesn't exactly imply that it's the case here.


WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: David Howells <dhowells@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [RESEND][PATCH 4/6] Add page becoming writable notification
Date: Fri, 15 Oct 2004 16:34:40 +0100	[thread overview]
Message-ID: <20041015153440.GA22607@infradead.org> (raw)
In-Reply-To: <28544.1097852703@redhat.com>

On Fri, Oct 15, 2004 at 04:05:03PM +0100, David Howells wrote:
> 
> > > +	/* 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.

huh?  that doesn't make any sense.  if a filesystem needs to do something
special win regards to the VM it should overload vm_operations.  Currently
that's only ncpfs and xfs.

> > 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.

Splitting in helpers makes sense if there's a sane interface.  The number of
arguments doesn't exactly imply that it's the case here.

--
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: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

  reply	other threads:[~2004-10-15 15:35 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-10-14 19:05 [RESEND][PATCH 4/6] Add page becoming writable notification David Howells
2004-10-14 19:05 ` David Howells
2004-10-14 19:29 ` Matthew Wilcox
2004-10-14 19:29   ` Matthew Wilcox
2004-10-14 20:35 ` Christoph Hellwig
2004-10-14 20:35   ` Christoph Hellwig
2004-10-15 15:05   ` David Howells
2004-10-15 15:05     ` David Howells
2004-10-15 15:34     ` Christoph Hellwig [this message]
2004-10-15 15:34       ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20041015153440.GA22607@infradead.org \
    --to=hch@infradead.org \
    --cc=dhowells@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.