linux-ext4 - Re: [RFC][PATCH 1/3] Add EXT4_IOC_MOVE

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:	Mon, 15 Jun 2009 17:03:39 +0900
From:	Akira Fujita <a-fujita@...jp.nec.com>
To:	Theodore Tso <tytso@....edu>
CC:	linux-ext4@...r.kernel.org
Subject: Re: [RFC][PATCH 1/3] Add EXT4_IOC_MOVE_EXT ioctl and related	functions

Hi Ted,

Thank you for your time to review my patch.

Theodore Tso wrote:
> On Fri, May 22, 2009 at 04:06:16PM +0900, Akira Fujita wrote:
>> ext4: online defrag -- Add EXT4_IOC_MOVE_EXT ioctl and related functions.
>>
>> From: Akira Fujita <a-fujita@...jp.nec.com>
>>
>> The EXT4_IOC_MOVE_EXT exchanges the blocks between orig_fd and donor_fd,
>> and then write the file data of orig_fd to donor_fd.
>> ext4_mext_move_extent() is the main fucntion of ext4 online defrag,
>> and this patch includes all functions related to ext4 online defrag.
> 
> Akira-san,
> 
> Thank you for all of the hard work and preserverance with the online
> defrag work!  This patch is much, *much* better; I've done a quick
> review, and I've only noted two things, which I've updated in the
> version I've now moved into the stable portion of the patch queue.
> One is that nothing actually uses orig_fd in the move_extent
> structure; so to avoid confusion, and I've renamed it to "reserved",
> and used explicit __u32 fields for the reserved and donor_fd fields.
> Also, I've renamed ext4_mext_move_extent() to ext4_move_extents();
> since it is the one published interface, I wanted it to have an
> easier-to-understand name.

Ok.  Certainly orig_fd of move_extent structure is not used
since fd of original file is passed via ioctl directly.
My recognition after the change is as follows:

struct move_extent {
	__u32 reserved;		/* reserved field */
	__u32 donor_fd;		/* donor file descriptor */
	__u64 orig_start;	/* logical start offset in block for orig */
	__u64 donor_start;	/* logical start offset in block for donor */
	__u64 len;		/* block length to be moved */
	__u64 moved_len;	/* moved block length */
};

int ext4_move_extent(struct file *o_filp, struct file *d_filp,
				__u64 start_orig, __u64 start_donor,
				__u64 len, __u64 *moved_len);

Little changes are needed for command to run ext4 online defrag,
so I will resend patch in a few days.


> As a side note, the static functions in fs/ext4/move_extent.c really
> don't need the ext4_mext prefix, since static functions don't have
> namespace issues that require a consistent naming scheme.  (Sometimes
> a shorter name can also be useful since it avoids needing to line wrap
> function calls with a long list of parameters.)

I will check all of the functions in move_extent.c
whether the function name can be shorter or not.

Regards,
Akira Fujita
--
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