[PATCH] XFS tuning on software RAID5 partitionable array; was: MDP major registration

[PATCH] XFS tuning on software RAID5 partitionable array; was: MDP major registration

[Hubert Verstraete]

[-- Attachment #1: Type: text/plain, Size: 2224 bytes --]

Hi XFS list,

please find attached a patch for libdisk/mkfs.xfs which tunes XFS on 
software partitionable RAID arrays, also called mdp.

Hubert Verstraete

Hubert Verstraete wrote:
> Bill Davidsen wrote:
>> Luca Berra wrote:
>>> On Tue, Mar 25, 2008 at 05:57:06PM +0100, Hubert Verstraete wrote:
>>>> Neil Brown wrote:
>>>>> On Thursday March 13, hubskml@free.fr wrote:
>>>>>> Neil,
>>>>>>
>>>>>> What is the status of the major for the partitionable arrays ?
>>>>>
>>>>> automatically determined at runtime.
>>>>>
>>>>>> I see that it is 254, which is in the experimental section, 
>>>>>> according to the official Linux device list 
>>>>>> (http://www.lanana.org/docs/device-list/).
>>>>>> Will there be an official registration ?
>>>>>
>>>>> No. Is there any need?
>>>>
>>>> I got this question in mind when I saw that mkfs.xfs source code was 
>>>> referring to the MD major to tune its parameters on an MD device, 
>>>> while it ignores MDP devices.
>>>> If there were reasons to register MD, wouldn't they apply to MDP too ?
>>>
>>> i don't think so:
>>> bluca@percy ~ $ grep mdp /proc/devices
>>> 253 mdp
>>
>> Why is it important to have XFS tune its parameters for md and not for 
>> mdp? I don't understand your conclusion here, is tuning not needed for 
>> mdp, or so meaningless that it doesn't matter, or that XFS code reads 
>> /proc/devices, or ??? I note that device-mapper also has a dynamic 
>> major, what does XFS make of that?
> 
> It reads from /proc/devices.
> 
>> I don't know how much difference tuning makes, but if it's worth doing 
>> at all, it should be done for mdp as well, I would think.
> 
> Same thought. I wrote the patch for mkfs.xfs but did not publish it for 
> two reasons:
> 1) MD is registered but not MDP. Now I understand, it's not a problem, 
> we just need to read /proc/devices as device-mapper does.
> 2) Tuning XFS for MDP can be achieved through the mkfs.xfs options. With 
> a few lines in shell, my XFS on MDP now has the same performance as XFS 
> on MD.
> 
> Hubert
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: xfsprogs_mdp.patch --]
[-- Type: text/x-patch, Size: 2082 bytes --]

diff -u -r xfsprogs-2.8.11/libdisk/md.c xfsprogs-2.8.11-mdp/libdisk/md.c
--- xfsprogs-2.8.11/libdisk/md.c	2006-06-26 07:01:15.000000000 +0200
+++ xfsprogs-2.8.11-mdp/libdisk/md.c	2008-03-26 20:12:38.000000000 +0100
@@ -24,8 +24,12 @@
 	dev_t		dev)
 {
 	if (major(dev) == MD_MAJOR)
-		return 1;
-	return get_driver_block_major("md", major(dev));
+		return MD_IS_MD;
+	if (get_driver_block_major("md", major(dev)))
+		return MD_IS_MD;
+	if (get_driver_block_major("mdp", major(dev)))
+		return MD_IS_MDP;
+	return 0;
 }
 
 int
@@ -37,12 +41,32 @@
 	int		*sectalign,
 	struct stat64	*sb)
 {
-	if (mnt_is_md_subvol(sb->st_rdev)) {
+	char *pc, *dfile2 = NULL;
+	int is_md;
+
+	if ((is_md = mnt_is_md_subvol(sb->st_rdev))) {
 		struct md_array_info	md;
 		int			fd;
 
+		if (is_md == MD_IS_MDP) {
+			if (!(pc = strrchr(dfile, 'd'))
+			|| !(pc = strchr(pc, 'p'))) {
+				fprintf(stderr,
+					_("Error getting MD array device from %s\n"),
+					dfile);
+				exit(1);
+			}
+			dfile2 = (char *) malloc(pc - dfile + 1);
+			if (dfile2 == NULL) {
+				fprintf(stderr,
+					_("Couldn't malloc device string\n"));
+				exit(1);
+			}
+			strncpy(dfile2, dfile, pc - dfile);
+			dfile2[pc - dfile + 1] = '\0';
+		}
 		/* Open device */
-		fd = open(dfile, O_RDONLY);
+		fd = open(dfile2 ? dfile2 : dfile, O_RDONLY);
 		if (fd == -1)
 			return 0;
 
@@ -50,10 +74,11 @@
 		if (ioctl(fd, GET_ARRAY_INFO, &md)) {
 			fprintf(stderr,
 				_("Error getting MD array info from %s\n"),
-				dfile);
+				dfile2 ? dfile2 : dfile);
 			exit(1);
 		}
 		close(fd);
+		if (dfile2) free(dfile2);
 
 		/*
 		 * Ignore levels we don't want aligned (e.g. linear)
diff -u -r xfsprogs-2.8.11/libdisk/md.h xfsprogs-2.8.11-mdp/libdisk/md.h
--- xfsprogs-2.8.11/libdisk/md.h	2006-06-26 07:01:15.000000000 +0200
+++ xfsprogs-2.8.11-mdp/libdisk/md.h	2008-03-26 20:12:10.000000000 +0100
@@ -20,6 +20,9 @@
 #define MD_MAJOR		9 /* we also check at runtime */
 #endif
 
+#define MD_IS_MD		1
+#define MD_IS_MDP		2
+
 #define GET_ARRAY_INFO          _IOR (MD_MAJOR, 0x11, struct md_array_info)
 
 #define MD_SB_CLEAN		0

Re: [PATCH] XFS tuning on software RAID5 partitionable array; error in xfsprogs 2.9.8

[Hubert Verstraete]

Hello

First, thank you for including the patch in xfsprogs-2.9.8.
I have just reviewed the new source code; unfortunately, I found out a 
change from the original patch that could generate a segmentation fault.

The line 89 in libdisk/md.c is: "free(dfile2);" but it should have 
remained "if (dfile2) free(dfile2);" because dfile2 is null in case 
mkfs.xfs is run on an MD device.

Regards,
Hubert Verstraete

Hubert Verstraete wrote:
> Hi XFS list,
>
> please find attached a patch for libdisk/mkfs.xfs which tunes XFS on 
> software partitionable RAID arrays, also called mdp.
>
> Hubert Verstraete
>
> ------------------------------------------------------------------------
> 
> diff -u -r xfsprogs-2.8.11/libdisk/md.c xfsprogs-2.8.11-mdp/libdisk/md.c
> --- xfsprogs-2.8.11/libdisk/md.c	2006-06-26 07:01:15.000000000 +0200
> +++ xfsprogs-2.8.11-mdp/libdisk/md.c	2008-03-26 20:12:38.000000000 +0100
> @@ -24,8 +24,12 @@
>  	dev_t		dev)
>  {
>  	if (major(dev) == MD_MAJOR)
> -		return 1;
> -	return get_driver_block_major("md", major(dev));
> +		return MD_IS_MD;
> +	if (get_driver_block_major("md", major(dev)))
> +		return MD_IS_MD;
> +	if (get_driver_block_major("mdp", major(dev)))
> +		return MD_IS_MDP;
> +	return 0;
>  }
>  
>  int
> @@ -37,12 +41,32 @@
>  	int		*sectalign,
>  	struct stat64	*sb)
>  {
> -	if (mnt_is_md_subvol(sb->st_rdev)) {
> +	char *pc, *dfile2 = NULL;
> +	int is_md;
> +
> +	if ((is_md = mnt_is_md_subvol(sb->st_rdev))) {
>  		struct md_array_info	md;
>  		int			fd;
>  
> +		if (is_md == MD_IS_MDP) {
> +			if (!(pc = strrchr(dfile, 'd'))
> +			|| !(pc = strchr(pc, 'p'))) {
> +				fprintf(stderr,
> +					_("Error getting MD array device from %s\n"),
> +					dfile);
> +				exit(1);
> +			}
> +			dfile2 = (char *) malloc(pc - dfile + 1);
> +			if (dfile2 == NULL) {
> +				fprintf(stderr,
> +					_("Couldn't malloc device string\n"));
> +				exit(1);
> +			}
> +			strncpy(dfile2, dfile, pc - dfile);
> +			dfile2[pc - dfile + 1] = '\0';
> +		}
>  		/* Open device */
> -		fd = open(dfile, O_RDONLY);
> +		fd = open(dfile2 ? dfile2 : dfile, O_RDONLY);
>  		if (fd == -1)
>  			return 0;
>  
> @@ -50,10 +74,11 @@
>  		if (ioctl(fd, GET_ARRAY_INFO, &md)) {
>  			fprintf(stderr,
>  				_("Error getting MD array info from %s\n"),
> -				dfile);
> +				dfile2 ? dfile2 : dfile);
>  			exit(1);
>  		}
>  		close(fd);
> +		if (dfile2) free(dfile2);
>  
>  		/*
>  		 * Ignore levels we don't want aligned (e.g. linear)
> diff -u -r xfsprogs-2.8.11/libdisk/md.h xfsprogs-2.8.11-mdp/libdisk/md.h
> --- xfsprogs-2.8.11/libdisk/md.h	2006-06-26 07:01:15.000000000 +0200
> +++ xfsprogs-2.8.11-mdp/libdisk/md.h	2008-03-26 20:12:10.000000000 +0100
> @@ -20,6 +20,9 @@
>  #define MD_MAJOR		9 /* we also check at runtime */
>  #endif
>  
> +#define MD_IS_MD		1
> +#define MD_IS_MDP		2
> +
>  #define GET_ARRAY_INFO          _IOR (MD_MAJOR, 0x11, struct md_array_info)
>
>  #define MD_SB_CLEAN		0

Re: [PATCH] XFS tuning on software RAID5 partitionable array; error in xfsprogs 2.9.8

[Andi Kleen]

Hubert Verstraete <hubskml@free.fr> writes:
>
> The line 89 in libdisk/md.c is: "free(dfile2);" but it should have
> remained "if (dfile2) free(dfile2);" because dfile2 is null in case
> mkfs.xfs is run on an MD device.

ISO-C free() is defined to do nothing on a NULL input.

-Andi