Date:	Fri, 16 Oct 2009 15:47:35 +0900
From:	Kazuya Mio <k-mio@...jp.nec.com>
To:	Greg Freemyer <greg.freemyer@...il.com>
CC:	Akira Fujita <a-fujita@...jp.nec.com>,
	Andreas Dilger <adilger@....com>, Theodore Tso <tytso@....edu>,
	ext4 <linux-ext4@...r.kernel.org>
Subject: Re: [RFC][PATCH 1/7]ext4: Add EXT4_IOC_ADD_GLOBAL_ALLOC_RULE 	restricts
 block allocation

Hi Greg,
I'm sorry for the late reply.

2009/08/10 3:18, Greg Freemyer wrote:
> Akira-san,
> 
> I joined the project ohsm team a couple weeks ago and we hope to use
> your patches / features to build on.  Below is our feedback as relates
> to ohsm as well as my personal feedback.
> 
[snip]
>> > Ioctl interfaces are as follows.
>> >
>> > a. EXT4_IOC_BALLOC_CONTROL (Set or clear balloc restriction)
>> >
>> >     EXT4_IOC_BALLOC_CONTROL
>> >        _IOW('f', 16, struct ext4_balloc_control balloc_control)
>> >
>> >     struct ext4_balloc_control {
>> >       __u64 start; /* start physical block offset balloc rest */
>> >       __u64 len;   /* block length */
>> >       __u32 flags; /* set or clear */
>> >     }
>> >
>> >    "flags" can be set following 2 types.
>> >    - SET_BALLOC_RESTRICTION
>> >        Set blocks in range to the balloc restriction list.
>> >    - CLR_BALLOC_RESTRICTION
>> >        Clear blocks from the balloc restriction list.
> 
> ohsm will be an in kernel user of the above, so we hope a kernel API
> is also provided.  I assume that would be a simple export and
> documenting it in Documentation/filesystems/ext4.
> 

When we implement this ioctl, we will consider about a kernel API.
However, we have no plan to make the kernel API.

Also we consider the documenting after we decide the specific of the
implementation.

> It seems you need to add 3 flags to the above:
> mandatory - Have a future block allocate request return ENO_SPACE_PA
> if the blocks cannot be found within the restricted range.
> advisory - Attempt future block allocate requests from the restricted
> range, but use entire unrestricted block range if that fails.
> mandatory_with_fallback - Not Implemented - If block allocate from
> restricted range fails, fallback to an alternate block range.  API and
> implementation details not yet agreed on.

I'm worried that you misunderstand about the restriction PA.
If you set a restriction PA, a block allocation cannot allocate
blocks into there. Setting a block range that is allocated first
is the role of the inode PA.

> 
[snip]
> 
>> > b. EXT4_IOC_ADD_PREALLOC (Add inode preferred range)
>> >
>> >     EXT4_IOC_ADD_PREALLOC _IOW('f', 18, struct ext4_balloc_control)
>> >
>> >     struct ext4_balloc_control {
>> >       __u64 start; /* start physical block offset */
>> >       __u64 len;   /* block length */
>> >       __u32 flags;  /* create and add mode for inode PA  */
>> >     }
>> >
>> >    "flags" must include one of the following create modes
>> >    (MANDATORY or ADVISORY).  In addition, one of the control modes also must
>> >    be set (REPLACER_INODE_PREALLOC or ADD_INODE_PREALLOC).
>> >     Create modes:
>> >     - MANDATORY
>> >         Find free extent which satisfies "start" and "len" completely.
>> >     - ADVISORY
>> >         Try to find free extent from "start" and "len" blocks.
>> >     Control modes:
>> >     - REPLACE_INODE_PREALLOC
>> >         Remove existed inode PA first, and then add specified range to
>> >         the inode PA list newly.
>> >     - ADD_INODE_PREALLOC
>> >         Add specified range to the inode PA list.
>> >
>> >     e.g.  flag = MANDATORY | ADD_INODE_PREALLOC
>> >           Find free extent which fulfills the requirements completely,
>> >           and if succeed, add this extent to the inode PA.
> 
> I am unsure how the above relates to EXT4_IOC_BALLOC_CONTROL.  It
> appears to be totally independent which I don't think is a good idea.
> Nor do I understand the use case of the advisory flag and
> add_inode_prealloc flag.
> 
> I would prefer if the above API were simplified to:
> 
> b. EXT4_IOC_RESET_PREALLOC (Ensure inode prealloc range is withing
> preferred block alloc range)
> 
>     EXT4_IOC_ADD_PREALLOC _IOW('f', 18, struct ext4_balloc_control)
> 
>     struct ext4_balloc_control {
>       __u32 flags;  /* Currently unused  */
>     }
> 
> Find appropriate free prealloc block extent within range set of inode
> via EXT4_IOC_BALLOC_CONTROL.
> 
> If unable to do so, a preallock block is set via the default logic and
> a error is returned to show that the prealloc block is not within the
> restricted block range.
> 
> This seems far simpler to code, understand, and use.
> 

In advisory mode of the inode PA, the ioctl tries to get the inode PA
that satisfies "start" and "len" completely. If it fails, the ioctl
gets an inode PA from somewhere. Your suggestion seems like this mode.
However, the mandatory mode of the inode PA is necessary for e4defrag,
so struct ext4_balloc_control has flag field that can be set two modes.

In ADD_INODE_PREALLOC mode of the inode PA, the ioctl appends an inode PA
without any changing of existed inode PAs. With hindsight, this flag
is not necessary for e4defrag, so I will remove the flags
ADD_INODE_PREALLOC and REPLACE_INODE_PREALLOC.

Regards,
Kazuya Mio

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists