Re: [PATCH 02/11] vfs: Add better VFS support for page_mkwrite when blocksize < pagesize

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:

quoted

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>