linux-kernel - Re: [PATCH 02/11] vfs: Add better VFS support for page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090706090804.GM2714@wotan.suse.de>
Date:	Mon, 6 Jul 2009 11:08:04 +0200
From:	Nick Piggin <npiggin@...e.de>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	Jan Kara <jack@...e.cz>, LKML <linux-kernel@...r.kernel.org>,
	linux-mm@...ck.org, linux-fsdevel@...r.kernel.org
Subject: 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:
> > 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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/