linux-kernel - Re: [patch 2/3] fs: introduce perform

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]

Date:	Fri, 9 Mar 2007 10:39:13 +0000
From:	Christoph Hellwig <hch@...radead.org>
To:	Nick Piggin <npiggin@...e.de>
Cc:	Linux Filesystems <linux-fsdevel@...r.kernel.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [patch 2/3] fs: introduce perform_write aop

Hi Nick,

sorry for my later reply, this has been on my to answer list for the last
month and I only managed to get back to it now.

On Thu, Feb 08, 2007 at 02:07:36PM +0100, Nick Piggin wrote:
> Add a new "perform_write" aop, which replaces prepare_write and commit_write
> as a single call to copy a given amount of userdata at the given offset. This
> is more flexible, because the implementation can determine how to best handle
> errors, or multi-page ranges (eg. it may use a gang lookup), and only requires
> one call into the fs.

I really like this idea, especially for avoiding to call into the allocator
for every block.  Have you contacted the reiser4 folks whether this would
superceed their batch_write op completely?

> One problem with this interface is that it cannot be used to write into the
> filesystem by any means other than already-initialised buffers via iovecs. So
> prepare/commit have to stay around for non-user data... 

Actually I think that's a a good thing to a certain extent.  It reminds
us that all other users are horrible abuse of the interface.  I'd even
go so far as to make batch_write a callback that the filesystem passes
to generic_file_aio_write to make clear it's not a generic thing but
a helper.  (It's not a generic thing because it's the upper layer writing
into the pagecache, not a pagecache to fs below operation).

The still leaves open on how to get rid of ->prepare_write and ->commit_write
compltely, and for that we'll probably need ->kernel_read and ->kernel_write
file operations.  But that's a step you shouldn't consider yet when doing
this work.

> Another thing is that it seems to be less able to be implemented in generic,
> reusable code. It should be possible to introduce a new 2-op interface (or
> maybe just a new error handler op) which can be used correctly in generic code.

We should be able to find a nice abstraction for this, see my next mails.

> +	/*
> +	 * perform_write replaces prepare and commit_write callbacks.
> +	 */

This is a rather useless comment :)  Better remove it and add a proper
descriptions to Documentation/filesystems/vfs.txt and
Documentation/filesystems/Locking

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