From: Nick Piggin <npiggin@suse.de>
To: Christoph Hellwig <hch@infradead.org>
Cc: Jan Kara <jack@suse.cz>, LKML <linux-kernel@vger.kernel.org>,
linux-mm@kvack.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 02/11] vfs: Add better VFS support for page_mkwrite when blocksize < pagesize
Date: Mon, 6 Jul 2009 11:08:04 +0200 [thread overview]
Message-ID: <20090706090804.GM2714@wotan.suse.de> (raw)
In-Reply-To: <20090704151801.GA19682@infradead.org>
On Sat, Jul 04, 2009 at 11:18:01AM -0400, Christoph Hellwig wrote:
> On Thu, Jul 02, 2009 at 09:22:25AM +0200, Nick Piggin wrote:
> > I guess truncate can be considered special because it operates on
> > data not only metadata.
> >
> > Looks like ->setsize would need a flag for ATTR_OPEN too? Any others?
> > I'll do a bit of an audit when I get around to it...
>
> In the end ATTR_SIZE should not be passed to ->setattr anymore, and
> ->setsize should become mandatory. For the transition I would recommend
> calling ->setsize if present else fall back to the current way. That
> way we can migreate one filesystem per patch to the new scheme.
>
> I would suggest giving the flags to ->setsize their own namespace with
> two flags so far SETSIZE_FTRUNCATE (need to update the file size and
> have a file struct available) and SETSIZE_OPEN for the ATTR_OPEN case.
>
> That beeing said I reallye hate the conditiona file argument for
> ftrunctate (currently hidden inside struct iattr), maybe we're better
> off having am optional int (*ftruncate)(struct file *) method for those
> filesystems that need it, with a fallback to ->setsize.
OK, hmm, but I wonder -- most of the time do_truncate will need to
call notify_change anyway, so I wonder if avoiding the double
indirection saves us anything? (It requires 2 indirect calls either
way). And if we call ->setsize from ->setattr, then a filesystem
which implements its own ->setattr could avoid one of those indirect
calls. Not so if do_truncate has to call ->setattr then ->setsize.
We definitely could call the method ->ftruncate, however (regardless
of where we call it from). In fact, we could just have a single new
->ftruncate where struct file * argument is NULL if not called via
an open file. This should also solve the namimg issue without
renaming the old method (having both ->truncate and ->ftruncate
could be slightly confusing at a glance, but we will remove
->truncate ASAP).
Anyway, let me finish the first draft and post my series and we
can go over it further.
> And yeah, maybe ->setsize might better be left as ->truncate, but if
> we want a nicely bisectable migration we'd have to rename the old
> truncate to e.g. ->old_truncate before. That's probably worth having
> the better naming in the end.
It is definitely better to not break things as my first patch has
done. I think it should not be too hard to have intermediate steps.
Thanks,
Nick
--
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:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2009-07-06 9:08 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-06-15 17:59 [PATCH 0/10] Fix page_mkwrite() for blocksize < pagesize (version 3) Jan Kara
2009-06-15 17:59 ` [PATCH 01/11] ext3: Get rid of extenddisksize parameter of ext3_get_blocks_handle() Jan Kara
2009-06-17 10:28 ` Nick Piggin
2009-06-17 11:49 ` Jan Kara
2009-06-15 17:59 ` [PATCH 02/11] vfs: Add better VFS support for page_mkwrite when blocksize < pagesize Jan Kara
2009-06-25 16:17 ` Nick Piggin
2009-06-25 16:43 ` Nick Piggin
2009-06-25 17:47 ` Christoph Hellwig
2009-06-26 8:42 ` Nick Piggin
2009-06-30 17:37 ` Christoph Hellwig
2009-07-02 7:22 ` Nick Piggin
2009-07-04 15:18 ` Christoph Hellwig
2009-07-06 9:08 ` Nick Piggin [this message]
2009-07-06 10:35 ` Christoph Hellwig
2009-07-06 11:49 ` Nick Piggin
2009-06-26 12:21 ` Jan Kara
2009-06-26 12:55 ` Nick Piggin
2009-06-26 16:08 ` Jan Kara
2009-06-29 5:54 ` Nick Piggin
2009-06-15 17:59 ` [PATCH 03/11] ext2: Allocate space for mmaped file on page fault Jan Kara
2009-06-15 17:59 ` [PATCH 04/11] ext4: Make sure blocks are properly allocated under mmaped page even when blocksize < pagesize Jan Kara
2009-06-15 17:59 ` [PATCH 05/11] ext3: Allocate space for mmaped file on page fault Jan Kara
2009-06-15 17:59 ` [PATCH 06/11] vfs: Implement generic per-cpu counters for delayed allocation Jan Kara
2009-06-15 17:59 ` [PATCH 07/11] vfs: Unmap underlying metadata of new data buffers only when buffer is mapped Jan Kara
2009-06-17 10:35 ` Nick Piggin
2009-06-17 12:05 ` Jan Kara
2009-06-17 13:53 ` Nick Piggin
2009-06-18 12:00 ` Theodore Tso
2009-06-18 11:51 ` OGAWA Hirofumi
2009-06-15 17:59 ` [PATCH 08/11] fs: Don't clear dirty bits in block_write_full_page() Jan Kara
2009-06-15 17:59 ` [PATCH 09/11] vfs: Export wakeup_pdflush Jan Kara
2009-06-15 17:59 ` [PATCH 10/11] ext3: Implement delayed allocation on page_mkwrite time Jan Kara
2009-06-15 18:02 ` [PATCH 0/10] Fix page_mkwrite() for blocksize < pagesize (version 3) Jan Kara
2009-06-15 18:17 ` Aneesh Kumar K.V
2009-06-16 10:28 ` Jan Kara
2009-06-16 14:34 ` Christoph Hellwig
2009-06-16 14:42 ` Jan Kara
2009-06-30 17:44 ` Christoph Hellwig
2009-07-01 10:29 ` Aneesh Kumar K.V
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=20090706090804.GM2714@wotan.suse.de \
--to=npiggin@suse.de \
--cc=hch@infradead.org \
--cc=jack@suse.cz \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).