On Tue, Apr 24, 2007 at 04:59:47PM +1000, Neil Brown wrote:
> On Tuesday April 24, [EMAIL PROTECTED] wrote:
> > +  write_begin: This is intended as a replacement for prepare_write. Called
> > +        by the generic buffered write code to ask the filesystem to prepare
> > +        to write len bytes at the given offset in the file. flags is a 
> > field
> > +        for AOP_FLAG_xxx flags, described in include/linux/mm.h.
> 
> Putting "This is intended as a replacement.." there sees a bit
> dangerous.  It could well accidentally remain when the documentation
> for prepare_write gets removed.  I would make it a separate paragraph
> and flesh it out.  And include text from prepare_write before that
> gets removed.
> 
>    write_begin:
>          This is intended as a replacement for prepare_write.  The key 
>          differences being that:
>               - it returns a locked page (in *pagep) rather than
>                   being given a pre-locked page:
>               - it can pass arbitrary state to write_end rather than
>                 having to hide stuff in some filesystem-internal
>                 data structure 
>               - The (largely undocumented) flags option.
> 
>          Called by  the generic bufferred write code to ask an
>          address_space to prepare to write len bytes at the given
>          offset in the file.
> 
>        The address_space should check that the write will be able to
>        complete, by allocating space if necessary and doing any other
>        internal housekeeping.  If the write will update parts of any
>        basic-blocks on storage, then those blocks should be pre-read
>        (if they haven't been read already) so that the updated blocks
>        can be written out properly.
>        The possible flags are listed in include/linux/fs.h (not
>        mm.h) and include
>               AOP_FLAG_UNINTERRUPTIBLE:
>                       It is unclear how this should be used.  No
>                       current code handles it.

Yeah, reasonable points. I'll do an incremental patch to clean up
some of the documentation.

BTW. AOP_FLAG_UNINTERRUPTIBLE can be used by filesystems to avoid
an initial read or other sequence they might be using to handle the
case of a short write. ecryptfs uses it, others can too.

For buffered writes, this doesn't get passed in (unless they are
coming from kernel space), so I was debating whether to have it at
all.  However, in the previous API, _nobody_ had to worry about
short writes, so this flag means I avoid making an API that is
fundamentally less performant in some situations.

> 
> (together with the rest...)
> > +
> > +        The filesystem must return the locked pagecache page for the caller
> > +        to write into.
> > +
> > +        A void * may be returned in fsdata, which then gets passed into
> > +        write_end.
> > +
> > +        Returns < 0 on failure, in which case all cleanup must be done and
> > +        write_end not called. 0 on success, in which case write_end must
> > +        be called.
> 
> 
> As you are not including perform_write in the current patchset, maybe
> it is best not to include the documentation yet either?

Right, missed that, thanks!

Nick
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to