Message-ID: <20070731135642.GA11550@amitarora.in.ibm.com>
Date:	Tue, 31 Jul 2007 19:26:42 +0530
From:	"Amit K. Arora" <aarora@...ux.vnet.ibm.com>
To:	Michael Kerrisk <mtk-manpages@....net>
Cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	dgc@....com
Subject: Re: fallocate() man page

Hi Michael,

On Mon, Jul 30, 2007 at 09:43:08PM +0200, Michael Kerrisk wrote:
> Hello Amit.
> 
> > On Mon, Jul 23, 2007 at 08:09:45AM +0200, Michael Kerrisk wrote:
> >> Amit,
> >>
> >> I've taken the page that you sent and made various minor formatting and
> >> wording fixes.  I've also added various FIXMEs to the page.  Some of these
> >> ("FIXME .") are things that I need to check up later.  Some others are
> >> questions for which I need input from you, David, or someone else with the
> >> relevant info (I've marked these "FIXME Amit:").  Could you please review,
> >> and send a new draft of the page back to me.
> > 
> > Thanks for going through the manpage and improving it!
> > 
> > My comments are below in between <Amit> ... </Amit> tags.
> > 
> > Thanks!
> [...]
> 
> > The
> > .I mode
> > argument determines the operation to be performed on the given range.
> > Currently only one flag is supported for
> > .IR mode :
> > .TP
> > .B FALLOC_FL_KEEP_SIZE
> > allocates and initializes to zero the disk space within the given range.
> > .\" FIXME Amit: The next two sentences seem to contradict
> > .\" each other somewhat.  On the one hand, later writes
> > .\" are guaranteed not to fail for lack of space; on the other
> > .\" hand, the file size id not changed even if it is currently
> > .\" smaller than offset+len bytes.
> > .\" Could you explain this a little further.  (E.g., how does
> > .\" the kernel guarantee space without changing the size
> > .\" of the file?)
> > .\" <Amit>
> > .\"     Well, this is a feature where you can allocate/reserve space for
> > .\" a file without changing the file size. This is done by allocating blocks
> > .\" to the file, but still not changing the size. As mentioned below, this
> > .\" helps applications that use append mode a lot. These can open
> > .\" a file in append mode and start writing to "preallocated" space.
> > .\" So, if someone does a stat on a file after fallocate() with this mode (where
> > .\" file size is not changed), he/she will see that the st_blocks
> > .\" increased, but st_size did not change.
> > .\" </Amit>
> 
> Okay -- I tried rewording the text here a little to make this clearer.  Can
> you review the new version to see that it's okay.
> 
> [...]
<Amit>
Ok. Will review the draft version soon and will get back to you.
</Amit>
 
> > .\" FIXME Amit: Which other flags are likely to appear
> > .\" for mode, and in which kernel version are they likely?
> > .\" <Amit>
> > .\"    There were few more flags which were discussed, but none of
> > .\" them have been finalized upon. Here are these flags:
> > .\" FA_FL_DEALLOC, FA_FL_DEL_DATA, FA_FL_ERR_FREE, FA_FL_NO_MTIME, FA_FL_NO_CTIME
> > .\" All of the above flags were debated upon and we can not say if any/which one
> > .\" of these flags will make it to the later kernels.
> > .\" </Amit>
> 
> Thanks for the info.
> 
> [...]
> 
> > .\" FIXME Amit: is it worth adding a few words to the following
> > .\" sentence to say why fallocate() may allocate a larger range
> > .\" than specified?
> > .\" <Amit>
> > .\"     The preallocation is done in block size chunks. Thus, if the last
> > .\" few bytes in the range falls in a new block, this entire block gets
> > .\" allocated to the file. Hence we may have slightly larger range allocated.
> > .\" I have tried to add one line to explain this below. Please see if it
> > .\" makes sense and is understandable. Thanks!
> > .\" </Amit>
> 
> Thanks.
> 
> > .PP
> > .BR fallocate ()
> > may allocate a larger range than that was specified.
> > .\" <Amit>
> > .\" This is because allocation is done in block size chunks and hence
> > .\" the allocation will automatically get block aligned.
> > .\" </Amit>
> 
> I made the sentence:
> 
>     Because allocation is done in block size chunks, fallocate()
>     may allocate a larger range than that which was specified.
> 
> okay?
> 
> [...]

<Amit>
Ok.
</Amit>
 
> > .TP
> > .B ENODEV
> > .I fd
> > does not refer to a regular file or a directory.
> > .TP
> > .B ENOSPC
> > There is not enough space left on the device containing the file
> > referred to by
> > .IR fd .
> > .TP
> > .B ESPIPE
> > .I fd
> > refers to a pipe of file descriptor.
> > .\" FIXME Amit: ENODEV says "fd is not a file or a directory";
> > .\" ESPIPE says (I had to fix the text a little) "refers to a pipe".
> > .\" This doesn't make sense: if fd is a pipe, then either one
> > .\" of these errors could occur.  Which is it supposed to be?
> > .\" <Amit>
> > .\"    This is inline with posix_fallocate manpage. If it is a pipe,
> > .\" user will get ESPIPE.
> > .\" </Amit>
> 
> Okay -- thanks.  I reworded the text for the ESNODEV error to make this
> clearer.  (Please check the wording in the next draft.)

<Amit>
Sure.
</Amit>
 
> By the way in fs/open.c I see the comment:
> 
>         /*
>          * Let individual file system decide if it supports preallocation
>          * for directories or not.
>          */
>         if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
>                 goto out_fput;
> 
> But that comment doesn't seem to accord with the line of code immediately
> below it (S_ISDIR() check is doene regardles of file system type).  Do I
> misunderstand something -- or is the comment wrong?
> 
> [...]

<Amit>
I think it is correct. We are failing ("goto out_fput;") _only_ if it is
not a regular file AND also not a directory. In the case when the
concerned object is a directory, the above "if" condition won't be true
and thus the "goto" won't get called. Hence, the individual file
system's ->fallocate() inode op will be called, which will decide if it
wants to support directories or not.
</Amit>
 
> > .TP
> > .B EOPNOTSUPP
> > .\" FIXME Amit: can you say a little more about the following error
> > .\" <Amit>
> > .\" How does following sound ?
> > .\" 'The specified mode is not supported on the object by the file system.'
> > .\" </Amit>
> 
> I made it:
> 
>     The mode is not supported by the file system containing the file
>     referred to by fd.
> 
> Okay?
> 
> [...]

<Amit>
Ok.
</Amit>
 
> New version of the page on its way soon.

<Amit>
I have received it. Will review it soon (maybe by tomorrow) and get
back. Thanks!
</Amit>

--
Regards,
Amit Arora
 
> Cheers,
> 
> Michael
-
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/

Powered by blists - more mailing lists